os/bluestore/bluefs: Fix false collision with lockdep module

Usually sequence of locking is 1) FileWriter 2) File.
In _compact_log_async_LD_NF_D it was in reversed order.
No real deadlock was possible, but lockdep complained.

Bonus: Improved lock dependency graph.

Fixes: https://tracker.ceph.com/issues/52939
Signed-off-by: Adam Kupczyk <akupczyk@redhat.com>
(cherry picked from commit 7b7945d6117eb7502729c5dd0b5d383d8bc73f10)

diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc
index e961522331b010d29f165ec362481f6fc8bd5653..cab97907badaed00c418eddae1e8ea468da414b3 100644 (file)
--- a/src/os/bluestore/BlueFS.cc
+++ b/src/os/bluestore/BlueFS.cc
@@ -2511,16 +2511,16 @@ void BlueFS::_compact_log_async_LD_NF_D() //also locks FW for new_writer
   new_log_writer = _create_writer(new_log);
 
   new_log_writer->append(bl);
-  new_log->lock.lock();
   new_log_writer->lock.lock();
+  new_log->lock.lock();
   // 3. flush
   r = _flush_special(new_log_writer);
   ceph_assert(r == 0);
 
   // 4. wait
   _flush_bdev(new_log_writer);
-  new_log_writer->lock.unlock();
   new_log->lock.unlock();
+  new_log_writer->lock.unlock();
   // 5. update our log fnode
   // discard first old_log_jump_to extents
 
@@ -3268,9 +3268,9 @@ int BlueFS::truncate(FileWriter *h, uint64_t offset)
 
 int BlueFS::fsync(FileWriter *h)
 {
-  std::unique_lock hl(h->lock);
   uint64_t old_dirty_seq = 0;
   {
+    std::unique_lock hl(h->lock);
     dout(10) << __func__ << " " << h << " " << h->file->fnode << dendl;
     int r = _flush_F(h, true);
     if (r < 0)

diff --git a/src/os/bluestore/BlueFS.h b/src/os/bluestore/BlueFS.h
index ab9c1747e01d441f68a51cf8abb7495a6cdfa1c4..65fad533c9888e908891c21aea117bd8d70d4624 100644 (file)
--- a/src/os/bluestore/BlueFS.h
+++ b/src/os/bluestore/BlueFS.h
@@ -698,13 +698,13 @@ public:
  * Column represents last lock taken.
  * Row represents next lock taken.
  *
- *     <        | L | D | N | F | W
+ *     >        | W | L | D | N | F
  * -------------|---|---|---|---|---
- * log        L |     <   <         
- * dirty      D |                   
- * nodes      N |             <     
- * File       F |                  
- * FileWriter W | <   <       <      
+ * FileWriter W |   | > | > |   | >
+ * log        L |       | > | > | >
+ * dirty      D |           |   | >
+ * nodes      N |               | >
+ * File       F |
  * 
  * Claim: Deadlock is possible IFF graph contains cycles.
  */