From 9a27c95b958a6f5b6c52087e8d05899dc465befe Mon Sep 17 00:00:00 2001
From: Matan Breizman <mbreizma@redhat.com>
Date: Wed, 3 Apr 2024 08:50:55 +0000
Subject: [PATCH] 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>
---
 src/crimson/osd/object_context_loader.cc      | 44 -------------------
 src/crimson/osd/object_context_loader.h       | 10 +----
 .../osd/osd_operations/snaptrim_event.cc      |  4 +-
 3 files changed, 4 insertions(+), 54 deletions(-)

diff --git a/src/crimson/osd/object_context_loader.cc b/src/crimson/osd/object_context_loader.cc
index 885231decda94..351bb3b46dea5 100644
--- 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 eeadb9daa92c4..a594f3db975e6 100644
--- 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 d75b8e4461a89..7af44c160cb79 100644
--- a/src/crimson/osd/osd_operations/snaptrim_event.cc
+++ b/src/crimson/osd/osd_operations/snaptrim_event.cc
@@ -442,8 +442,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());
-- 
2.39.5