From 15cb6c7682cf4d9b0bef51591700b4d3eaaa5579 Mon Sep 17 00:00:00 2001 From: Xuehan Xu Date: Mon, 21 Jul 2025 20:01:18 +0800 Subject: [PATCH] crimson/os/seastore/transaction_manager: invalidate RetiredExtentPlaceholder when reading extents Signed-off-by: Xuehan Xu --- src/crimson/os/seastore/cache.h | 15 +++++- src/crimson/os/seastore/linked_tree_node.h | 54 +++++++++++++++++-- src/crimson/os/seastore/logical_child_node.h | 6 +++ src/crimson/os/seastore/transaction_manager.h | 8 ++- 4 files changed, 75 insertions(+), 8 deletions(-) diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index 97cc7b6568a93..1259f52eac0de 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -102,7 +102,8 @@ class SegmentProvider; * - TRACE: DEBUG details * - seastore_t logs */ -class Cache : public ExtentTransViewRetriever { +class Cache : public ExtentTransViewRetriever, + public RetiredExtentPlaceholderInvalidater { public: using base_ertr = crimson::errorator< crimson::ct_error::input_output_error>; @@ -369,6 +370,18 @@ public: } } + void invalidate_retired_placeholder( + Transaction &t, + CachedExtent &retired_placeholder, + CachedExtent &extent) { + // replace placeholder in transactions + while (!retired_placeholder.read_transactions.empty()) { + auto t = retired_placeholder.read_transactions.begin()->t; + t->replace_placeholder(retired_placeholder, extent); + } + retired_placeholder.set_invalid(t); + } + /* * get_absent_extent * diff --git a/src/crimson/os/seastore/linked_tree_node.h b/src/crimson/os/seastore/linked_tree_node.h index 888ecb5d6bf17..308f53ff8dc93 100644 --- a/src/crimson/os/seastore/linked_tree_node.h +++ b/src/crimson/os/seastore/linked_tree_node.h @@ -9,11 +9,27 @@ namespace crimson::os::seastore { +struct RetiredExtentPlaceholderInvalidater { + virtual void invalidate_retired_placeholder( + Transaction &t, + CachedExtent &retired_placeholder, + CachedExtent &extent) = 0; +}; + template class child_pos_t { public: child_pos_t(TCachedExtentRef stable_parent, btreenode_pos_t pos) : stable_parent(stable_parent), pos(pos) {} + child_pos_t( + TCachedExtentRef stable_parent, + btreenode_pos_t pos, + CachedExtent* placeholder) + : stable_parent(stable_parent), + pos(pos), + retired_placeholder(placeholder) { + assert(retired_placeholder->is_placeholder()); + } TCachedExtentRef get_parent() { ceph_assert(stable_parent); @@ -26,9 +42,18 @@ public: void link_child(ChildT *c) { get_parent()->link_child(c, pos); } + void invalidate_retired_placeholder( + Transaction &t, + RetiredExtentPlaceholderInvalidater &repi, + CachedExtent &extent) { + if (retired_placeholder) { + repi.invalidate_retired_placeholder(t, *retired_placeholder, extent); + } + } private: TCachedExtentRef stable_parent; btreenode_pos_t pos = std::numeric_limits::max(); + CachedExtentRef retired_placeholder; }; using get_child_iertr = trans_iertrget_parent(); } virtual key_t node_begin() const = 0; + virtual bool is_retired_placeholder() const = 0; protected: parent_tracker_ref parent_tracker; virtual bool _is_valid() const = 0; @@ -328,15 +354,26 @@ public: auto child = children[pos]; ceph_assert(!is_reserved_ptr(child)); if (is_valid_child_ptr(child)) { - return etvr.get_extent_viewable_by_trans( - t, static_cast(child)); + if (child->is_retired_placeholder()) { + assert(me.is_stable()); + return child_pos_t( + &me, pos, dynamic_cast(child)); + } else { + return etvr.get_extent_viewable_by_trans( + t, static_cast(child)); + } } else if (me.is_pending()) { auto &sparent = me.get_stable_for_key(key); auto spos = sparent.lower_bound(key).get_offset(); auto child = sparent.children[spos]; if (is_valid_child_ptr(child)) { - return etvr.get_extent_viewable_by_trans( - t, static_cast(child)); + if (child->is_retired_placeholder()) { + return child_pos_t( + &sparent, spos, dynamic_cast(child)); + } else { + return etvr.get_extent_viewable_by_trans( + t, static_cast(child)); + } } else { return child_pos_t(&sparent, spos); } @@ -352,7 +389,10 @@ public: assert(child); ceph_assert(me.is_stable()); assert(child->_is_stable()); - assert(!children[pos]); + if (unlikely(children[pos] != nullptr)) { + assert(is_valid_child_ptr(children[pos])); + assert(children[pos]->is_retired_placeholder()); + } ceph_assert(is_valid_child_ptr(child)); update_child_ptr(pos, child); } @@ -1034,6 +1074,10 @@ public: } } + bool is_retired_placeholder() const final { + auto &me = down_cast(); + return me.is_placeholder(); + } protected: void on_invalidated() { this->reset_parent_tracker(); diff --git a/src/crimson/os/seastore/logical_child_node.h b/src/crimson/os/seastore/logical_child_node.h index 71e6ee0ed136b..722060d8b7a6d 100644 --- a/src/crimson/os/seastore/logical_child_node.h +++ b/src/crimson/os/seastore/logical_child_node.h @@ -25,6 +25,8 @@ class RetiredExtentPlaceholder : public CachedExtent, public ChildNode { + using lba_child_node_t = + ChildNode; public: RetiredExtentPlaceholder(extent_len_t length) : CachedExtent(CachedExtent::retired_placeholder_construct_t{}, length) {} @@ -35,6 +37,10 @@ public: } } + void on_invalidated(Transaction&) final { + this->lba_child_node_t::on_invalidated(); + } + CachedExtentRef duplicate_for_write(Transaction&) final { ceph_abort_msg("Should never happen for a placeholder"); return CachedExtentRef(); diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index 98f0d01881830..9bfb310cce1f8 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -1445,12 +1445,14 @@ private: partial_len, [laddr=pin.get_intermediate_base(), maybe_init=std::move(maybe_init), - child_pos=std::move(child_pos)] + child_pos=std::move(child_pos), + &t, this] (T &extent) mutable { assert(extent.is_logical()); assert(!extent.has_laddr()); assert(!extent.has_been_invalidated()); child_pos.link_child(&extent); + child_pos.invalidate_retired_placeholder(t, *cache, extent); extent.set_laddr(laddr); maybe_init(extent); extent.set_seen_by_users(); @@ -1517,12 +1519,14 @@ private: pin.get_val(), direct_key, direct_length, - [direct_key, child_pos=std::move(child_pos)](CachedExtent &extent) mutable { + [direct_key, child_pos=std::move(child_pos), + &t, this](CachedExtent &extent) mutable { assert(extent.is_logical()); auto &lextent = static_cast(extent); assert(!lextent.has_laddr()); assert(!lextent.has_been_invalidated()); child_pos.link_child(&lextent); + child_pos.invalidate_retired_placeholder(t, *cache, lextent); lextent.set_laddr(direct_key); // No change to extent::seen_by_user because this path is only // for background cleaning. -- 2.39.5