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 <[email protected]>

Author: cyx1231st

src/crimson/common/tri_mutex.cc

@@ -35,37 +35,26 @@ 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()
 {
   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()
 {
   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());

src/crimson/common/tri_mutex.h

@@ -27,21 +27,14 @@ class excl_lock {
 // 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 @@ class excl_lock_from_excl {
 /// - 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 @@ class tri_mutex : private read_lock,
   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();

src/crimson/osd/object_context.h

@@ -121,7 +121,7 @@ class ObjectContext : public ceph::common::intrusive_lru_base<
   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 @@ class ObjectContext : public ceph::common::intrusive_lru_base<
     });
   }
 
+  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 @@ class ObjectContext : public ceph::common::intrusive_lru_base<
       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 @@ class ObjectContext : public ceph::common::intrusive_lru_base<
     } 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: