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 de147f2ee0e050d37fded09e5b50571ab1ed750d..3f3f78964c4390750c52babc76ea6d7d7a8fd075 100644 (file)
--- 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 b51e10caf77e8720cd39cb4a15824ba5dafb9615..80cea787630d6dd7690dc436befa57860cb1fdb9 100644 (file)
--- 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);
 };