From: Igor Fedotov Date: Mon, 21 Mar 2022 11:58:18 +0000 (+0300) Subject: os/bluestore: proper locking for Allocators' dump methods X-Git-Tag: v16.2.11~102^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=299ed59ff06c5a37caffd2bb17f4147172f12559;p=ceph.git os/bluestore: proper locking for Allocators' dump methods Plus renaming parametrized dump to foreach() Fixes: https://tracker.ceph.com/issues/54973 Signed-off-by: Igor Fedotov (cherry picked from commit d7aa5decf369f48616a7b9b83136f2bbbd82ff60) Conflicts: src/os/bluestore/BlueStore.cc src/os/bluestore/BtreeAllocator.cc src/os/bluestore/BtreeAllocator.h src/os/bluestore/ZonedAllocator.cc caused by freshier stuff not present in Pacific --- diff --git a/src/os/bluestore/Allocator.cc b/src/os/bluestore/Allocator.cc index 91a001b5c974..f92821f0c853 100644 --- a/src/os/bluestore/Allocator.cc +++ b/src/os/bluestore/Allocator.cc @@ -87,7 +87,7 @@ public: f->dump_string("length", len_hex); f->close_section(); }; - alloc->dump(iterated_allocation); + alloc->foreach(iterated_allocation); f->close_section(); f->close_section(); } else if (command == "bluestore allocator score " + name) { @@ -203,7 +203,7 @@ double Allocator::get_fragmentation_score() score_sum += get_score(len); sum += len; }; - dump(iterated_allocation); + foreach(iterated_allocation); double ideal = get_score(sum); diff --git a/src/os/bluestore/Allocator.h b/src/os/bluestore/Allocator.h index 177438025d73..38ea4f997388 100644 --- a/src/os/bluestore/Allocator.h +++ b/src/os/bluestore/Allocator.h @@ -54,7 +54,8 @@ public: void release(const PExtentVector& release_set); virtual void dump() = 0; - virtual void dump(std::function notify) = 0; + virtual void foreach( + std::function notify) = 0; virtual void zoned_set_zone_states(std::vector &&_zone_states) {} virtual bool zoned_get_zones_to_clean(std::deque *zones_to_clean) { diff --git a/src/os/bluestore/AvlAllocator.cc b/src/os/bluestore/AvlAllocator.cc index b2eb999906ac..5f17a3689b18 100644 --- a/src/os/bluestore/AvlAllocator.cc +++ b/src/os/bluestore/AvlAllocator.cc @@ -419,7 +419,6 @@ void AvlAllocator::_dump() const << std::dec << dendl; } - ldout(cct, 0) << __func__ << " range_size_tree: " << dendl; for (auto& rs : range_size_tree) { ldout(cct, 0) << std::hex @@ -429,7 +428,15 @@ void AvlAllocator::_dump() const } } -void AvlAllocator::dump(std::function notify) +void AvlAllocator::foreach( + std::function notify) +{ + std::lock_guard l(lock); + _foreach(notify); +} + +void AvlAllocator::_foreach( + std::function notify) const { for (auto& rs : range_tree) { notify(rs.start, rs.end - rs.start); diff --git a/src/os/bluestore/AvlAllocator.h b/src/os/bluestore/AvlAllocator.h index 47ac9e4e7134..f47ee5be003e 100644 --- a/src/os/bluestore/AvlAllocator.h +++ b/src/os/bluestore/AvlAllocator.h @@ -93,7 +93,8 @@ public: double get_fragmentation() override; void dump() override; - void dump(std::function notify) override; + void foreach( + std::function notify) override; void init_add_free(uint64_t offset, uint64_t length) override; void init_rm_free(uint64_t offset, uint64_t length) override; void shutdown() override; @@ -250,6 +251,7 @@ protected: return (static_cast(range_tree.size() - 1) / (free_blocks - 1)); } void _dump() const; + void _foreach(std::function) const; uint64_t _lowest_size_available() { auto rs = range_size_tree.begin(); diff --git a/src/os/bluestore/BitmapAllocator.cc b/src/os/bluestore/BitmapAllocator.cc index a744eb17bfea..9304cb3f239c 100644 --- a/src/os/bluestore/BitmapAllocator.cc +++ b/src/os/bluestore/BitmapAllocator.cc @@ -95,20 +95,10 @@ void BitmapAllocator::dump() auto it = bins_overall.begin(); while (it != bins_overall.end()) { ldout(cct, 0) << __func__ - << " bin " << it->first - << "(< " << byte_u_t((1 << (it->first + 1)) * get_min_alloc_size()) << ")" - << " : " << it->second << " extents" - << dendl; + << " bin " << it->first + << "(< " << byte_u_t((1 << (it->first + 1)) * get_min_alloc_size()) << ")" + << " : " << it->second << " extents" + << dendl; ++it; } } - -void BitmapAllocator::dump(std::function notify) -{ - size_t alloc_size = get_min_alloc_size(); - auto multiply_by_alloc_size = [alloc_size, notify](size_t off, size_t len) { - notify(off * alloc_size, len * alloc_size); - }; - std::lock_guard lck(lock); - l1.dump(multiply_by_alloc_size); -} diff --git a/src/os/bluestore/BitmapAllocator.h b/src/os/bluestore/BitmapAllocator.h index 5c768a4ac2fb..bb6fa73a1a53 100644 --- a/src/os/bluestore/BitmapAllocator.h +++ b/src/os/bluestore/BitmapAllocator.h @@ -41,10 +41,14 @@ public: } void dump() override; - void dump(std::function notify) override; + void foreach( + std::function notify) override + { + foreach_internal(notify); + } double get_fragmentation() override { - return _get_fragmentation(); + return get_fragmentation_internal(); } void init_add_free(uint64_t offset, uint64_t length) override; diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc index e9c5d7af835e..e8e1cb688c79 100644 --- a/src/os/bluestore/BlueFS.cc +++ b/src/os/bluestore/BlueFS.cc @@ -3803,7 +3803,7 @@ size_t BlueFS::probe_alloc_avail(int dev, uint64_t alloc_size) total += p2align(len, alloc_size); }; if (alloc[dev]) { - alloc[dev]->dump(iterated_allocation); + alloc[dev]->foreach(iterated_allocation); } return total; } diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc index 8eb6f36b2ed0..f14cb68a3f2e 100644 --- a/src/os/bluestore/BlueStore.cc +++ b/src/os/bluestore/BlueStore.cc @@ -16935,3 +16935,4 @@ void RocksDBBlueFSVolumeSelector::dump(ostream& sout) { } // ======================================================= +// ======================================================= diff --git a/src/os/bluestore/HybridAllocator.cc b/src/os/bluestore/HybridAllocator.cc index 3a2da67379b5..b78a99ffeba3 100644 --- a/src/os/bluestore/HybridAllocator.cc +++ b/src/os/bluestore/HybridAllocator.cc @@ -145,11 +145,13 @@ void HybridAllocator::dump() << dendl; } -void HybridAllocator::dump(std::function notify) +void HybridAllocator::foreach( + std::function notify) { - AvlAllocator::dump(notify); + std::lock_guard l(lock); + AvlAllocator::_foreach(notify); if (bmap_alloc) { - bmap_alloc->dump(notify); + bmap_alloc->foreach(notify); } } diff --git a/src/os/bluestore/HybridAllocator.h b/src/os/bluestore/HybridAllocator.h index 92e8b9e80b18..d60fc1a319c7 100644 --- a/src/os/bluestore/HybridAllocator.h +++ b/src/os/bluestore/HybridAllocator.h @@ -31,7 +31,8 @@ public: double get_fragmentation() override; void dump() override; - void dump(std::function notify) override; + void foreach( + std::function notify) override; void init_rm_free(uint64_t offset, uint64_t length) override; void shutdown() override; diff --git a/src/os/bluestore/StupidAllocator.cc b/src/os/bluestore/StupidAllocator.cc index bdebe10a5cb7..805a51fb090b 100644 --- a/src/os/bluestore/StupidAllocator.cc +++ b/src/os/bluestore/StupidAllocator.cc @@ -299,7 +299,7 @@ void StupidAllocator::dump() } } -void StupidAllocator::dump(std::function notify) +void StupidAllocator::foreach(std::function notify) { std::lock_guard l(lock); for (unsigned bin = 0; bin < free.size(); ++bin) { diff --git a/src/os/bluestore/StupidAllocator.h b/src/os/bluestore/StupidAllocator.h index bd99a7c835fc..e1fc101e0879 100644 --- a/src/os/bluestore/StupidAllocator.h +++ b/src/os/bluestore/StupidAllocator.h @@ -62,7 +62,7 @@ public: double get_fragmentation() override; void dump() override; - void dump(std::function notify) override; + void foreach(std::function notify) override; void init_add_free(uint64_t offset, uint64_t length) override; void init_rm_free(uint64_t offset, uint64_t length) override; diff --git a/src/os/bluestore/ZonedAllocator.cc b/src/os/bluestore/ZonedAllocator.cc index 0ea278c37dad..71f0e6e8905b 100644 --- a/src/os/bluestore/ZonedAllocator.cc +++ b/src/os/bluestore/ZonedAllocator.cc @@ -108,8 +108,9 @@ void ZonedAllocator::dump() { std::lock_guard l(lock); } -void ZonedAllocator::dump(std::function notify) { +void ZonedAllocator::foreach( + std::function notify) +{ std::lock_guard l(lock); } diff --git a/src/os/bluestore/ZonedAllocator.h b/src/os/bluestore/ZonedAllocator.h index 5bdf197a58d5..47cb46be7646 100644 --- a/src/os/bluestore/ZonedAllocator.h +++ b/src/os/bluestore/ZonedAllocator.h @@ -76,8 +76,8 @@ public: uint64_t get_free() override; void dump() override; - void dump(std::function notify) override; + void foreach( + std::function notify) override; void zoned_set_zone_states(std::vector &&_zone_states) override; bool zoned_get_zones_to_clean(std::deque *zones_to_clean) override; diff --git a/src/os/bluestore/fastbmap_allocator_impl.cc b/src/os/bluestore/fastbmap_allocator_impl.cc index 1b548eab71dd..595b124856f3 100644 --- a/src/os/bluestore/fastbmap_allocator_impl.cc +++ b/src/os/bluestore/fastbmap_allocator_impl.cc @@ -567,7 +567,7 @@ inline ssize_t AllocatorLevel01Loose::count_0s(slot_t slot_val, size_t start_pos { return count_0s(~slot_val, start_pos); } -void AllocatorLevel01Loose::dump( +void AllocatorLevel01Loose::foreach_internal( std::function notify) { size_t len = 0; diff --git a/src/os/bluestore/fastbmap_allocator_impl.h b/src/os/bluestore/fastbmap_allocator_impl.h index 52a1edee2f8a..e44fc76d7500 100644 --- a/src/os/bluestore/fastbmap_allocator_impl.h +++ b/src/os/bluestore/fastbmap_allocator_impl.h @@ -503,7 +503,7 @@ public: static inline ssize_t count_0s(slot_t slot_val, size_t start_pos); static inline ssize_t count_1s(slot_t slot_val, size_t start_pos); - void dump(std::function notify); + void foreach_internal(std::function notify); }; @@ -578,6 +578,22 @@ public: _mark_l2_on_l1(l2_pos, l2_pos_end); return allocated; } + + void foreach_internal( + std::function notify) + { + size_t alloc_size = get_min_alloc_size(); + auto multiply_by_alloc_size = [alloc_size, notify](size_t off, size_t len) { + notify(off * alloc_size, len * alloc_size); + }; + std::lock_guard l(lock); + l1.foreach_internal(multiply_by_alloc_size); + } + double get_fragmentation_internal() { + std::lock_guard l(lock); + return l1.get_fragmentation(); + } + protected: ceph::mutex lock = ceph::make_mutex("AllocatorLevel02::lock"); L1 l1; @@ -824,10 +840,6 @@ protected: { last_pos = 0; } - double _get_fragmentation() { - std::lock_guard l(lock); - return l1.get_fragmentation(); - } }; #endif diff --git a/src/test/objectstore/Allocator_test.cc b/src/test/objectstore/Allocator_test.cc index 0b417ae18640..3e969abc485b 100644 --- a/src/test/objectstore/Allocator_test.cc +++ b/src/test/objectstore/Allocator_test.cc @@ -385,7 +385,7 @@ TEST_P(AllocTest, test_dump_fragmentation_score) ceph_assert(len > 0); free_sum += len; }; - alloc->dump(iterated_allocation); + alloc->foreach(iterated_allocation); EXPECT_GT(1, alloc->get_fragmentation_score()); EXPECT_EQ(capacity, free_sum + allocated_cnt); }