Merge pull request ceph#61253 from Matan-B/wip-matanb-crimson-tri_mutex_pointer

crimson/common/tri_mutex: improve logging

Reviewed-by: Yingxin Cheng <[email protected]>
Reviewed-by: Aishwarya Mathuria <[email protected]>

Author: Matan-B

src/crimson/common/tri_mutex.h

@@ -6,7 +6,6 @@
 #include <seastar/core/future.hh>
 #include <seastar/core/circular_buffer.hh>
 
-#include "common/hobject.h"
 #include "crimson/common/log.h"
 
 class read_lock {
@@ -47,9 +46,10 @@ class tri_mutex : private read_lock,
 public:
   tri_mutex() = default;
 #ifdef NDEBUG
-  tri_mutex(const hobject_t &obj_name) : name() {}
+  tri_mutex(const std::string &obj_str) : name() {}
 #else
-  tri_mutex(const hobject_t &obj_name) : name(obj_name) {}
+  tri_mutex(const std::string &obj_str) : name(obj_str) {
+  }
 #endif
   ~tri_mutex();
 
@@ -101,8 +101,8 @@ class tri_mutex : private read_lock,
     }
   }
 
-  std::string get_name() const{
-    return name.to_str();
+  std::string_view get_name() const{
+    return name;
   }
 
 private:
@@ -124,7 +124,7 @@ class tri_mutex : private read_lock,
     type_t type;
   };
   seastar::circular_buffer<waiter_t> waiters;
-  const hobject_t name;
+  const std::string name;
   friend class read_lock;
   friend class write_lock;
   friend class excl_lock;
@@ -134,9 +134,9 @@ class tri_mutex : private read_lock,
 inline std::ostream& operator<<(std::ostream& os, const tri_mutex& tm)
 {
   os << fmt::format("tri_mutex {} writers {} readers {}"
-                    " exclusively_used {} waiters: {}",
+                    " exclusively_used {} waiters: {} address {}",
                     tm.get_name(), tm.get_writers(), tm.get_readers(),
-                    tm.exclusively_used, tm.waiters.size());
+                    tm.exclusively_used, tm.waiters.size(), fmt::ptr(&tm));
   return os;
 }
 

src/crimson/osd/object_context.h

@@ -76,12 +76,13 @@ class ObjectContext : public ceph::common::intrusive_lru_base<
 
   CommonOBCPipeline obc_pipeline;
 
-  ObjectContext(hobject_t hoid) : lock(hoid),
+  ObjectContext(hobject_t hoid) : lock(hoid.to_str()),
                                   obs(std::move(hoid)) {}
 
-  void update_from(const ObjectContext &obc) {
-    obs = obc.obs;
-    ssc = obc.ssc;
+  void update_from(
+    std::pair<ObjectState, SnapSetContextRef> obc_data) {
+    obs = obc_data.first;
+    ssc = obc_data.second;
   }
 
   const hobject_t &get_oid() const {

src/crimson/osd/pg.cc

@@ -967,12 +967,12 @@ PG::BackgroundProcessLock::lock() noexcept
 }
 
 // We may need to rollback the ObjectContext on failed op execution.
-// Copy the current obc before mutating it in order to recover on failures.
-ObjectContextRef duplicate_obc(const ObjectContextRef &obc) {
-  ObjectContextRef object_context = new ObjectContext(obc->obs.oi.soid);
-  object_context->obs = obc->obs;
-  object_context->ssc = new SnapSetContext(*obc->ssc);
-  return object_context;
+// Copy the current obc data before mutating it in order to recover on failures.
+std::pair<ObjectState, SnapSetContextRef>
+duplicate_obc_data(const ObjectContextRef &obc) {
+  ObjectState os = obc->obs;
+  SnapSetContextRef ssc = new SnapSetContext(*obc->ssc);
+  return {os, ssc};
 }
 
 PG::interruptible_future<> PG::complete_error_log(const ceph_tid_t& rep_tid,
@@ -1089,8 +1089,8 @@ PG::run_executer_fut PG::run_executer(
 {
   LOG_PREFIX(PG::run_executer);
   auto rollbacker = ox.create_rollbacker(
-    [stored_obc=duplicate_obc(obc)](auto &obc) mutable {
-      obc->update_from(*stored_obc);
+    [obc_data = duplicate_obc_data(obc)](auto &obc) mutable {
+      obc->update_from(obc_data);
     });
   auto rollback_on_error = seastar::defer([&rollbacker] {
     rollbacker.rollback_obc_if_modified();