From: Matan Breizman <mbreizma@redhat.com>
Date: Thu, 6 Jun 2024 09:48:09 +0000 (+0000)
Subject: crimson/osd/object_context_loader: get_or_load to support atomicity
X-Git-Tag: v20.0.0~1802^2
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=1675ce8c1b5347ad13b65389686cd45853a4149e;p=ceph.git

crimson/osd/object_context_loader: get_or_load to support atomicity

make use of try_lock in order to support atomicity when called in
ObjectContext::_with_lock()

Co-authored-by: Yingxin Cheng <yingxin.cheng@intel.com>
Signed-off-by: Matan Breizman <mbreizma@redhat.com>
---

diff --git a/src/crimson/osd/object_context_loader.cc b/src/crimson/osd/object_context_loader.cc
index de147f2ee0e05..3f3f78964c439 100644
--- a/src/crimson/osd/object_context_loader.cc
+++ b/src/crimson/osd/object_context_loader.cc
@@ -150,20 +150,46 @@ using crimson::common::local_conf;
              dpp, obc->get_oid(),
              obc->fully_loaded,
              obc->invalidated_by_interval_change);
-    return interruptor::with_lock(obc->loading_mutex,
-    [this, obc, existed, FNAME] {
-      if (existed) {
-        ceph_assert(obc->is_valid() && obc->is_loaded());
-        DEBUGDPP("cache hit on {}", dpp, obc->get_oid());
-        return load_obc_iertr::make_ready_future<ObjectContextRef>(obc);
-      } else {
-        DEBUGDPP("cache miss on {}", dpp, obc->get_oid());
-        return obc->template with_promoted_lock<State, IOInterruptCondition>(
-          [obc, this] {
-          return load_obc(obc);
-        });
-      }
-    });
+    if (existed) {
+      // obc is already loaded - avoid loading_mutex usage
+      DEBUGDPP("cache hit on {}", dpp, obc->get_oid());
+      return get_obc(obc, existed);
+    }
+    // See ObjectContext::_with_lock(),
+    // this function must be able to support atomicity before
+    // acquiring the lock
+    if (obc->loading_mutex.try_lock()) {
+      return _get_or_load_obc<State>(obc, existed
+      ).finally([obc]{
+        obc->loading_mutex.unlock();
+      });
+    } else {
+      return interruptor::with_lock(obc->loading_mutex,
+      [this, obc, existed, FNAME] {
+        // Previous user already loaded the obc
+        DEBUGDPP("{} finished waiting for loader, cache hit on {}",
+                 dpp, FNAME, obc->get_oid());
+        return get_obc(obc, existed);
+      });
+    }
+  }
+
+  template<RWState::State State>
+  ObjectContextLoader::load_obc_iertr::future<ObjectContextRef>
+  ObjectContextLoader::_get_or_load_obc(ObjectContextRef obc,
+                                        bool existed)
+  {
+    LOG_PREFIX(ObjectContextLoader::_get_or_load_obc);
+    if (existed) {
+      DEBUGDPP("cache hit on {}", dpp, obc->get_oid());
+      return get_obc(obc, existed);
+    } else {
+      DEBUGDPP("cache miss on {}", dpp, obc->get_oid());
+      return obc->template with_promoted_lock<State, IOInterruptCondition>(
+        [obc, this] {
+        return load_obc(obc);
+      });
+    }
   }
 
   ObjectContextLoader::load_obc_iertr::future<>
diff --git a/src/crimson/osd/object_context_loader.h b/src/crimson/osd/object_context_loader.h
index b51e10caf77e8..80cea787630d6 100644
--- a/src/crimson/osd/object_context_loader.h
+++ b/src/crimson/osd/object_context_loader.h
@@ -80,6 +80,18 @@ private:
   get_or_load_obc(ObjectContextRef obc,
                   bool existed);
 
+  template<RWState::State State>
+  load_obc_iertr::future<ObjectContextRef>
+  _get_or_load_obc(ObjectContextRef obc,
+                  bool existed);
+
+  static inline load_obc_iertr::future<ObjectContextRef>
+  get_obc(ObjectContextRef obc,
+          bool existed) {
+    ceph_assert(existed && obc->is_valid() && obc->is_loaded());
+    return load_obc_iertr::make_ready_future<ObjectContextRef>(obc);
+  }
+
   load_obc_iertr::future<ObjectContextRef>
   load_obc(ObjectContextRef obc);
 };