From 9709b14066c359fdb4a56331c0e4feb4f0e732dc Mon Sep 17 00:00:00 2001 From: Adam Kupczyk Date: Sat, 4 Dec 2021 16:50:22 +0100 Subject: [PATCH] os/bluestore/bluefs: Review comments, misc changes. Signed-off-by: Adam Kupczyk --- src/os/bluestore/BlueFS.cc | 42 +++++++++++++++++++++++--------------- src/os/bluestore/BlueFS.h | 5 ++--- 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc index 9b4d87f1bf0a1..8fcac1a24fd0c 100644 --- a/src/os/bluestore/BlueFS.cc +++ b/src/os/bluestore/BlueFS.cc @@ -1649,7 +1649,6 @@ int BlueFS::_replay(bool noop, bool to_stdout) log.seq_live = log_seq + 1; dirty.seq_live = log_seq + 1; log.t.seq = log.seq_live; - //log.seq_stable = log_seq; dirty.seq_stable = log_seq; } @@ -1990,10 +1989,20 @@ BlueFS::FileRef BlueFS::_get_file(uint64_t ino) } } +struct lock_fnode_print { + BlueFS::FileRef file; + lock_fnode_print(BlueFS::FileRef file) : file(file) {}; +}; +std::ostream& operator<<(std::ostream& out, const lock_fnode_print& to_lock) { + std::lock_guard l(to_lock.file->lock); + out << to_lock.file->fnode; + return out; +} + void BlueFS::_drop_link_D(FileRef file) { dout(20) << __func__ << " had refs " << file->refs - << " on " << file->fnode << dendl; + << " on " << lock_fnode_print(file) << dendl; ceph_assert(file->refs > 0); ceph_assert(ceph_mutex_is_locked(log.lock)); @@ -2003,14 +2012,13 @@ void BlueFS::_drop_link_D(FileRef file) ceph_assert(file->num_reading.load() == 0); vselector->sub_usage(file->vselector_hint, file->fnode); log.t.op_file_remove(file->fnode.ino); + nodes.file_map.erase(file->fnode.ino); + file->deleted = true; std::lock_guard dl(dirty.lock); for (auto& r : file->fnode.extents) { pending_release[r.bdev].insert(r.offset, r.length); } - nodes.file_map.erase(file->fnode.ino); - file->deleted = true; - if (file->dirty_seq > dirty.seq_stable) { // retract request to serialize changes ceph_assert(dirty.files.count(file->dirty_seq)); @@ -2032,7 +2040,7 @@ int64_t BlueFS::_read_random( int64_t ret = 0; dout(10) << __func__ << " h " << h << " 0x" << std::hex << off << "~" << len << std::dec - << " from " << h->file->fnode << dendl; + << " from " << lock_fnode_print(h->file) << dendl; ++h->file->num_reading; @@ -2125,7 +2133,7 @@ int64_t BlueFS::_read( bool prefetch = !outbl && !out; dout(10) << __func__ << " h " << h << " 0x" << std::hex << off << "~" << len << std::dec - << " from " << h->file->fnode + << " from " << lock_fnode_print(h->file) << (prefetch ? " prefetch" : "") << dendl; @@ -2242,7 +2250,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 " << f->fnode + dout(10) << __func__ << " file " << lock_fnode_print(f) << " 0x" << std::hex << offset << "~" << length << std::dec << dendl; if (offset & ~super.block_mask()) { @@ -2322,7 +2330,7 @@ void BlueFS::_compact_log_dump_metadata_N(bluefs_transaction_t *t, if (ino == 1) continue; ceph_assert(ino > 1); - //AK - TODO - touching fnode - need to lock + std::lock_guard fl(file_ref->lock); for(auto& e : file_ref->fnode.extents) { auto bdev = e.bdev; auto bdev_new = bdev; @@ -2359,8 +2367,8 @@ void BlueFS::_compact_log_dump_metadata_N(bluefs_transaction_t *t, } } /* Streams to t files modified before *capture_before_seq* and all dirs */ -void BlueFS::compact_log_async_dump_metadata_NF(bluefs_transaction_t *t, - uint64_t capture_before_seq) +void BlueFS::_compact_log_async_dump_metadata_NF(bluefs_transaction_t *t, + uint64_t capture_before_seq) { std::lock_guard nl(nodes.lock); @@ -2592,7 +2600,7 @@ void BlueFS::_compact_log_async_LD_NF_D() //also locks FW for new_writer // 2. prepare compacted log bluefs_transaction_t t; - compact_log_async_dump_metadata_NF(&t, seq_now); + _compact_log_async_dump_metadata_NF(&t, seq_now); // now state is captured to bufferlist // log can be used to write to, ops in log will be continuation of captured state @@ -2882,7 +2890,7 @@ void BlueFS::_clear_dirty_set_stable_D(uint64_t seq) while (l != p->second.end()) { File *file = &*l; ceph_assert(file->dirty_seq <= dirty.seq_stable); - dout(20) << __func__ << " cleaned file " << file->fnode << dendl; + dout(20) << __func__ << " cleaned file " << file->fnode.ino << dendl; file->dirty_seq = dirty.seq_stable; p->second.erase(l++); } @@ -2938,7 +2946,7 @@ int BlueFS::_flush_and_sync_log_LD(uint64_t want_seq) if (available_runway == -EWOULDBLOCK) { // we are in need of adding runway, but we are during log-switch from compaction dirty.lock.unlock(); - //instead log.lock_unlock() do move ownership + //instead log.lock.unlock() do move ownership std::unique_lock ll(log.lock, std::adopt_lock); while (log_forbidden_to_expand.load()) { log_cond.wait(ll); @@ -3080,6 +3088,9 @@ void BlueFS::flush_range/*WF*/(FileWriter *h, uint64_t offset, uint64_t length) int BlueFS::_flush_range_F(FileWriter *h, uint64_t offset, uint64_t length) { ceph_assert(ceph_mutex_is_locked(h->lock)); + ceph_assert(h->file->num_readers.load() == 0); + ceph_assert(h->file->fnode.ino > 1); + dout(10) << __func__ << " " << h << " pos 0x" << std::hex << h->pos << " 0x" << offset << "~" << length << std::dec << " to " << h->file->fnode << dendl; @@ -3088,10 +3099,7 @@ int BlueFS::_flush_range_F(FileWriter *h, uint64_t offset, uint64_t length) return 0; } - ceph_assert(h->file->num_readers.load() == 0); - bool buffered = cct->_conf->bluefs_buffered_io; - ceph_assert(h->file->fnode.ino > 1); if (offset + length <= h->pos) return 0; diff --git a/src/os/bluestore/BlueFS.h b/src/os/bluestore/BlueFS.h index abdd117ad2376..931e1e3114a0b 100644 --- a/src/os/bluestore/BlueFS.h +++ b/src/os/bluestore/BlueFS.h @@ -341,7 +341,6 @@ private: struct { ceph::mutex lock = ceph::make_mutex("BlueFS::log.lock"); - //uint64_t seq_stable = 0; //seq that is now stable on disk AK - consider also mirroring this uint64_t seq_live = 1; //seq that log is currently writing to; mirrors dirty.seq_live FileWriter *writer = 0; bluefs_transaction_t t; @@ -455,8 +454,8 @@ private: }; void _compact_log_dump_metadata_N(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_async_dump_metadata_NF(bluefs_transaction_t *t, + uint64_t capture_before_seq); void _compact_log_sync_LN_LD(); void _compact_log_async_LD_NF_D(); -- 2.39.5