From fa6ed28eddedff368bb77447cefac64b196f6bf8 Mon Sep 17 00:00:00 2001 From: Xuehan Xu Date: Mon, 21 Apr 2025 14:39:08 +0800 Subject: [PATCH] crimson/os/seastore: remove child_pos in LBAMapping Signed-off-by: Xuehan Xu --- .../lba_manager/btree/btree_lba_manager.cc | 6 +---- .../lba_manager/btree/btree_lba_manager.h | 6 +---- src/crimson/os/seastore/lba_mapping.h | 7 ----- src/crimson/os/seastore/transaction_manager.h | 27 +++++++++++++------ 4 files changed, 21 insertions(+), 25 deletions(-) diff --git a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc index f4c36b30312d8..3dacacbc2590d 100644 --- a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc +++ b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc @@ -99,11 +99,7 @@ BtreeLBAMapping::get_logical_extent(Transaction &t) auto k = this->is_indirect() ? this->get_intermediate_base() : get_key(); - auto v = p.template get_child(ctx.trans, ctx.cache, pos, k); - if (!v.has_child()) { - this->child_pos = v.get_child_pos(); - } - return v; + return p.template get_child(ctx.trans, ctx.cache, pos, k); } bool BtreeLBAMapping::is_stable() const diff --git a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.h b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.h index 3dac0e5afe327..32261b6e79f42 100644 --- a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.h +++ b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.h @@ -84,11 +84,7 @@ public: raw_val(val.pladdr), map_val(val), parent_modifications(parent->modifications) - { - if (!parent->is_pending()) { - this->child_pos = {parent, pos}; - } - } + {} lba_map_val_t get_map_val() const { return map_val; diff --git a/src/crimson/os/seastore/lba_mapping.h b/src/crimson/os/seastore/lba_mapping.h index 1f3d4d53233d9..ec79c57da43ca 100644 --- a/src/crimson/os/seastore/lba_mapping.h +++ b/src/crimson/os/seastore/lba_mapping.h @@ -84,10 +84,6 @@ public: virtual get_child_ret_t get_logical_extent(Transaction &t) = 0; - void link_child(LogicalChildNode *c) { - ceph_assert(child_pos); - child_pos->link_child(c); - } virtual LBAMappingRef refresh_with_pending_parent() = 0; // For reserved mappings, the return values are @@ -112,9 +108,6 @@ protected: extent_len_t len = 0; fixed_kv_node_meta_t range; uint16_t pos = std::numeric_limits::max(); - - std::optional> child_pos = std::nullopt; }; std::ostream &operator<<(std::ostream &out, const LBAMapping &rhs); diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index dd60fb8ac3cb1..4fffcbc88f44a 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -311,8 +311,9 @@ public: return std::move(extent); }); } else { + auto &r = std::get<0>(ret); return this->pin_to_extent( - t, std::move(std::get<0>(ret)), + t, std::move(r.mapping), std::move(r.child_pos), direct_partial_off, partial_len, std::move(maybe_init)); } @@ -981,8 +982,13 @@ private: shard_stats_t& shard_stats; + using LBALeafNode = lba_manager::btree::LBALeafNode; + struct unlinked_child_t { + LBAMappingRef mapping; + child_pos_t child_pos; + }; template - std::variant> + std::variant> get_extent_if_linked( Transaction &t, LBAMappingRef pin) @@ -1005,7 +1011,9 @@ private: return extent->template cast(); }); } else { - return pin; + return unlinked_child_t{ + std::move(pin), + v.get_child_pos()}; } } @@ -1027,7 +1035,7 @@ private: return ext; }); } else { - return pin_to_extent_by_type(t, std::move(pin), type); + return pin_to_extent_by_type(t, std::move(pin), v.get_child_pos(), type); } } @@ -1059,6 +1067,7 @@ private: pin_to_extent_ret pin_to_extent( Transaction &t, LBAMappingRef pin, + child_pos_t child_pos, extent_len_t direct_partial_off, extent_len_t partial_len, lextent_init_func_t &&maybe_init) { @@ -1083,14 +1092,15 @@ private: direct_length, direct_partial_off, partial_len, - [&pref, maybe_init=std::move(maybe_init)] + [&pref, maybe_init=std::move(maybe_init), + child_pos=std::move(child_pos)] (T &extent) mutable { assert(extent.is_logical()); assert(!extent.has_laddr()); assert(!extent.has_been_invalidated()); assert(!pref.has_been_invalidated()); assert(pref.get_parent()); - pref.link_child(&extent); + child_pos.link_child(&extent); extent.maybe_set_intermediate_laddr(pref); maybe_init(extent); extent.set_seen_by_users(); @@ -1140,6 +1150,7 @@ private: pin_to_extent_by_type_ret pin_to_extent_by_type( Transaction &t, LBAMappingRef pin, + child_pos_t child_pos, extent_types_t type) { LOG_PREFIX(TransactionManager::pin_to_extent_by_type); @@ -1163,7 +1174,7 @@ private: pref.get_val(), direct_key, direct_length, - [&pref](CachedExtent &extent) mutable { + [&pref, child_pos=std::move(child_pos)](CachedExtent &extent) mutable { assert(extent.is_logical()); auto &lextent = static_cast(extent); assert(!lextent.has_laddr()); @@ -1171,7 +1182,7 @@ private: assert(!pref.has_been_invalidated()); assert(pref.get_parent()); assert(!pref.get_parent()->is_pending()); - pref.link_child(&lextent); + child_pos.link_child(&lextent); lextent.maybe_set_intermediate_laddr(pref); // No change to extent::seen_by_user because this path is only // for background cleaning. -- 2.39.5