os/bluestore/bluefs: Cleanup on pending_release variable

Moved pending_release to struct dirty {}.
Restructured BlueFS::open_for_write to modify pending_release under dirty.lock.
Now all pending_release modifications are under dirty.lock.

Signed-off-by: Adam Kupczyk <akupczyk@redhat.com>
(cherry picked from commit 2b36f27707ba6efb80eeebef487d7403a478a528)

diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc
index 93defb4c8721f898849751d3e275619ad7b7f8a3..8369f63e472d7233b7f671162c949d3f825f8d25 100644 (file)
--- a/src/os/bluestore/BlueFS.cc
+++ b/src/os/bluestore/BlueFS.cc
@@ -189,9 +189,9 @@ BlueFS::BlueFS(CephContext* cct)
     ioc(MAX_BDEV),
     block_reserved(MAX_BDEV),
     alloc(MAX_BDEV),
-    alloc_size(MAX_BDEV, 0),
-    pending_release(MAX_BDEV)
+    alloc_size(MAX_BDEV, 0)
 {
+  dirty.pending_release.resize(MAX_BDEV);
   discard_cb[BDEV_WAL] = wal_discard_cb;
   discard_cb[BDEV_DB] = db_discard_cb;
   discard_cb[BDEV_SLOW] = slow_discard_cb;
@@ -1904,7 +1904,7 @@ void BlueFS::_drop_link_D(FileRef file)
 
     std::lock_guard dl(dirty.lock);
     for (auto& r : file->fnode.extents) {
-      pending_release[r.bdev].insert(r.offset, r.length);
+      dirty.pending_release[r.bdev].insert(r.offset, r.length);
     }
     if (file->dirty_seq > dirty.seq_stable) {
       // retract request to serialize changes
@@ -2395,7 +2395,7 @@ void BlueFS::_rewrite_log_and_layout_sync_LN_LD(bool allocate_with_fallback,
   dout(10) << __func__ << " release old log extents " << old_fnode.extents << dendl;
   std::lock_guard dl(dirty.lock);
   for (auto& r : old_fnode.extents) {
-    pending_release[r.bdev].insert(r.offset, r.length);
+    dirty.pending_release[r.bdev].insert(r.offset, r.length);
   }
 }
 
@@ -2597,7 +2597,7 @@ void BlueFS::_compact_log_async_LD_NF_D() //also locks FW for new_writer
   {
     std::lock_guard dl(dirty.lock);
     for (auto& r : old_extents) {
-      pending_release[r.bdev].insert(r.offset, r.length);
+      dirty.pending_release[r.bdev].insert(r.offset, r.length);
     }
   }
 
@@ -2843,8 +2843,8 @@ int BlueFS::_flush_and_sync_log_LD(uint64_t want_seq)
   ceph_assert(want_seq == 0 || want_seq <= dirty.seq_live); // illegal to request seq that was not created yet
   uint64_t seq =_log_advance_seq();
   _consume_dirty(seq);
-  vector<interval_set<uint64_t>> to_release(pending_release.size());
-  to_release.swap(pending_release);
+  vector<interval_set<uint64_t>> to_release(dirty.pending_release.size());
+  to_release.swap(dirty.pending_release);
   dirty.lock.unlock();
 
   _flush_and_sync_log_core(available_runway);
@@ -2872,8 +2872,8 @@ int BlueFS::_flush_and_sync_log_jump_D(uint64_t jump_to,
   dirty.lock.lock();
   uint64_t seq =_log_advance_seq();
   _consume_dirty(seq);
-  vector<interval_set<uint64_t>> to_release(pending_release.size());
-  to_release.swap(pending_release);
+  vector<interval_set<uint64_t>> to_release(dirty.pending_release.size());
+  to_release.swap(dirty.pending_release);
   dirty.lock.unlock();
   _flush_and_sync_log_core(available_runway);
 
@@ -3537,6 +3537,11 @@ int BlueFS::open_for_write(
   FileWriter **h,
   bool overwrite)
 {
+  FileRef file;
+  bool create = false;
+  bool truncate = false;
+  mempool::bluefs::vector<bluefs_extent_t> pending_release_extents;
+  {
   std::unique_lock nl(nodes.lock);
   dout(10) << __func__ << " " << dirname << "/" << filename << dendl;
   map<string,DirRef>::iterator p = nodes.dir_map.find(dirname);
@@ -3550,9 +3555,6 @@ int BlueFS::open_for_write(
     dir = p->second;
   }
 
-  FileRef file;
-  bool create = false;
-  bool truncate = false;
   map<string,FileRef>::iterator q = dir->file_map.find(filename);
   if (q == dir->file_map.end()) {
     if (overwrite) {
@@ -3581,9 +3583,7 @@ int BlueFS::open_for_write(
               << " already exists, truncate + overwrite" << dendl;
       vselector->sub_usage(file->vselector_hint, file->fnode);
       file->fnode.size = 0;
-      for (auto& p : file->fnode.extents) {
-       pending_release[p.bdev].insert(p.offset, p.length);
-      }
+      pending_release_extents.swap(file->fnode.extents);
       truncate = true;
 
       file->fnode.clear_extents();
@@ -3600,12 +3600,17 @@ int BlueFS::open_for_write(
   dout(20) << __func__ << " mapping " << dirname << "/" << filename
           << " vsel_hint " << file->vselector_hint
           << dendl;
-  nl.unlock();
+  }
   {
     std::lock_guard ll(log.lock);
     log.t.op_file_update(file->fnode);
     if (create)
       log.t.op_dir_link(dirname, filename, file->fnode.ino);
+
+    std::lock_guard dl(dirty.lock);
+    for (auto& p : pending_release_extents) {
+      dirty.pending_release[p.bdev].insert(p.offset, p.length);
+    }
   }
   *h = _create_writer(file);
 

diff --git a/src/os/bluestore/BlueFS.h b/src/os/bluestore/BlueFS.h
index 7b6397d280ea72677a6a847c3ea4930162015632..54dc62eaf87597f240b14e9ad3964a3760f871bc 100644 (file)
--- a/src/os/bluestore/BlueFS.h
+++ b/src/os/bluestore/BlueFS.h
@@ -336,6 +336,11 @@ private:
     uint64_t seq_live = 1;   //seq that is ongoing and dirty files will be written to
     // map of dirty files, files of same dirty_seq are grouped into list.
     std::map<uint64_t, dirty_file_list_t> files;
+    std::vector<interval_set<uint64_t>> pending_release; ///< extents to release
+    // TODO: it should be examined what makes pending_release immune to
+    // eras in a way similar to dirty_files. Hints:
+    // 1) we have actually only 2 eras: log_seq and log_seq+1
+    // 2) we usually not remove extents from files. And when we do, we force log-syncing.
   } dirty;
 
   ceph::condition_variable log_cond;                             ///< used for state control between log flush / log compaction
@@ -354,11 +359,6 @@ private:
   std::vector<uint64_t> block_reserved;            ///< starting reserve extent per device
   std::vector<Allocator*> alloc;                   ///< allocators for bdevs
   std::vector<uint64_t> alloc_size;                ///< alloc size for each device
-  std::vector<interval_set<uint64_t>> pending_release; ///< extents to release
-  // TODO: it should be examined what makes pending_release immune to
-  // eras in a way similar to dirty_files. Hints:
-  // 1) we have actually only 2 eras: log_seq and log_seq+1
-  // 2) we usually not remove extents from files. And when we do, we force log-syncing.
 
   //std::vector<interval_set<uint64_t>> block_unused_too_granular;