crimson/common/tri_mutex: make promotion atomic with func

Specifically, make promotion atomic with load-obc to fix
assert(readers/writers == 1) failures.

Fixes: https://tracker.ceph.com/issues/65451
Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>

diff --git a/src/crimson/common/tri_mutex.cc b/src/crimson/common/tri_mutex.cc
index 172406fd8155339a8fdcd1037f90e5a012642e07..f79d25668854376eb4f04afd633601228b6ae6d0 100644 (file)
--- a/src/crimson/common/tri_mutex.cc
+++ b/src/crimson/common/tri_mutex.cc
@@ -35,10 +35,9 @@ void excl_lock::unlock()
   static_cast<tri_mutex*>(this)->unlock_for_excl();
 }
 
-seastar::future<> excl_lock_from_read::lock()
+void excl_lock_from_read::lock()
 {
   static_cast<tri_mutex*>(this)->promote_from_read();
-  return seastar::now();
 }
 
 void excl_lock_from_read::unlock()
@@ -46,10 +45,9 @@ void excl_lock_from_read::unlock()
   static_cast<tri_mutex*>(this)->demote_to_read();
 }
 
-seastar::future<> excl_lock_from_write::lock()
+void excl_lock_from_write::lock()
 {
   static_cast<tri_mutex*>(this)->promote_from_write();
-  return seastar::now();
 }
 
 void excl_lock_from_write::unlock()
@@ -57,15 +55,6 @@ void excl_lock_from_write::unlock()
   static_cast<tri_mutex*>(this)->demote_to_write();
 }
 
-seastar::future<> excl_lock_from_excl::lock()
-{
-  return seastar::now();
-}
-
-void excl_lock_from_excl::unlock()
-{
-}
-
 tri_mutex::~tri_mutex()
 {
   assert(!is_acquired());

diff --git a/src/crimson/common/tri_mutex.h b/src/crimson/common/tri_mutex.h
index a75895fa8decc48415996e0df4c869b6a674f06f..d1c215be27e206539c0bd55b7e8238ce84b0888e 100644 (file)
--- a/src/crimson/common/tri_mutex.h
+++ b/src/crimson/common/tri_mutex.h
@@ -27,21 +27,14 @@ public:
 // promote from read to excl
 class excl_lock_from_read {
 public:
-  seastar::future<> lock();
+  void lock();
   void unlock();
 };
 
 // promote from write to excl
 class excl_lock_from_write {
 public:
-  seastar::future<> lock();
-  void unlock();
-};
-
-// promote from excl to excl
-class excl_lock_from_excl {
-public:
-  seastar::future<> lock();
+  void lock();
   void unlock();
 };
 
@@ -58,12 +51,14 @@ public:
 /// - readers
 /// - writers
 /// - exclusive users
+///
+/// For lock promotion, a read or a write lock is only allowed to be promoted
+/// atomically upon the first locking.
 class tri_mutex : private read_lock,
                           write_lock,
                           excl_lock,
                           excl_lock_from_read,
-                          excl_lock_from_write,
-                          excl_lock_from_excl
+                          excl_lock_from_write
 {
 public:
   tri_mutex() = default;
@@ -84,9 +79,6 @@ public:
   excl_lock_from_write& excl_from_write() {
     return *this;
   }
-  excl_lock_from_excl& excl_from_excl() {
-    return *this;
-  }
 
   // for shared readers
   seastar::future<> lock_for_read();

diff --git a/src/crimson/osd/object_context.h b/src/crimson/osd/object_context.h
index f38e52f009ce44ce064b1b4179e334e9cd3e8743..dc10223de1e60a374a4ff209713e6aa08360a87b 100644 (file)
--- a/src/crimson/osd/object_context.h
+++ b/src/crimson/osd/object_context.h
@@ -121,7 +121,7 @@ private:
   bool recovery_read_marker = false;
 
   template <typename Lock, typename Func>
-  auto _with_lock(Lock&& lock, Func&& func) {
+  auto _with_lock(Lock& lock, Func&& func) {
     Ref obc = this;
     return lock.lock().then([&lock, func = std::forward<Func>(func), obc]() mutable {
       return seastar::futurize_invoke(func).finally([&lock, obc] {
@@ -130,6 +130,15 @@ private:
     });
   }
 
+  template <typename Lock, typename Func>
+  auto _with_promoted_lock(Lock& lock, Func&& func) {
+    Ref obc = this;
+    lock.lock();
+    return seastar::futurize_invoke(func).finally([&lock, obc] {
+      lock.unlock();
+    });
+  }
+
   boost::intrusive::list_member_hook<> obc_accessing_hook;
   uint64_t list_link_cnt = 0;
   bool fully_loaded = false;
@@ -196,11 +205,11 @@ public:
       auto wrapper = ::crimson::interruptible::interruptor<InterruptCond>::wrap_function(std::forward<Func>(func));
       switch (Type) {
       case RWState::RWWRITE:
-       return _with_lock(lock.excl_from_write(), std::move(wrapper));
+       return _with_promoted_lock(lock.excl_from_write(), std::move(wrapper));
       case RWState::RWREAD:
-       return _with_lock(lock.excl_from_read(), std::move(wrapper));
+       return _with_promoted_lock(lock.excl_from_read(), std::move(wrapper));
       case RWState::RWEXCL:
-       return _with_lock(lock.excl_from_excl(), std::move(wrapper));
+       return seastar::futurize_invoke(std::move(wrapper));
       case RWState::RWNONE:
        return _with_lock(lock.for_excl(), std::move(wrapper));
        default:
@@ -209,11 +218,11 @@ public:
     } else {
       switch (Type) {
       case RWState::RWWRITE:
-       return _with_lock(lock.excl_from_write(), std::forward<Func>(func));
+       return _with_promoted_lock(lock.excl_from_write(), std::forward<Func>(func));
       case RWState::RWREAD:
-       return _with_lock(lock.excl_from_read(), std::forward<Func>(func));
+       return _with_promoted_lock(lock.excl_from_read(), std::forward<Func>(func));
       case RWState::RWEXCL:
-       return _with_lock(lock.excl_from_excl(), std::forward<Func>(func));
+       return seastar::futurize_invoke(std::forward<Func>(func));
       case RWState::RWNONE:
        return _with_lock(lock.for_excl(), std::forward<Func>(func));
        default: