os/bluestore/bluefs: Add tags and comments for locks

Add function tags and comments related to locks.
Fixed locking graph documentation.

Signed-off-by: Adam Kupczyk <akupczyk@redhat.com>

diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc
index 24088bce83719a58fe689baf8944efbba6612527..7bd6e2ff658c9e38fde63c251f4cbbc344f3fa8e 100644 (file)
--- a/src/os/bluestore/BlueFS.cc
+++ b/src/os/bluestore/BlueFS.cc
@@ -2291,13 +2291,13 @@ uint64_t BlueFS::_estimate_log_size_N()
   return round_up_to(size, super.block_size);
 }
 
-void BlueFS::compact_log()
+void BlueFS::compact_log()/*_LNF_LD_NF_D*/
 {
   if (!cct->_conf->bluefs_replay_recovery_disable_compact) {
     if (cct->_conf->bluefs_compact_log_sync) {
       _compact_log_sync_LNF_LD();
     } else {
-      _compact_log_async_LD_NF_D();
+      _compact_log_async_LD_LNF_D();
     }
   }
 }
@@ -2551,7 +2551,7 @@ void BlueFS::_rewrite_log_and_layout_sync_LNF_LD(bool allocate_with_fallback,
  * 8. Release the old log space.  Clean up.
  */
 
-void BlueFS::_compact_log_async_LD_NF_D() //also locks FW for new_writer
+void BlueFS::_compact_log_async_LD_LNF_D() //also locks FW for new_writer
 {
   dout(10) << __func__ << dendl;
   // only one compaction allowed at one time
@@ -3080,7 +3080,8 @@ int BlueFS::_signal_dirty_to_log_D(FileWriter *h)
   return 0;
 }
 
-void BlueFS::flush_range(FileWriter *h, uint64_t offset, uint64_t length) {
+void BlueFS::flush_range(FileWriter *h, uint64_t offset, uint64_t length)/*_WF*/
+{
   std::unique_lock hl(h->lock);
   _flush_range_F(h, offset, length);
 }
@@ -3255,7 +3256,7 @@ void BlueFS::_wait_for_aio(FileWriter *h)
 }
 #endif
 
-void BlueFS::append_try_flush(FileWriter *h, const char* buf, size_t len)
+void BlueFS::append_try_flush(FileWriter *h, const char* buf, size_t len)/*_WF_LNF_NF_LD_D*/
 {
   bool flushed_sum = false;
   {
@@ -3287,7 +3288,7 @@ void BlueFS::append_try_flush(FileWriter *h, const char* buf, size_t len)
   }
 }
 
-void BlueFS::flush(FileWriter *h, bool force)
+void BlueFS::flush(FileWriter *h, bool force)/*_WF_LNF_NF_LD_D*/
 {
   bool flushed = false;
   int r;
@@ -3352,7 +3353,7 @@ int BlueFS::_flush_special(FileWriter *h)
   return _flush_data(h, offset, length, false);
 }
 
-int BlueFS::truncate(FileWriter *h, uint64_t offset)
+int BlueFS::truncate(FileWriter *h, uint64_t offset)/*_WF_L*/
 {
   std::lock_guard hl(h->lock);
   dout(10) << __func__ << " 0x" << std::hex << offset << std::dec
@@ -3394,7 +3395,7 @@ int BlueFS::truncate(FileWriter *h, uint64_t offset)
   return 0;
 }
 
-int BlueFS::fsync(FileWriter *h)
+int BlueFS::fsync(FileWriter *h)/*_WF_WD_WLD_WLNF_WNF*/
 {
   std::unique_lock hl(h->lock);
   uint64_t old_dirty_seq = 0;
@@ -3576,7 +3577,7 @@ int BlueFS::_allocate(uint8_t id, uint64_t len,
   return 0;
 }
 
-int BlueFS::preallocate(FileRef f, uint64_t off, uint64_t len)
+int BlueFS::preallocate(FileRef f, uint64_t off, uint64_t len)/*_LF*/
 {
   std::lock_guard ll(log.lock);
   std::lock_guard fl(f->lock);
@@ -3604,7 +3605,7 @@ int BlueFS::preallocate(FileRef f, uint64_t off, uint64_t len)
   return 0;
 }
 
-void BlueFS::sync_metadata(bool avoid_compact)
+void BlueFS::sync_metadata(bool avoid_compact)/*_LNF_NF_LD_D*/
 {
   bool can_skip_flush;
   {
@@ -3636,7 +3637,7 @@ void BlueFS::_maybe_compact_log_LNF_NF_LD_D()
     if (cct->_conf->bluefs_compact_log_sync) {
       _compact_log_sync_LNF_LD();
     } else {
-      _compact_log_async_LD_NF_D();
+      _compact_log_async_LD_LNF_D();
     }
   }
 }
@@ -3645,7 +3646,7 @@ int BlueFS::open_for_write(
   std::string_view dirname,
   std::string_view filename,
   FileWriter **h,
-  bool overwrite)
+  bool overwrite)/*_N_LD*/
 {
   FileRef file;
   bool create = false;
@@ -3799,7 +3800,7 @@ int BlueFS::open_for_read(
   std::string_view dirname,
   std::string_view filename,
   FileReader **h,
-  bool random)
+  bool random)/*_N*/
 {
   std::lock_guard nl(nodes.lock);
   dout(10) << __func__ << " " << dirname << "/" << filename
@@ -3828,7 +3829,7 @@ int BlueFS::open_for_read(
 
 int BlueFS::rename(
   std::string_view old_dirname, std::string_view old_filename,
-  std::string_view new_dirname, std::string_view new_filename)
+  std::string_view new_dirname, std::string_view new_filename)/*_LND*/
 {
   std::lock_guard ll(log.lock);
   std::lock_guard nl(nodes.lock);
@@ -3876,7 +3877,7 @@ int BlueFS::rename(
   return 0;
 }
 
-int BlueFS::mkdir(std::string_view dirname)
+int BlueFS::mkdir(std::string_view dirname)/*_LN*/
 {
   std::lock_guard ll(log.lock);
   std::lock_guard nl(nodes.lock);
@@ -3891,7 +3892,7 @@ int BlueFS::mkdir(std::string_view dirname)
   return 0;
 }
 
-int BlueFS::rmdir(std::string_view dirname)
+int BlueFS::rmdir(std::string_view dirname)/*_LN*/
 {
   std::lock_guard ll(log.lock);
   std::lock_guard nl(nodes.lock);
@@ -3911,7 +3912,7 @@ int BlueFS::rmdir(std::string_view dirname)
   return 0;
 }
 
-bool BlueFS::dir_exists(std::string_view dirname)
+bool BlueFS::dir_exists(std::string_view dirname)/*_N*/
 {
   std::lock_guard nl(nodes.lock);
   map<string,DirRef>::iterator p = nodes.dir_map.find(dirname);
@@ -3921,7 +3922,7 @@ bool BlueFS::dir_exists(std::string_view dirname)
 }
 
 int BlueFS::stat(std::string_view dirname, std::string_view filename,
-                uint64_t *size, utime_t *mtime)
+                uint64_t *size, utime_t *mtime)/*_N*/
 {
   std::lock_guard nl(nodes.lock);
   dout(10) << __func__ << " " << dirname << "/" << filename << dendl;
@@ -3949,7 +3950,7 @@ int BlueFS::stat(std::string_view dirname, std::string_view filename,
 }
 
 int BlueFS::lock_file(std::string_view dirname, std::string_view filename,
-                     FileLock **plock)
+                     FileLock **plock)/*_LN*/
 {
   std::lock_guard ll(log.lock);
   std::lock_guard nl(nodes.lock);
@@ -3989,7 +3990,7 @@ int BlueFS::lock_file(std::string_view dirname, std::string_view filename,
   return 0;
 }
 
-int BlueFS::unlock_file(FileLock *fl)
+int BlueFS::unlock_file(FileLock *fl)/*_N*/
 {
   std::lock_guard nl(nodes.lock);
   dout(10) << __func__ << " " << fl << " on " << fl->file->fnode << dendl;
@@ -3999,7 +4000,7 @@ int BlueFS::unlock_file(FileLock *fl)
   return 0;
 }
 
-int BlueFS::readdir(std::string_view dirname, vector<string> *ls)
+int BlueFS::readdir(std::string_view dirname, vector<string> *ls)/*_N*/
 {
   // dirname may contain a trailing /
   if (!dirname.empty() && dirname.back() == '/') {
@@ -4031,7 +4032,7 @@ int BlueFS::readdir(std::string_view dirname, vector<string> *ls)
   return 0;
 }
 
-int BlueFS::unlink(std::string_view dirname, std::string_view filename)
+int BlueFS::unlink(std::string_view dirname, std::string_view filename)/*_LND*/
 {
   std::lock_guard ll(log.lock);
   std::lock_guard nl(nodes.lock);

diff --git a/src/os/bluestore/BlueFS.h b/src/os/bluestore/BlueFS.h
index ef17b2003187360433626354a454666e831fedfe..c942327bf805230c3c5737011864f655d06458d0 100644 (file)
--- a/src/os/bluestore/BlueFS.h
+++ b/src/os/bluestore/BlueFS.h
@@ -458,7 +458,7 @@ private:
                                           uint64_t capture_before_seq);
 
   void _compact_log_sync_LNF_LD();
-  void _compact_log_async_LD_NF_D();
+  void _compact_log_async_LD_LNF_D();
 
   void _rewrite_log_and_layout_sync_LNF_LD(bool allocate_with_fallback,
                                    int super_dev,
@@ -590,8 +590,6 @@ public:
 
   /// sync any uncommitted state to disk
   void sync_metadata(bool avoid_compact);
-  /// test and compact log, if necessary
-  void _maybe_compact_log_LNF_NF_LD_D();
 
   void set_volume_selector(BlueFSVolumeSelector* s) {
     vselector.reset(s);
@@ -635,11 +633,6 @@ public:
   void invalidate_cache(FileRef f, uint64_t offset, uint64_t len);
   int preallocate(FileRef f, uint64_t offset, uint64_t len);
   int truncate(FileWriter *h, uint64_t offset);
-  int _do_replay_recovery_read(FileReader *log,
-                              size_t log_pos,
-                              size_t read_offset,
-                              size_t read_len,
-                              bufferlist* bl);
 
   size_t probe_alloc_avail(int dev, uint64_t alloc_size);
 
@@ -660,6 +653,14 @@ private:
   int _bdev_read(uint8_t ndev, uint64_t off, uint64_t len,
     ceph::buffer::list* pbl, IOContext* ioc, bool buffered);
   int _bdev_read_random(uint8_t ndev, uint64_t off, uint64_t len, char* buf, bool buffered);
+
+  /// test and compact log, if necessary
+  void _maybe_compact_log_LNF_NF_LD_D();
+  int _do_replay_recovery_read(FileReader *log,
+                              size_t log_pos,
+                              size_t read_offset,
+                              size_t read_len,
+                              bufferlist* bl);
 };
 
 class OriginalVolumeSelector : public BlueFSVolumeSelector {
@@ -714,15 +715,15 @@ public:
  * Vertices - Locks. Edges (directed) - locking progression.
  * Edge A->B exist if last taken lock was A and next taken lock is B.
  * 
- * Column represents last lock taken.
- * Row represents next lock taken.
+ * Row represents last lock taken.
+ * Column represents next lock taken.
  *
- *     >        | W | L | D | N | F
+ *     >        | W | L | N | D | F
  * -------------|---|---|---|---|---
- * FileWriter W |   | > | > |   | >
+ * FileWriter W |   | > | > | > | >
  * log        L |       | > | > | >
+ * nodes      N |           | > | >
  * dirty      D |           |   | >
- * nodes      N |               | >
  * File       F |
  * 
  * Claim: Deadlock is possible IFF graph contains cycles.