Merge pull request ceph#57977 from athanatos/sjust/wip-66308-obc-locking

crimson: simplify obc loading by locking excl for load and demoting to needed lock

Reviewed-by: Yingxin Cheng <[email protected]>
Reviewed-by: Matan Breizman <[email protected]>

Author: athanatos

src/crimson/common/tri_mutex.cc

@@ -8,8 +8,7 @@
 SET_SUBSYS(osd);
 //TODO: SET_SUBSYS(crimson_tri_mutex);
 
-std::optional<seastar::future<>>
-read_lock::lock()
+seastar::future<> read_lock::lock()
 {
   return static_cast<tri_mutex*>(this)->lock_for_read();
 }
@@ -19,8 +18,7 @@ void read_lock::unlock()
   static_cast<tri_mutex*>(this)->unlock_for_read();
 }
 
-std::optional<seastar::future<>>
-write_lock::lock()
+seastar::future<> write_lock::lock()
 {
   return static_cast<tri_mutex*>(this)->lock_for_write();
 }
@@ -30,8 +28,7 @@ void write_lock::unlock()
   static_cast<tri_mutex*>(this)->unlock_for_write();
 }
 
-std::optional<seastar::future<>>
-excl_lock::lock()
+seastar::future<> excl_lock::lock()
 {
   return static_cast<tri_mutex*>(this)->lock_for_excl();
 }
@@ -41,44 +38,23 @@ void excl_lock::unlock()
   static_cast<tri_mutex*>(this)->unlock_for_excl();
 }
 
-void excl_lock_from_read::lock()
-{
-  static_cast<tri_mutex*>(this)->promote_from_read();
-}
-
-void excl_lock_from_read::unlock()
-{
-  static_cast<tri_mutex*>(this)->demote_to_read();
-}
-
-void excl_lock_from_write::lock()
-{
-  static_cast<tri_mutex*>(this)->promote_from_write();
-}
-
-void excl_lock_from_write::unlock()
-{
-  static_cast<tri_mutex*>(this)->demote_to_write();
-}
-
 tri_mutex::~tri_mutex()
 {
   LOG_PREFIX(tri_mutex::~tri_mutex());
   DEBUGDPP("", *this);
   assert(!is_acquired());
 }
 
-std::optional<seastar::future<>>
-tri_mutex::lock_for_read()
+seastar::future<> tri_mutex::lock_for_read()
 {
   LOG_PREFIX(tri_mutex::lock_for_read());
   DEBUGDPP("", *this);
   if (try_lock_for_read()) {
     DEBUGDPP("lock_for_read successfully", *this);
-    return std::nullopt;
+    return seastar::now();
   }
   DEBUGDPP("can't lock_for_read, adding to waiters", *this);
-  waiters.emplace_back(seastar::promise<>(), type_t::read, name);
+  waiters.emplace_back(seastar::promise<>(), type_t::read);
   return waiters.back().pr.get_future();
 }
 
@@ -103,15 +79,6 @@ void tri_mutex::unlock_for_read()
   }
 }
 
-void tri_mutex::promote_from_read()
-{
-  LOG_PREFIX(tri_mutex::promote_from_read());
-  DEBUGDPP("", *this);
-  assert(readers == 1);
-  --readers;
-  exclusively_used = true;
-}
-
 void tri_mutex::demote_to_read()
 {
   LOG_PREFIX(tri_mutex::demote_to_read());
@@ -121,17 +88,16 @@ void tri_mutex::demote_to_read()
   ++readers;
 }
 
-std::optional<seastar::future<>>
-tri_mutex::lock_for_write()
+seastar::future<> tri_mutex::lock_for_write()
 {
   LOG_PREFIX(tri_mutex::lock_for_write());
   DEBUGDPP("", *this);
   if (try_lock_for_write()) {
     DEBUGDPP("lock_for_write successfully", *this);
-    return std::nullopt;
+    return seastar::now();
   }
   DEBUGDPP("can't lock_for_write, adding to waiters", *this);
-  waiters.emplace_back(seastar::promise<>(), type_t::write, name);
+  waiters.emplace_back(seastar::promise<>(), type_t::write);
   return waiters.back().pr.get_future();
 }
 
@@ -156,15 +122,6 @@ void tri_mutex::unlock_for_write()
   }
 }
 
-void tri_mutex::promote_from_write()
-{
-  LOG_PREFIX(tri_mutex::promote_from_write());
-  DEBUGDPP("", *this);
-  assert(writers == 1);
-  --writers;
-  exclusively_used = true;
-}
-
 void tri_mutex::demote_to_write()
 {
   LOG_PREFIX(tri_mutex::demote_to_write());
@@ -175,17 +132,16 @@ void tri_mutex::demote_to_write()
 }
 
 // for exclusive users
