From: Adam Kupczyk Date: Thu, 23 Dec 2021 14:12:52 +0000 (+0100) Subject: os/bluestore/bluefs: Cleanup on pending_release variable X-Git-Tag: v16.2.14~23^2~17 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=2f0c52ac8e792b0bb90b663cd0ff58f17ab97f44;p=ceph.git 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 (cherry picked from commit 2b36f27707ba6efb80eeebef487d7403a478a528) --- diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc index 93defb4c8721f..8369f63e472d7 100644 --- 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> to_release(pending_release.size()); - to_release.swap(pending_release); + vector> 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> to_release(pending_release.size()); - to_release.swap(pending_release); + vector> 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 pending_release_extents; + { std::unique_lock nl(nodes.lock); dout(10) << __func__ << " " << dirname << "/" << filename << dendl; map::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::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 7b6397d280ea7..54dc62eaf8759 100644 --- 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 files; + std::vector> 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 block_reserved; ///< starting reserve extent per device std::vector alloc; ///< allocators for bdevs std::vector alloc_size; ///< alloc size for each device - std::vector> 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> block_unused_too_granular;