Merge pull request ceph#59301 from xxhdx1985126/wip-67604

crimson/common/tri_mutex: also wake up waiters when demoting

Reviewed-by: Samuel Just <[email protected]>
Reviewed-by: Yingxin Cheng <[email protected]>

Author: Matan-B

src/crimson/common/tri_mutex.cc

@@ -75,7 +75,8 @@ void tri_mutex::unlock_for_read()
   DEBUGDPP("", *this);
   assert(readers > 0);
   if (--readers == 0) {
-    wake();
+    assert(!readers && !writers && !exclusively_used);
+    wake(type_t::none);
   }
 }
 
@@ -85,7 +86,9 @@ void tri_mutex::demote_to_read()
   DEBUGDPP("", *this);
   assert(exclusively_used);
   exclusively_used = false;
+  assert(!readers && !writers && !exclusively_used);
   ++readers;
+  wake(type_t::read);
 }
 
 seastar::future<> tri_mutex::lock_for_write()
@@ -118,7 +121,8 @@ void tri_mutex::unlock_for_write()
   DEBUGDPP("", *this);
   assert(writers > 0);
   if (--writers == 0) {
-    wake();
+    assert(!readers && !writers && !exclusively_used);
+    wake(type_t::none);
   }
 }
 
@@ -128,7 +132,9 @@ void tri_mutex::demote_to_write()
   DEBUGDPP("", *this);
   assert(exclusively_used);
   exclusively_used = false;
+  assert(!readers && !writers && !exclusively_used);
   ++writers;
+  wake(type_t::write);
 }
 
 // for exclusive users
@@ -163,7 +169,8 @@ void tri_mutex::unlock_for_excl()
   DEBUGDPP("", *this);
   assert(exclusively_used);
   exclusively_used = false;
-  wake();
+  assert(!readers && !writers && !exclusively_used);
+  wake(type_t::none);
 }
 
 bool tri_mutex::is_acquired() const
@@ -181,23 +188,22 @@ bool tri_mutex::is_acquired() const
   }
 }
 
-void tri_mutex::wake()
+void tri_mutex::wake(type_t type_to_wake)
 {
   LOG_PREFIX(tri_mutex::wake());
   DEBUGDPP("", *this);
-  assert(!readers && !writers && !exclusively_used);
-  type_t type = type_t::none;
+  assert(type_to_wake != type_t::exclusive);
   while (!waiters.empty()) {
     auto& waiter = waiters.front();
-    if (type == type_t::exclusive) {
+    if (type_to_wake == type_t::exclusive) {
       break;
-    } if (type == type_t::none) {
-      type = waiter.type;
-    } else if (type != waiter.type) {
+    } if (type_to_wake == type_t::none) {
+      type_to_wake = waiter.type;
+    } else if (type_to_wake != waiter.type) {
       // to be woken in the next batch
       break;
     }
-    switch (type) {
+    switch (type_to_wake) {
     case type_t::read:
       ++readers;
       break;

src/crimson/common/tri_mutex.h

@@ -106,7 +106,6 @@ class tri_mutex : private read_lock,
   }
 
 private:
-  void wake();
   unsigned readers = 0;
   unsigned writers = 0;
   bool exclusively_used = false;
@@ -116,6 +115,7 @@ class tri_mutex : private read_lock,
     exclusive,
     none,
   };
+  void wake(type_t);
   struct waiter_t {
     waiter_t(seastar::promise<>&& pr, type_t type)
       : pr(std::move(pr)), type(type)

src/crimson/osd/osd_operations/client_request.cc

@@ -308,9 +308,9 @@ ClientRequest::recover_missing_snaps(
     if (oid) {
       auto unfound = co_await do_recover_missing(pg, *oid, m->get_reqid());
       if (unfound) {
-        DEBUGDPP("{} unfound, hang it for now", *pg, m->get_hobj().get_head());
+        DEBUGDPP("{} unfound, hang it for now", *pg, *oid);
         co_await interruptor::make_interruptible(
-          pg->get_recovery_backend()->add_unfound(m->get_hobj().get_head()));
+          pg->get_recovery_backend()->add_unfound(*oid));
       }
     }
   }