From: Xuehan Xu Date: Fri, 13 May 2022 08:50:10 +0000 (+0800) Subject: crimson/os/seastore: mandate all access to backrefs to go through backref manager X-Git-Tag: v18.0.0~813^2~3 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=86433e23e88609d403c1eb7e619ad3ce777d3cd0;p=ceph.git crimson/os/seastore: mandate all access to backrefs to go through backref manager this would avoid other components' unnecessary dependency on Cache Signed-off-by: Xuehan Xu --- diff --git a/src/crimson/os/seastore/backref/btree_backref_manager.cc b/src/crimson/os/seastore/backref/btree_backref_manager.cc index 18e289823b97..b4acf43bc197 100644 --- a/src/crimson/os/seastore/backref/btree_backref_manager.cc +++ b/src/crimson/os/seastore/backref/btree_backref_manager.cc @@ -456,4 +456,78 @@ void BtreeBackrefManager::complete_transaction( } } +Cache::backref_buf_entry_query_set_t +BtreeBackrefManager::get_cached_backrefs_in_range( + paddr_t start, + paddr_t end) +{ + return cache.get_backrefs_in_range(start, end); +} + +Cache::backref_buf_entry_query_set_t +BtreeBackrefManager::get_cached_backref_removals_in_range( + paddr_t start, + paddr_t end) +{ + return cache.get_del_backrefs_in_range(start, end); +} + +const backref_buf_entry_t::set_t& +BtreeBackrefManager::get_cached_backref_removals() +{ + return cache.get_del_backrefs(); +} + +const backref_buf_entry_t::set_t& +BtreeBackrefManager::get_cached_backrefs() +{ + return cache.get_backrefs(); +} + +backref_buf_entry_t +BtreeBackrefManager::get_cached_backref_removal(paddr_t addr) +{ + return cache.get_del_backref(addr); +} + +Cache::backref_extent_buf_entry_query_set_t +BtreeBackrefManager::get_cached_backref_extents_in_range( + paddr_t start, + paddr_t end) +{ + return cache.get_backref_extents_in_range(start, end); +} + +void BtreeBackrefManager::cache_new_backref_extent( + paddr_t paddr, + extent_types_t type) +{ + return cache.add_backref_extent(paddr, type); +} + +BtreeBackrefManager::retrieve_backref_extents_ret +BtreeBackrefManager::retrieve_backref_extents( + Transaction &t, + Cache::backref_extent_buf_entry_query_set_t &&backref_extents, + std::vector &extents) +{ + return trans_intr::parallel_for_each( + backref_extents, + [this, &extents, &t](auto &ent) { + // only the gc fiber which is single can rewrite backref extents, + // so it must be alive + assert(is_backref_node(ent.type)); + LOG_PREFIX(BtreeBackrefManager::retrieve_backref_extents); + DEBUGT("getting backref extent of type {} at {}", + t, + ent.type, + ent.paddr); + return cache.get_extent_by_type( + t, ent.type, ent.paddr, L_ADDR_NULL, BACKREF_NODE_SIZE + ).si_then([&extents](auto ext) { + extents.emplace_back(std::move(ext)); + }); + }); +} + } // namespace crimson::os::seastore::backref diff --git a/src/crimson/os/seastore/backref/btree_backref_manager.h b/src/crimson/os/seastore/backref/btree_backref_manager.h index fb0dd50ab0f3..4a48b475759a 100644 --- a/src/crimson/os/seastore/backref/btree_backref_manager.h +++ b/src/crimson/os/seastore/backref/btree_backref_manager.h @@ -105,6 +105,33 @@ public: auto *bpin = reinterpret_cast(&pin); pin_set.retire(bpin->get_range_pin()); } + + Cache::backref_buf_entry_query_set_t + get_cached_backrefs_in_range( + paddr_t start, + paddr_t end) final; + + Cache::backref_buf_entry_query_set_t + get_cached_backref_removals_in_range( + paddr_t start, + paddr_t end) final; + + const backref_buf_entry_t::set_t& get_cached_backref_removals() final; + const backref_buf_entry_t::set_t& get_cached_backrefs() final; + backref_buf_entry_t get_cached_backref_removal(paddr_t addr) final; + + Cache::backref_extent_buf_entry_query_set_t + get_cached_backref_extents_in_range( + paddr_t start, + paddr_t end) final; + + retrieve_backref_extents_ret retrieve_backref_extents( + Transaction &t, + Cache::backref_extent_buf_entry_query_set_t &&backref_extents, + std::vector &extents) final; + + void cache_new_backref_extent(paddr_t paddr, extent_types_t type) final; + private: SegmentManagerGroup &sm_group; Cache &cache; diff --git a/src/crimson/os/seastore/backref_manager.h b/src/crimson/os/seastore/backref_manager.h index 8afa21ef79ea..d1ba4b954c9b 100644 --- a/src/crimson/os/seastore/backref_manager.h +++ b/src/crimson/os/seastore/backref_manager.h @@ -97,6 +97,38 @@ public: // created by this insertion ) = 0; + virtual Cache::backref_buf_entry_query_set_t + get_cached_backrefs_in_range( + paddr_t start, + paddr_t end) = 0; + + virtual Cache::backref_buf_entry_query_set_t + get_cached_backref_removals_in_range( + paddr_t start, + paddr_t end) = 0; + + virtual const backref_buf_entry_t::set_t& get_cached_backref_removals() = 0; + virtual const backref_buf_entry_t::set_t& get_cached_backrefs() = 0; + virtual backref_buf_entry_t get_cached_backref_removal(paddr_t addr) = 0; + + virtual Cache::backref_extent_buf_entry_query_set_t + get_cached_backref_extents_in_range( + paddr_t start, + paddr_t end) = 0; + + using retrieve_backref_extents_iertr = trans_iertr< + crimson::errorator< + crimson::ct_error::input_output_error> + >; + using retrieve_backref_extents_ret = + retrieve_backref_extents_iertr::future<>; + virtual retrieve_backref_extents_ret retrieve_backref_extents( + Transaction &t, + Cache::backref_extent_buf_entry_query_set_t &&backref_extents, + std::vector &extents) = 0; + + virtual void cache_new_backref_extent(paddr_t paddr, extent_types_t type) = 0; + /** * insert new mappings directly from Cache */ diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index 6c20ceb4fe0d..ac84df837b39 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -19,8 +19,13 @@ #include "crimson/os/seastore/segment_manager.h" #include "crimson/os/seastore/transaction.h" +namespace crimson::os::seastore::backref { +class BtreeBackrefManager; +} + namespace crimson::os::seastore { +class BackrefManager; class SegmentCleaner; struct backref_buf_entry_t { @@ -530,45 +535,13 @@ private: // backrefs that needs to be inserted into the backref tree backref_buf_entry_t::set_t backref_inserted_set; backref_buf_entry_t::set_t backref_remove_set; // backrefs needs to be removed - // from the backref tree -public: - /** - * get_extent_by_type - * - * Based on type, instantiate the correct concrete type - * and read in the extent at location offset~length. - */ - template - get_extent_by_type_ret get_extent_by_type( - Transaction &t, ///< [in] transaction - extent_types_t type, ///< [in] type tag - paddr_t offset, ///< [in] starting addr - laddr_t laddr, ///< [in] logical address if logical - seastore_off_t length, ///< [in] length - Func &&extent_init_func ///< [in] extent init func - ) { - return _get_extent_by_type( - t, - type, - offset, - laddr, - length, - extent_init_func_t(std::forward(extent_init_func))); - } - get_extent_by_type_ret get_extent_by_type( - Transaction &t, - extent_types_t type, - paddr_t offset, - laddr_t laddr, - seastore_off_t length - ) { - return get_extent_by_type( - t, type, offset, laddr, length, [](CachedExtent &) {}); - } + // from the backref tree - std::set< - backref_buf_entry_t, - backref_buf_entry_t::cmp_t> get_backrefs_in_range( + using backref_buf_entry_query_set_t = + std::set< + backref_buf_entry_t, + backref_buf_entry_t::cmp_t>; + backref_buf_entry_query_set_t get_backrefs_in_range( paddr_t start, paddr_t end) { auto start_iter = backref_inserted_set.lower_bound( @@ -588,9 +561,7 @@ public: return res; } - std::set< - backref_buf_entry_t, - backref_buf_entry_t::cmp_t> get_del_backrefs_in_range( + backref_buf_entry_query_set_t get_del_backrefs_in_range( paddr_t start, paddr_t end) { LOG_PREFIX(Cache::get_del_backrefs_in_range); @@ -632,6 +603,41 @@ public: return backref_buffer; } +public: + /** + * get_extent_by_type + * + * Based on type, instantiate the correct concrete type + * and read in the extent at location offset~length. + */ + template + get_extent_by_type_ret get_extent_by_type( + Transaction &t, ///< [in] transaction + extent_types_t type, ///< [in] type tag + paddr_t offset, ///< [in] starting addr + laddr_t laddr, ///< [in] logical address if logical + seastore_off_t length, ///< [in] length + Func &&extent_init_func ///< [in] extent init func + ) { + return _get_extent_by_type( + t, + type, + offset, + laddr, + length, + extent_init_func_t(std::forward(extent_init_func))); + } + get_extent_by_type_ret get_extent_by_type( + Transaction &t, + extent_types_t type, + paddr_t offset, + laddr_t laddr, + seastore_off_t length + ) { + return get_extent_by_type( + t, type, offset, laddr, length, [](CachedExtent &) {}); + } + void trim_backref_bufs(const journal_seq_t &trim_to) { LOG_PREFIX(Cache::trim_backref_bufs); SUBDEBUG(seastore_cache, "trimming to {}", trim_to); @@ -946,6 +952,26 @@ public: }; }; +private: + ExtentPlacementManager& epm; + RootBlockRef root; ///< ref to current root + ExtentIndex extents; ///< set of live extents + + journal_seq_t last_commit = JOURNAL_SEQ_MIN; + + /** + * dirty + * + * holds refs to dirty extents. Ordered by CachedExtent::get_dirty_from(). + */ + CachedExtent::list dirty; + + using backref_extent_buf_entry_query_set_t = + std::set< + backref_extent_buf_entry_t, + backref_extent_buf_entry_t::cmp_t>; + backref_extent_buf_entry_query_set_t backref_extents; + void add_backref_extent(paddr_t paddr, extent_types_t type) { assert(!paddr.is_relative()); auto [iter, inserted] = backref_extents.emplace(paddr, type); @@ -959,38 +985,18 @@ public: backref_extents.erase(iter); } - std::set< - backref_extent_buf_entry_t, - backref_extent_buf_entry_t::cmp_t> get_backref_extents_in_range( + backref_extent_buf_entry_query_set_t get_backref_extents_in_range( paddr_t start, paddr_t end) { auto start_iter = backref_extents.lower_bound(start); auto end_iter = backref_extents.upper_bound(end); - std::set< - backref_extent_buf_entry_t, - backref_extent_buf_entry_t::cmp_t> res; + backref_extent_buf_entry_query_set_t res; res.insert(start_iter, end_iter); return res; } -private: - ExtentPlacementManager& epm; - RootBlockRef root; ///< ref to current root - ExtentIndex extents; ///< set of live extents - - journal_seq_t last_commit = JOURNAL_SEQ_MIN; - - /** - * dirty - * - * holds refs to dirty extents. Ordered by CachedExtent::get_dirty_from(). - */ - CachedExtent::list dirty; - - std::set< - backref_extent_buf_entry_t, - backref_extent_buf_entry_t::cmp_t> backref_extents; - + friend class crimson::os::seastore::backref::BtreeBackrefManager; + friend class crimson::os::seastore::BackrefManager; /** * lru * diff --git a/src/crimson/os/seastore/lba_manager/btree/lba_btree_node.h b/src/crimson/os/seastore/lba_manager/btree/lba_btree_node.h index 004a5778001c..09a9a53f3e8c 100644 --- a/src/crimson/os/seastore/lba_manager/btree/lba_btree_node.h +++ b/src/crimson/os/seastore/lba_manager/btree/lba_btree_node.h @@ -77,7 +77,7 @@ using lba_node_meta_le_t = fixed_kv_node_meta_le_t; * TODO: make the above capacity calculation part of FixedKVNodeLayout * TODO: the above alignment probably isn't portable without further work */ -constexpr size_t INTERNAL_NODE_CAPACITY = 254; +constexpr size_t INTERNAL_NODE_CAPACITY = 32; struct LBAInternalNode : FixedKVInternalNode< INTERNAL_NODE_CAPACITY, @@ -115,7 +115,7 @@ using LBAInternalNodeRef = LBAInternalNode::Ref; * TODO: update FixedKVNodeLayout to handle the above calculation * TODO: the above alignment probably isn't portable without further work */ -constexpr size_t LEAF_NODE_CAPACITY = 145; +constexpr size_t LEAF_NODE_CAPACITY = 32; /** * lba_map_val_le_t diff --git a/src/crimson/os/seastore/segment_cleaner.cc b/src/crimson/os/seastore/segment_cleaner.cc index fedcce9579cc..b35fc74710a2 100644 --- a/src/crimson/os/seastore/segment_cleaner.cc +++ b/src/crimson/os/seastore/segment_cleaner.cc @@ -390,13 +390,11 @@ SegmentCleaner::SegmentCleaner( config_t config, SegmentManagerGroupRef&& sm_group, BackrefManager &backref_manager, - Cache &cache, bool detailed) : detailed(detailed), config(config), sm_group(std::move(sm_group)), backref_manager(backref_manager), - cache(cache), ool_segment_seq_allocator( new SegmentSeqAllocator(segment_type_t::OOL)), gc_process(*this) @@ -769,33 +767,6 @@ SegmentCleaner::gc_trim_journal_ret SegmentCleaner::gc_trim_journal() }); } -SegmentCleaner::retrieve_backref_extents_ret -SegmentCleaner::_retrieve_backref_extents( - Transaction &t, - std::set< - Cache::backref_extent_buf_entry_t, - Cache::backref_extent_buf_entry_t::cmp_t> &&backref_extents, - std::vector &extents) -{ - return trans_intr::parallel_for_each( - backref_extents, - [this, &extents, &t](auto &ent) { - // only the gc fiber which is single can rewrite backref extents, - // so it must be alive - assert(is_backref_node(ent.type)); - LOG_PREFIX(SegmentCleaner::_retrieve_backref_extents); - DEBUGT("getting backref extent of type {} at {}", - t, - ent.type, - ent.paddr); - return cache.get_extent_by_type( - t, ent.type, ent.paddr, L_ADDR_NULL, BACKREF_NODE_SIZE - ).si_then([&extents](auto ext) { - extents.emplace_back(std::move(ext)); - }); - }); -} - SegmentCleaner::retrieve_live_extents_ret SegmentCleaner::_retrieve_live_extents( Transaction &t, @@ -822,7 +793,7 @@ SegmentCleaner::_retrieve_live_extents( ).si_then([this, FNAME, &extents, &ent, &seq, &t](auto ext) { if (!ext) { DEBUGT("addr {} dead, skipping", t, ent.paddr); - auto backref = cache.get_del_backref(ent.paddr); + auto backref = backref_manager.get_cached_backref_removal(ent.paddr); if (seq == JOURNAL_SEQ_NULL || seq < backref.seq) { seq = backref.seq; } @@ -872,10 +843,11 @@ SegmentCleaner::gc_reclaim_space_ret SegmentCleaner::gc_reclaim_space() reclaimed = 0; runs++; return seastar::do_with( - cache.get_backref_extents_in_range( + backref_manager.get_cached_backref_extents_in_range( + *next_reclaim_pos, end_paddr), + backref_manager.get_cached_backrefs_in_range( *next_reclaim_pos, end_paddr), - cache.get_backrefs_in_range(*next_reclaim_pos, end_paddr), - cache.get_del_backrefs_in_range( + backref_manager.get_cached_backref_removals_in_range( *next_reclaim_pos, end_paddr), JOURNAL_SEQ_NULL, [this, segment_id, &reclaimed, end_paddr] @@ -920,7 +892,7 @@ SegmentCleaner::gc_reclaim_space_ret SegmentCleaner::gc_reclaim_space() std::vector(), [this, &backref_extents, &backrefs, &reclaimed, &t, &seq] (auto &extents) { - return _retrieve_backref_extents( + return backref_manager.retrieve_backref_extents( t, std::move(backref_extents), extents ).si_then([this, &extents, &t, &backrefs] { return _retrieve_live_extents( @@ -958,8 +930,9 @@ SegmentCleaner::gc_reclaim_space_ret SegmentCleaner::gc_reclaim_space() [&reclaimed, this, pavail_ratio, start, &runs, end_paddr] { LOG_PREFIX(SegmentCleaner::gc_reclaim_space); #ifndef NDEBUG - auto ndel_backrefs = cache.get_del_backrefs_in_range( - *next_reclaim_pos, end_paddr); + auto ndel_backrefs = + backref_manager.get_cached_backref_removals_in_range( + *next_reclaim_pos, end_paddr); if (!ndel_backrefs.empty()) { for (auto &del_br : ndel_backrefs) { ERROR("unexpected del_backref {}~{} {} {}", diff --git a/src/crimson/os/seastore/segment_cleaner.h b/src/crimson/os/seastore/segment_cleaner.h index 6d0b6d33bd27..bd321e84bb4b 100644 --- a/src/crimson/os/seastore/segment_cleaner.h +++ b/src/crimson/os/seastore/segment_cleaner.h @@ -639,7 +639,6 @@ private: SegmentManagerGroupRef sm_group; BackrefManager &backref_manager; - Cache &cache; SpaceTrackerIRef space_tracker; segments_info_t segments; @@ -716,7 +715,6 @@ public: config_t config, SegmentManagerGroupRef&& sm_group, BackrefManager &backref_manager, - Cache &cache, bool detailed = false); SegmentSeqAllocator& get_ool_segment_seq_allocator() { @@ -1027,15 +1025,6 @@ private: using gc_reclaim_space_ret = gc_reclaim_space_ertr::future<>; gc_reclaim_space_ret gc_reclaim_space(); - using retrieve_backref_extents_iertr = work_iertr; - using retrieve_backref_extents_ret = - retrieve_backref_extents_iertr::future<>; - retrieve_backref_extents_ret _retrieve_backref_extents( - Transaction &t, - std::set< - Cache::backref_extent_buf_entry_t, - Cache::backref_extent_buf_entry_t::cmp_t> &&backref_extents, - std::vector &extents); using retrieve_live_extents_iertr = work_iertr; using retrieve_live_extents_ret = diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc index 16ef8ef5ae4d..f52f59089f99 100644 --- a/src/crimson/os/seastore/transaction_manager.cc +++ b/src/crimson/os/seastore/transaction_manager.cc @@ -144,16 +144,16 @@ TransactionManager::mount_ertr::future<> TransactionManager::mount() } if (depth) { if (depth > 1) { - cache->add_backref_extent( + backref_manager->cache_new_backref_extent( addr, extent_types_t::BACKREF_INTERNAL); } else { - cache->add_backref_extent( + backref_manager->cache_new_backref_extent( addr, extent_types_t::BACKREF_LEAF); } } }).si_then([this] { LOG_PREFIX(TransactionManager::mount); - auto &backrefs = cache->get_backrefs(); + auto &backrefs = backref_manager->get_cached_backrefs(); DEBUG("marking {} backrefs used", backrefs.size()); for (auto &backref : backrefs) { segment_cleaner->mark_space_used( @@ -163,7 +163,7 @@ TransactionManager::mount_ertr::future<> TransactionManager::mount() seastar::lowres_system_clock::time_point(), true); } - auto &del_backrefs = cache->get_del_backrefs(); + auto &del_backrefs = backref_manager->get_cached_backref_removals(); DEBUG("marking {} backrefs free", del_backrefs.size()); for (auto &del_backref : del_backrefs) { segment_cleaner->mark_space_free( @@ -658,7 +658,6 @@ TransactionManagerRef make_transaction_manager(tm_make_config_t config) cleaner_config, std::move(sms), *backref_manager, - *cache, cleaner_is_detailed); JournalRef journal; diff --git a/src/test/crimson/seastore/test_transaction_manager.cc b/src/test/crimson/seastore/test_transaction_manager.cc index 8ed60e430e08..31382e2e2a91 100644 --- a/src/test/crimson/seastore/test_transaction_manager.cc +++ b/src/test/crimson/seastore/test_transaction_manager.cc @@ -408,7 +408,7 @@ struct transaction_manager_test_t : len); } }).si_then([&tracker, this] { - auto &backrefs = cache->get_backrefs(); + auto &backrefs = backref_manager->get_cached_backrefs(); for (auto &backref : backrefs) { if (backref.paddr.get_addr_type() == addr_types_t::SEGMENT) { logger().debug("check_usage: by backref, tracker alloc {}~{}", @@ -419,7 +419,7 @@ struct transaction_manager_test_t : backref.len); } } - auto &del_backrefs = cache->get_del_backrefs(); + auto &del_backrefs = backref_manager->get_cached_backref_removals(); for (auto &del_backref : del_backrefs) { if (del_backref.paddr.get_addr_type() == addr_types_t::SEGMENT) { logger().debug("check_usage: by backref, tracker release {}~{}",