-std::optional<seastar::future<>>
-tri_mutex::lock_for_excl()
+seastar::future<> tri_mutex::lock_for_excl()
 {
   LOG_PREFIX(tri_mutex::lock_for_excl());
   DEBUGDPP("", *this);
   if (try_lock_for_excl()) {
     DEBUGDPP("lock_for_excl, successfully", *this);
-    return std::nullopt;
+    return seastar::now();
   }
   DEBUGDPP("can't lock_for_excl, adding to waiters", *this);
-  waiters.emplace_back(seastar::promise<>(), type_t::exclusive, name);
+  waiters.emplace_back(seastar::promise<>(), type_t::exclusive);
   return waiters.back().pr.get_future();
 }
 
@@ -254,7 +210,7 @@ void tri_mutex::wake()
     default:
       assert(0);
     }
-    DEBUGDPP("waking up {}", *this, waiter.waiter_name);
+    DEBUGDPP("waking up", *this);
     waiter.pr.set_value();
     waiters.pop_front();
   }

src/crimson/common/tri_mutex.h

@@ -3,41 +3,27 @@
 
 #pragma once
 
-#include <optional>
-
 #include <seastar/core/future.hh>
 #include <seastar/core/circular_buffer.hh>
+
+#include "common/hobject.h"
 #include "crimson/common/log.h"
 
 class read_lock {
 public:
-  std::optional<seastar::future<>> lock();
+  seastar::future<> lock();
   void unlock();
 };
 
 class write_lock {
 public:
-  std::optional<seastar::future<>> lock();
+  seastar::future<> lock();
   void unlock();
 };
 
 class excl_lock {
 public:
-  std::optional<seastar::future<>> lock();
-  void unlock();
-};
-
-// promote from read to excl
-class excl_lock_from_read {
-public:
-  void lock();
-  void unlock();
-};
-
-// promote from write to excl
-class excl_lock_from_write {
-public:
-  void lock();
+  seastar::future<> lock();
   void unlock();
 };
 
@@ -54,21 +40,16 @@ class excl_lock_from_write {
 /// - 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
 {
 public:
   tri_mutex() = default;
 #ifdef NDEBUG
-  tri_mutex(const std::string obj_name) : name() {}
+  tri_mutex(const hobject_t &obj_name) : name() {}
 #else
-  tri_mutex(const std::string obj_name) : name(obj_name) {}
+  tri_mutex(const hobject_t &obj_name) : name(obj_name) {}
 #endif
   ~tri_mutex();
 
@@ -81,35 +62,27 @@ class tri_mutex : private read_lock,
   excl_lock& for_excl() {
     return *this;
   }
-  excl_lock_from_read& excl_from_read() {
-    return *this;
-  }
-  excl_lock_from_write& excl_from_write() {
-    return *this;
-  }
 
   // for shared readers
-  std::optional<seastar::future<>> lock_for_read();
+  seastar::future<> lock_for_read();
   bool try_lock_for_read() noexcept;
   void unlock_for_read();
-  void promote_from_read();
   void demote_to_read();
   unsigned get_readers() const {
     return readers;
   }
 
   // for shared writers
-  std::optional<seastar::future<>> lock_for_write();
+  seastar::future<> lock_for_write();
   bool try_lock_for_write() noexcept;
   void unlock_for_write();
-  void promote_from_write();
   void demote_to_write();
   unsigned get_writers() const {
     return writers;
   }
 
   // for exclusive users
-  std::optional<seastar::future<>> lock_for_excl();
+  seastar::future<> lock_for_excl();
   bool try_lock_for_excl() noexcept;
   void unlock_for_excl();
   bool is_excl_acquired() const {
@@ -128,7 +101,7 @@ class tri_mutex : private read_lock,
     }
   }
 
-  std::string_view get_name() const{
+  const hobject_t &get_name() const{
     return name;
   }
 
@@ -144,21 +117,17 @@ class tri_mutex : private read_lock,
     none,
   };
   struct waiter_t {
-    waiter_t(seastar::promise<>&& pr, type_t type, std::string_view waiter_name)
+    waiter_t(seastar::promise<>&& pr, type_t type)
       : pr(std::move(pr)), type(type)
     {}
     seastar::promise<> pr;
     type_t type;
-    std::string_view waiter_name;
   };
   seastar::circular_buffer<waiter_t> waiters;
-  const std::string name;
+  const hobject_t name;
   friend class read_lock;
   friend class write_lock;
   friend class excl_lock;
-  friend class excl_lock_from_read;
-  friend class excl_lock_from_write;
-  friend class excl_lock_from_excl;
   friend std::ostream& operator<<(std::ostream &lhs, const tri_mutex &rhs);
 };