crimson/osd/object_context_loader: cleanup with_clone_obc_direct

ObjectContextLoader interface provides two variants:

* with_obc:
  // Use this variant by default
  // If oid is a clone object, the clone obc *and* it's
  // matching head obc will be locked and can be used in func.

* with_clone_obc_only:
  // Use this variant in the case where the head object
  // obc is already locked and only the clone obc is needed.
  // Avoid nesting with_head_obc() calls by using with_clone_obc()
  // with an already locked head.

with_clone_obc_direct variant is equal to with_obc on a clone obc
since both the head and the clone obcs will be locked and can be used.

Signed-off-by: Matan Breizman <mbreizma@redhat.com>
(cherry picked from commit 9a27c95b958a6f5b6c52087e8d05899dc465befe)

diff --git a/src/crimson/osd/object_context_loader.cc b/src/crimson/osd/object_context_loader.cc
index 885231decda946ddc5910c8af59f92d4fbfbf990..351bb3b46dea5b4defdc61f679b9e76aa1e7e252 100644 (file)
--- a/src/crimson/osd/object_context_loader.cc
+++ b/src/crimson/osd/object_context_loader.cc
@@ -91,45 +91,6 @@ using crimson::common::local_conf;
     });
   }
 
-  template<RWState::State State>
-  ObjectContextLoader::load_obc_iertr::future<>
-  ObjectContextLoader::with_clone_obc_direct(
-    hobject_t oid,
-    with_obc_func_t&& func)
-  {
-    LOG_PREFIX(ObjectContextLoader::with_clone_obc_direct);
-    assert(!oid.is_head());
-    return with_obc<RWState::RWREAD>(
-      oid.get_head(),
-      [FNAME, oid, func=std::move(func), this](auto head, auto) mutable
-      -> load_obc_iertr::future<> {
-      if (!head->obs.exists) {
-        ERRORDPP("head doesn't exist for object {}", dpp, head->obs.oi.soid);
-        return load_obc_iertr::future<>{
-          crimson::ct_error::enoent::make()
-        };
-      }
-#ifndef NDEBUG
-      auto &ss = head->get_head_ss();
-      auto cit = std::find(
-       std::begin(ss.clones), std::end(ss.clones), oid.snap);
-      assert(cit != std::end(ss.clones));
-#endif
-      auto [clone, existed] = obc_registry.get_cached_obc(oid);
-      return clone->template with_lock<State, IOInterruptCondition>(
-        [existed=existed, clone=std::move(clone),
-         func=std::move(func), head=std::move(head), this]()
-        -> load_obc_iertr::future<> {
-        auto loaded = get_or_load_obc<State>(clone, existed);
-        return loaded.safe_then_interruptible(
-          [func = std::move(func), head=std::move(head)](auto clone) {
-          clone->set_clone_ssc(head->ssc);
-          return std::move(func)(std::move(head), std::move(clone));
-        });
-      });
-    });
-  }
-
   template<RWState::State State>
   ObjectContextLoader::load_obc_iertr::future<>
   ObjectContextLoader::with_obc(hobject_t oid,
@@ -248,9 +209,4 @@ using crimson::common::local_conf;
   template ObjectContextLoader::load_obc_iertr::future<>
   ObjectContextLoader::with_obc<RWState::RWEXCL>(hobject_t,
                                                  with_obc_func_t&&);
-
-  template ObjectContextLoader::load_obc_iertr::future<>
-  ObjectContextLoader::with_clone_obc_direct<RWState::RWWRITE>(
-    hobject_t,
-    with_obc_func_t&&);
 }

diff --git a/src/crimson/osd/object_context_loader.h b/src/crimson/osd/object_context_loader.h
index eeadb9daa92c4fa9886f763071a75951b31eaa4d..a594f3db975e6de71ad890353577c5d17485eed6 100644 (file)
--- a/src/crimson/osd/object_context_loader.h
+++ b/src/crimson/osd/object_context_loader.h
@@ -33,6 +33,8 @@ public:
     std::function<load_obc_iertr::future<> (ObjectContextRef, ObjectContextRef)>;
 
   // Use this variant by default
+  // If oid is a clone object, the clone obc *and* it's
+  // matching head obc will be locked and can be used in func.
   template<RWState::State State>
   load_obc_iertr::future<> with_obc(hobject_t oid,
                                     with_obc_func_t&& func);
@@ -46,14 +48,6 @@ public:
                                                hobject_t clone_oid,
                                                with_obc_func_t&& func);
 
-  // Use this variant in the case where both the head
-  // object *and* the matching clone object are being used
-  // in func.
-  template<RWState::State State>
-  load_obc_iertr::future<> with_clone_obc_direct(
-    hobject_t oid,
-    with_obc_func_t&& func);
-
   load_obc_iertr::future<> reload_obc(ObjectContext& obc) const;
 
   void notify_on_change(bool is_primary);

diff --git a/src/crimson/osd/osd_operations/snaptrim_event.cc b/src/crimson/osd/osd_operations/snaptrim_event.cc
index 53efab05131af0487c41c890e84dffc0a85a2829..fea83cbaf46232b2bc04734e7d9f64080cf29f5a 100644 (file)
--- a/src/crimson/osd/osd_operations/snaptrim_event.cc
+++ b/src/crimson/osd/osd_operations/snaptrim_event.cc
@@ -451,8 +451,8 @@ SnapTrimObjSubEvent::start()
   }).then_interruptible([this] {
     logger().debug("{}: getting obc for {}", *this, coid);
     // end of commonality
-    // with_clone_obc_direct lock both clone's and head's obcs
-    return pg->obc_loader.with_clone_obc_direct<RWState::RWWRITE>(
+    // lock both clone's and head's obcs
+    return pg->obc_loader.with_obc<RWState::RWWRITE>(
       coid,
       [this](auto head_obc, auto clone_obc) {
       logger().debug("{}: got clone_obc={}", *this, clone_obc->get_oid());