From bf32dac1867902c7aa87a4e2956d1dad9a29d12f Mon Sep 17 00:00:00 2001 From: Xuehan Xu Date: Wed, 20 May 2026 16:31:29 +0800 Subject: [PATCH] crimson/os/seastore/cache: re-implement Cache::retire_absent_extent_addr The new implementation retire an absent extent by constructing a real empty extent and add it to the transaction's retired_set, instead of creating a retired placeholder Signed-off-by: Xuehan Xu --- src/crimson/os/seastore/cache.cc | 67 +++++++---- src/crimson/os/seastore/cache.h | 108 +++++++++++++----- .../os/seastore/lba/btree_lba_manager.cc | 2 +- src/crimson/os/seastore/lba/lba_btree_node.h | 6 + src/crimson/os/seastore/lba_mapping.h | 15 +++ .../os/seastore/transaction_manager.cc | 21 +++- src/crimson/os/seastore/transaction_manager.h | 18 ++- 7 files changed, 172 insertions(+), 65 deletions(-) diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc index e1682f835cdd..d8041d01d9b5 100644 --- a/src/crimson/os/seastore/cache.cc +++ b/src/crimson/os/seastore/cache.cc @@ -95,31 +95,50 @@ Cache::retire_extent_ret Cache::retire_extent_addr( return retire_extent_iertr::now(); } -CachedExtentRef Cache::retire_absent_extent_addr( - Transaction &t, laddr_t laddr, paddr_t paddr, extent_len_t length) +CachedExtentRef Cache::retire_absent_extent_addr_by_type( + Transaction &t, + laddr_t laddr, + paddr_t addr, + extent_len_t length, + extent_types_t type, + extent_init_func_t &&extent_init_func) { - assert(paddr.is_absolute()); - - CachedExtentRef ext; -#ifndef NDEBUG - auto result = t.get_extent(paddr, &ext); - assert(result != Transaction::get_extent_ret::PRESENT - && result != Transaction::get_extent_ret::RETIRED); - assert(!query_cache(paddr)); -#endif - LOG_PREFIX(Cache::retire_absent_extent_addr); - // add a new placeholder to Cache - ext = CachedExtent::make_cached_extent_ref< - RetiredExtentPlaceholder>(length); - ext->init( - CachedExtent::extent_state_t::CLEAN, paddr, - PLACEMENT_HINT_NULL, NULL_GENERATION, TRANS_ID_NULL); - static_cast(*ext).set_laddr(laddr); - DEBUGT("retire {}~0x{:x} as placeholder, add extent -- {}", - t, paddr, length, *ext); - add_extent(ext); - t.add_absent_to_retired_set(ext); - return ext; + switch (type) { + case extent_types_t::ROOT_META: + return retire_absent_extent_addr( + t, laddr, addr, length, std::move(extent_init_func)); + case extent_types_t::ONODE_BLOCK_STAGED: + return retire_absent_extent_addr( + t, laddr, addr, length, std::move(extent_init_func)); + case extent_types_t::OMAP_INNER: + return retire_absent_extent_addr( + t, laddr, addr, length, std::move(extent_init_func)); + case extent_types_t::OMAP_LEAF: + return retire_absent_extent_addr( + t, laddr, addr, length, std::move(extent_init_func)); + case extent_types_t::COLL_BLOCK: + return retire_absent_extent_addr( + t, laddr, addr, length, std::move(extent_init_func)); + case extent_types_t::TEST_BLOCK_PHYSICAL: + return retire_absent_extent_addr( + t, laddr, addr, length, std::move(extent_init_func)); + case extent_types_t::LOG_NODE: + return retire_absent_extent_addr( + t, laddr, addr, length, std::move(extent_init_func)); + case extent_types_t::OBJECT_DATA_BLOCK: + return retire_absent_extent_addr( + t, laddr, addr, length, std::move(extent_init_func)); + case extent_types_t::TEST_BLOCK: + return retire_absent_extent_addr( + t, laddr, addr, length, std::move(extent_init_func)); + case extent_types_t::NONE: { + ceph_assert(0 == "NONE is an invalid extent type"); + return CachedExtentRef(); + } + default: + ceph_assert(0 == "impossible"); + return CachedExtentRef(); + } } void Cache::dump_contents() diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index 3a9883ccdf81..b900b3b6cb9d 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -160,8 +160,22 @@ public: retire_extent_ret retire_extent_addr( Transaction &t, paddr_t addr, extent_len_t length); - CachedExtentRef retire_absent_extent_addr( - Transaction &t, laddr_t laddr, paddr_t addr, extent_len_t length); + template + TCachedExtentRef retire_absent_extent_addr( + Transaction &t, + laddr_t laddr, + paddr_t paddr, + extent_len_t length, + Func &&extent_init_func) { + LOG_PREFIX(Cache::retire_absent_extent_addr); + SUBDEBUGT(seastore_cache, "retire {}~0x{:x} laddr={}", + t, paddr, length, laddr); + auto ext = alloc_absent_extent( + t, paddr, length, 0, length, std::move(extent_init_func)); + SUBDEBUGT(seastore_cache, "retire {}", t, *ext); + retire_extent(t, ext); + return ext; + } /** * get_root @@ -392,41 +406,19 @@ public: extent_len_t partial_off, extent_len_t partial_len, Func &&extent_init_func) { - LOG_PREFIX(Cached::prepare_absent_extent); - -#ifndef NDEBUG - { - CachedExtentRef ret; - auto r = t.get_extent(offset, &ret); - if (r != Transaction::get_extent_ret::ABSENT) { - SUBERRORT(seastore_cache, "unexpected non-absent extent {}", t, *ret); - ceph_abort(); - } - } -#endif - - SUBTRACET(seastore_cache, "{} {}~0x{:x}", t, T::TYPE, offset, length); - ceph_assert(!booting); + auto ret = alloc_absent_extent( + t, + offset, + length, + partial_off, + partial_len, + std::forward(extent_init_func)); const auto t_src = t.get_src(); - - // partial read - TCachedExtentRef ret = CachedExtent::make_cached_extent_ref(length); - ret->init(CachedExtent::extent_state_t::CLEAN, - offset, - PLACEMENT_HINT_NULL, - NULL_GENERATION, - TRANS_ID_NULL); - SUBDEBUGT(seastore_cache, - "{} {}~0x{:x} is absent, add extent and reading range 0x{:x}~0x{:x} ... -- {}", - t, T::TYPE, offset, length, partial_off, partial_len, *ret); - add_extent(ret); - extent_init_func(*ret); cache_access_stats_t& access_stats = get_by_ext( get_by_src(stats.access_by_src_ext, t_src), T::TYPE); ++access_stats.load_absent; ++stats.access.load_absent; - t.add_to_read_set(CachedExtentRef(ret)); touch_extent_by_range( *ret, &t_src, t.get_cache_hint(), partial_off, partial_len); @@ -832,6 +824,47 @@ private: } } + /** + */ + template + TCachedExtentRef alloc_absent_extent( + Transaction &t, + paddr_t offset, + extent_len_t length, + extent_len_t partial_off, + extent_len_t partial_len, + Func &&extent_init_func) { + LOG_PREFIX(Cache::alloc_absent_extent); +#ifndef NDEBUG + { + CachedExtentRef ret; + auto r = t.get_extent(offset, &ret); + if (r != Transaction::get_extent_ret::ABSENT) { + SUBERRORT(seastore_cache, "unexpected non-absent extent {}", t, *ret); + ceph_abort(); + } + } +#endif + + SUBTRACET(seastore_cache, "{} {}~0x{:x}", t, T::TYPE, offset, length); + ceph_assert(!booting); + + // partial read + TCachedExtentRef ret = CachedExtent::make_cached_extent_ref(length); + ret->init(CachedExtent::extent_state_t::CLEAN, + offset, + PLACEMENT_HINT_NULL, + NULL_GENERATION, + TRANS_ID_NULL); + SUBDEBUGT(seastore_cache, + "{} {}~0x{:x} is absent, add extent and reading range 0x{:x}~0x{:x} ... -- {}", + t, T::TYPE, offset, length, partial_off, partial_len, *ret); + add_extent(ret); + extent_init_func(*ret); + t.add_to_read_set(ret); + return ret; + } + /** * do_get_caching_extent * @@ -1306,6 +1339,19 @@ public: rewrite_gen_t gen ///< [in] rewrite generation ); + /** + * retire_absent_extent_addr_by_type + * + * Construct a fresh extent, and add it to the retired_set of the transaction. + */ + CachedExtentRef retire_absent_extent_addr_by_type( + Transaction &t, + laddr_t laddr, + paddr_t addr, + extent_len_t length, + extent_types_t type, + extent_init_func_t &&extent_init_func); + /** * Allocates mutable buffer from extent_set on offset~len * diff --git a/src/crimson/os/seastore/lba/btree_lba_manager.cc b/src/crimson/os/seastore/lba/btree_lba_manager.cc index b132257b05bd..c836b5d3b7f8 100644 --- a/src/crimson/os/seastore/lba/btree_lba_manager.cc +++ b/src/crimson/os/seastore/lba/btree_lba_manager.cc @@ -342,7 +342,7 @@ BtreeLBAManager::clone_mapping( inter_key, EXTENT_DEFAULT_REF_COUNT, 0, - extent_types_t::NONE}, + mapping->get_extent_type()}, get_reserved_ptr()); auto &[iter, inserted] = p; co_await mapping->refresh(); diff --git a/src/crimson/os/seastore/lba/lba_btree_node.h b/src/crimson/os/seastore/lba/lba_btree_node.h index c42be7c1ba88..0a5f56b62cab 100644 --- a/src/crimson/os/seastore/lba/lba_btree_node.h +++ b/src/crimson/os/seastore/lba/lba_btree_node.h @@ -442,6 +442,12 @@ struct LBACursor : BtreeCursor { return iter.get_val().refcount; } + extent_types_t get_extent_type() const { + assert(is_viewable()); + assert(!is_end()); + return iter.get_val().type; + } + base_iertr::future<> refresh(); private: diff --git a/src/crimson/os/seastore/lba_mapping.h b/src/crimson/os/seastore/lba_mapping.h index fbfa10ac8bdd..5ef5f8a62b8c 100644 --- a/src/crimson/os/seastore/lba_mapping.h +++ b/src/crimson/os/seastore/lba_mapping.h @@ -175,6 +175,21 @@ public: extent_len_t>(get_intermediate_key()); } + extent_types_t get_extent_type() const { + if (direct_cursor && indirect_cursor) { + assert(direct_cursor->get_extent_type() + == indirect_cursor->get_extent_type()); + } + if (direct_cursor) { + return direct_cursor->get_extent_type(); + } else if (indirect_cursor) { + return indirect_cursor->get_extent_type(); + } else { + ceph_abort("invalid LBAMapping"); + return extent_types_t::NONE; + } + } + get_child_ret_t get_logical_extent(Transaction &t) const; diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc index 147a2c48b7ff..720719139faf 100644 --- a/src/crimson/os/seastore/transaction_manager.cc +++ b/src/crimson/os/seastore/transaction_manager.cc @@ -297,12 +297,23 @@ TransactionManager::_remove( ceph_assert(extent); cache->retire_extent(t, std::move(extent)); } else { - auto retired_placeholder = cache->retire_absent_extent_addr( - t, mapping.get_intermediate_base(), + auto &child_pos = maybe_mapped_extent.get_child_pos(); + auto laddr = mapping.get_intermediate_base(); + std::ignore = cache->retire_absent_extent_addr_by_type( + t, laddr, mapping.get_val(), - mapping.get_intermediate_length() - )->template cast(); - maybe_mapped_extent.get_child_pos().link_child(retired_placeholder.get()); + mapping.get_intermediate_length(), + mapping.get_extent_type(), + [this, &child_pos, laddr, &t](auto &extent) mutable { + auto lextent = extent.template cast(); + assert(extent.is_logical()); + assert(!lextent->has_laddr()); + assert(!extent.has_been_invalidated()); + child_pos.link_child(lextent.get()); + child_pos.invalidate_retired_placeholder(t, *cache, extent); + lextent->set_laddr(laddr); + } + ); } } diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index 4032c400927d..69fcee33df3b 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -1410,10 +1410,20 @@ private: } } else { SUBTRACET(seastore_tm, "retire extent place holder...", t); - auto retired_placeholder = cache->retire_absent_extent_addr( - t, pin.get_key(), original_paddr, original_len - )->template cast(); - ret.get_child_pos().link_child(retired_placeholder.get()); + auto &child_pos = ret.get_child_pos(); + auto laddr = pin.get_key(); + std::ignore = cache->retire_absent_extent_addr_by_type( + t, laddr, original_paddr, original_len, pin.get_extent_type(), + [this, &child_pos, laddr, &t](auto &extent) mutable { + auto lextent = extent.template cast(); + assert(extent.is_logical()); + assert(!lextent->has_laddr()); + assert(!extent.has_been_invalidated()); + child_pos.link_child(lextent.get()); + child_pos.invalidate_retired_placeholder(t, *cache, extent); + lextent->set_laddr(laddr); + } + ); } } -- 2.47.3