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>

diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc
index 43245987f0b37e9547de473e57ed51a88e4208b0..610f9921fa2644d26209962046e1e8835c63059c 100644 (file)
--- a/src/os/bluestore/BlueFS.cc
+++ b/src/os/bluestore/BlueFS.cc
@@ -2623,16 +2623,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
 
@@ -3374,9 +3374,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 5b052d7652aeef25502c7ce86dd44a6faa516d9f..abdd117ad2376f5b98a0e49c944aa3a27b25371f 100644 (file)
--- a/src/os/bluestore/BlueFS.h
+++ b/src/os/bluestore/BlueFS.h
@@ -718,13 +718,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.
  */