From 6b7912dabd47b55ee75179de2d9ac595adbf61f1 Mon Sep 17 00:00:00 2001 From: Adam Kupczyk Date: Thu, 23 Dec 2021 15:30:59 +0100 Subject: [PATCH] os/bluestore/bluefs: Added minor improvements Added comments. Removed comments. Fixed lock-tracking suffixes to functions stemming from change _compact_log_dump_metadata_N -> _compact_log_dump_metadata_NF Removed extra lock_fnode_print. Signed-off-by: Adam Kupczyk (cherry picked from commit fd9ef8b0c1d4de01be36904a638b86fb8fa58702) --- src/os/bluestore/BlueFS.cc | 71 ++++++++++++++++++-------------------- src/os/bluestore/BlueFS.h | 8 ++--- 2 files changed, 38 insertions(+), 41 deletions(-) diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc index 8369f63e472d7..1b63bc81cc22c 100644 --- a/src/os/bluestore/BlueFS.cc +++ b/src/os/bluestore/BlueFS.cc @@ -842,13 +842,13 @@ void BlueFS::umount(bool avoid_compact) _close_writer(log.writer); log.writer = NULL; + log.t.clear(); vselector.reset(nullptr); _stop_alloc(); nodes.file_map.clear(); nodes.dir_map.clear(); super = bluefs_super_t(); - log.t.clear(); _shutdown_logger(); } @@ -863,7 +863,7 @@ int BlueFS::prepare_new_device(int id, const bluefs_layout_t& layout) new_log_dev_cur = BDEV_NEWDB; new_log_dev_next = BDEV_DB; } - _rewrite_log_and_layout_sync_LN_LD(false, + _rewrite_log_and_layout_sync_LNF_LD(false, BDEV_NEWDB, new_log_dev_cur, new_log_dev_next, @@ -871,7 +871,7 @@ int BlueFS::prepare_new_device(int id, const bluefs_layout_t& layout) layout); //} } else if(id == BDEV_NEWWAL) { - _rewrite_log_and_layout_sync_LN_LD(false, + _rewrite_log_and_layout_sync_LNF_LD(false, BDEV_DB, BDEV_NEWWAL, BDEV_WAL, @@ -1713,7 +1713,7 @@ int BlueFS::device_migrate_to_existing( new_log_dev_next; } - _rewrite_log_and_layout_sync_LN_LD( + _rewrite_log_and_layout_sync_LNF_LD( false, (flags & REMOVE_DB) ? BDEV_SLOW : BDEV_DB, new_log_dev_cur, @@ -1848,7 +1848,7 @@ int BlueFS::device_migrate_to_new( BDEV_DB : BDEV_SLOW; - _rewrite_log_and_layout_sync_LN_LD( + _rewrite_log_and_layout_sync_LNF_LD( false, super_dev, new_log_dev_cur, @@ -1875,6 +1875,14 @@ BlueFS::FileRef BlueFS::_get_file(uint64_t ino) } } + +/** +To modify fnode both FileWriter::lock and File::lock must be obtained. +The special case is when we modify bluefs log (ino 1) or +we are compacting log (ino 0). + +In any case it is enough to hold File::lock to be sure fnode will not be modified. +*/ struct lock_fnode_print { BlueFS::FileRef file; lock_fnode_print(BlueFS::FileRef file) : file(file) {}; @@ -1891,6 +1899,7 @@ void BlueFS::_drop_link_D(FileRef file) << " on " << lock_fnode_print(file) << dendl; ceph_assert(file->refs > 0); ceph_assert(ceph_mutex_is_locked(log.lock)); + ceph_assert(ceph_mutex_is_locked(nodes.lock)); --file->refs; if (file->refs == 0) { @@ -2136,7 +2145,7 @@ int64_t BlueFS::_read( void BlueFS::invalidate_cache(FileRef f, uint64_t offset, uint64_t length) { std::lock_guard l(f->lock); - dout(10) << __func__ << " file " << lock_fnode_print(f) + dout(10) << __func__ << " file " << f->fnode << " 0x" << std::hex << offset << "~" << length << std::dec << dendl; if (offset & ~super.block_mask()) { @@ -2171,7 +2180,7 @@ void BlueFS::compact_log() { if (!cct->_conf->bluefs_replay_recovery_disable_compact) { if (cct->_conf->bluefs_compact_log_sync) { - _compact_log_sync_LN_LD(); + _compact_log_sync_LNF_LD(); } else { _compact_log_async_LD_NF_D(); } @@ -2202,7 +2211,7 @@ bool BlueFS::_should_start_compact_log_L_N() return true; } -void BlueFS::_compact_log_dump_metadata_N(bluefs_transaction_t *t, +void BlueFS::_compact_log_dump_metadata_NF(bluefs_transaction_t *t, int flags) { std::lock_guard nl(nodes.lock); @@ -2287,12 +2296,16 @@ void BlueFS::_compact_log_async_dump_metadata_NF(bluefs_transaction_t *t, } } -void BlueFS::_compact_log_sync_LN_LD() +void BlueFS::_compact_log_sync_LNF_LD() { dout(10) << __func__ << dendl; - auto prefer_bdev = - vselector->select_prefer_bdev(log.writer->file->vselector_hint); - _rewrite_log_and_layout_sync_LN_LD(true, + uint8_t prefer_bdev; + { + std::lock_guard ll(log.lock); + prefer_bdev = + vselector->select_prefer_bdev(log.writer->file->vselector_hint); + } + _rewrite_log_and_layout_sync_LNF_LD(true, BDEV_DB, prefer_bdev, prefer_bdev, @@ -2301,7 +2314,7 @@ void BlueFS::_compact_log_sync_LN_LD() logger->inc(l_bluefs_log_compactions); } -void BlueFS::_rewrite_log_and_layout_sync_LN_LD(bool allocate_with_fallback, +void BlueFS::_rewrite_log_and_layout_sync_LNF_LD(bool allocate_with_fallback, int super_dev, int log_dev, int log_dev_new, @@ -2326,7 +2339,7 @@ void BlueFS::_rewrite_log_and_layout_sync_LN_LD(bool allocate_with_fallback, << " flags:" << flags << dendl; bluefs_transaction_t t; - _compact_log_dump_metadata_N(&t, flags); + _compact_log_dump_metadata_NF(&t, flags); dout(20) << __func__ << " op_jump_seq " << log.seq_live << dendl; t.op_jump_seq(log.seq_live); @@ -2507,9 +2520,6 @@ void BlueFS::_compact_log_async_LD_NF_D() //also locks FW for new_writer &new_log->fnode); ceph_assert(r == 0); - // we might have some more ops in log_t due to _allocate call - // t.claim_ops(log_t); no we no longer track allocations in log - bufferlist bl; encode(t, bl); _pad_bl(bl); @@ -2624,19 +2634,6 @@ void BlueFS::_pad_bl(bufferlist& bl) } } -/** - * Adds file modifications from `dirty.files` to bluefs transactions - * already stored in `log.t`. Writes them to disk and waits until are stable. - * Guarantees that file modifications with `want_seq` are already stable on disk. - * In addition may insert jump forward transaction to log write position `jump_to`. - * - * it should lock ability to: - * 1) add to log.t - * 2) modify dirty.files - * 3) add to pending_release - * - * pending_release and log.t go with same lock - */ // Returns log seq that was live before advance. uint64_t BlueFS::_log_advance_seq() @@ -2971,7 +2968,7 @@ int BlueFS::_signal_dirty_to_log_D(FileWriter *h) return 0; } -void BlueFS::flush_range/*WF*/(FileWriter *h, uint64_t offset, uint64_t length) { +void BlueFS::flush_range(FileWriter *h, uint64_t offset, uint64_t length) { std::unique_lock hl(h->lock); _flush_range_F(h, offset, length); } @@ -3174,7 +3171,7 @@ void BlueFS::append_try_flush(FileWriter *h, const char* buf, size_t len) } } if (flushed_sum) { - _maybe_compact_log_LN_NF_LD_D(); + _maybe_compact_log_LNF_NF_LD_D(); } } @@ -3188,7 +3185,7 @@ void BlueFS::flush(FileWriter *h, bool force) ceph_assert(r == 0); } if (r == 0 && flushed) { - _maybe_compact_log_LN_NF_LD_D(); + _maybe_compact_log_LNF_NF_LD_D(); } } @@ -3312,7 +3309,7 @@ int BlueFS::fsync(FileWriter *h) if (old_dirty_seq) { _flush_and_sync_log_LD(old_dirty_seq); } - _maybe_compact_log_LN_NF_LD_D(); + _maybe_compact_log_LNF_NF_LD_D(); return 0; } @@ -3515,16 +3512,16 @@ void BlueFS::sync_metadata(bool avoid_compact) } if (!avoid_compact) { - _maybe_compact_log_LN_NF_LD_D(); + _maybe_compact_log_LNF_NF_LD_D(); } } -void BlueFS::_maybe_compact_log_LN_NF_LD_D() +void BlueFS::_maybe_compact_log_LNF_NF_LD_D() { if (!cct->_conf->bluefs_replay_recovery_disable_compact && _should_start_compact_log_L_N()) { if (cct->_conf->bluefs_compact_log_sync) { - _compact_log_sync_LN_LD(); + _compact_log_sync_LNF_LD(); } else { _compact_log_async_LD_NF_D(); } diff --git a/src/os/bluestore/BlueFS.h b/src/os/bluestore/BlueFS.h index 54dc62eaf8759..2ffb2b8bb780c 100644 --- a/src/os/bluestore/BlueFS.h +++ b/src/os/bluestore/BlueFS.h @@ -436,15 +436,15 @@ private: RENAME_SLOW2DB = 4, RENAME_DB2SLOW = 8, }; - void _compact_log_dump_metadata_N(bluefs_transaction_t *t, + void _compact_log_dump_metadata_NF(bluefs_transaction_t *t, int flags); void _compact_log_async_dump_metadata_NF(bluefs_transaction_t *t, uint64_t capture_before_seq); - void _compact_log_sync_LN_LD(); + void _compact_log_sync_LNF_LD(); void _compact_log_async_LD_NF_D(); - void _rewrite_log_and_layout_sync_LN_LD(bool allocate_with_fallback, + void _rewrite_log_and_layout_sync_LNF_LD(bool allocate_with_fallback, int super_dev, int log_dev, int new_log_dev, @@ -575,7 +575,7 @@ public: /// sync any uncommitted state to disk void sync_metadata(bool avoid_compact); /// test and compact log, if necessary - void _maybe_compact_log_LN_NF_LD_D(); + void _maybe_compact_log_LNF_NF_LD_D(); void set_volume_selector(BlueFSVolumeSelector* s) { vselector.reset(s); -- 2.39.5