From: Xuehan Xu Date: Mon, 13 Jan 2025 07:37:15 +0000 (+0800) Subject: crimson/os/seastore: only LBALeafnodes take logical extents' pointers as X-Git-Tag: v20.0.0~123^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=90101c75f258adc0d7e214a6a60e9ecd8d533c16;p=ceph.git crimson/os/seastore: only LBALeafnodes take logical extents' pointers as update parameters BackrefLeafNodes don't any more. Signed-off-by: Xuehan Xu --- diff --git a/src/crimson/os/seastore/backref/backref_tree_node.h b/src/crimson/os/seastore/backref/backref_tree_node.h index 0d4f8406960..742d5074a05 100644 --- a/src/crimson/os/seastore/backref/backref_tree_node.h +++ b/src/crimson/os/seastore/backref/backref_tree_node.h @@ -110,9 +110,7 @@ class BackrefLeafNode backref_map_val_t, backref_map_val_le_t, BACKREF_NODE_SIZE, BackrefInternalNode, - BackrefLeafNode, - LogicalChildNode, - false> { + BackrefLeafNode> { static_assert( check_capacity(BACKREF_NODE_SIZE), "LEAF_NODE_CAPACITY doesn't fit in BACKREF_NODE_SIZE"); @@ -131,8 +129,7 @@ public: const_iterator insert( const_iterator iter, paddr_t key, - backref_map_val_t val, - LogicalChildNode*) final { + backref_map_val_t val) final { journal_insert( iter, key, @@ -143,8 +140,7 @@ public: void update( const_iterator iter, - backref_map_val_t val, - LogicalChildNode*) final { + backref_map_val_t val) final { return journal_update( iter, val, @@ -157,6 +153,46 @@ public: maybe_get_delta_buffer()); } + void do_on_rewrite(Transaction &t, CachedExtent &extent) final {} + void do_on_replace_prior() final {} + void do_prepare_commit() final {} + + + void on_split( + Transaction &t, + BackrefLeafNode &left, + BackrefLeafNode &right) final {} + + void on_merge( + Transaction &t, + BackrefLeafNode &left, + BackrefLeafNode &right) final {} + + void on_balance( + Transaction &t, + BackrefLeafNode &left, + BackrefLeafNode &right, + bool prefer_left, + BackrefLeafNode &replacement_left, + BackrefLeafNode &replacement_right) final {} + + void adjust_copy_src_dest_on_split( + Transaction &t, + BackrefLeafNode &left, + BackrefLeafNode &right) final {} + + void adjust_copy_src_dest_on_merge( + Transaction &t, + BackrefLeafNode &left, + BackrefLeafNode &right) final {} + + void adjust_copy_src_dest_on_balance( + Transaction &t, + BackrefLeafNode &left, + BackrefLeafNode &right, + bool prefer_left, + BackrefLeafNode &replacement_left, + BackrefLeafNode &replacement_right) final {} // backref leaf nodes don't have to resolve relative addresses void resolve_relative_addrs(paddr_t base) final {} diff --git a/src/crimson/os/seastore/backref/btree_backref_manager.cc b/src/crimson/os/seastore/backref/btree_backref_manager.cc index 8a30b1ee94d..c021ad92b3f 100644 --- a/src/crimson/os/seastore/backref/btree_backref_manager.cc +++ b/src/crimson/os/seastore/backref/btree_backref_manager.cc @@ -228,8 +228,7 @@ BtreeBackrefManager::new_mapping( c, *state.insert_iter, state.last_end, - val, - nullptr + val ).si_then([&state, c, addr, len, key](auto &&p) { LOG_PREFIX(BtreeBackrefManager::new_mapping); auto [iter, inserted] = std::move(p); diff --git a/src/crimson/os/seastore/backref/btree_backref_manager.h b/src/crimson/os/seastore/backref/btree_backref_manager.h index a747ef6d3bd..8d1839fc12f 100644 --- a/src/crimson/os/seastore/backref/btree_backref_manager.h +++ b/src/crimson/os/seastore/backref/btree_backref_manager.h @@ -33,7 +33,7 @@ constexpr size_t BACKREF_BLOCK_SIZE = 4096; using BackrefBtree = FixedKVBtree< paddr_t, backref_map_val_t, BackrefInternalNode, - BackrefLeafNode, BtreeBackrefMapping, BACKREF_BLOCK_SIZE, false>; + BackrefLeafNode, BtreeBackrefMapping, BACKREF_BLOCK_SIZE>; class BtreeBackrefManager : public BackrefManager { public: diff --git a/src/crimson/os/seastore/btree/fixed_kv_btree.h b/src/crimson/os/seastore/btree/fixed_kv_btree.h index 1dd666e8a24..a635407dcad 100644 --- a/src/crimson/os/seastore/btree/fixed_kv_btree.h +++ b/src/crimson/os/seastore/btree/fixed_kv_btree.h @@ -42,8 +42,7 @@ template < typename internal_node_t, typename leaf_node_t, typename pin_t, - size_t node_size, - bool leaf_has_children> + size_t node_size> class FixedKVBtree { static constexpr size_t MAX_DEPTH = 16; using self_type = FixedKVBtree< @@ -52,8 +51,7 @@ class FixedKVBtree { internal_node_t, leaf_node_t, pin_t, - node_size, - leaf_has_children>; + node_size>; public: using InternalNodeRef = TCachedExtentRef; using LeafNodeRef = TCachedExtentRef; @@ -64,6 +62,8 @@ public: class iterator; using iterator_fut = base_iertr::future; + static constexpr bool leaf_has_children = + std::is_base_of_v, leaf_node_t>; using mapped_space_visitor_t = std::function< void(paddr_t, node_key_t, extent_len_t, depth_t, extent_types_t, iterator&)>; @@ -489,7 +489,6 @@ public: op_context_t c, TCachedExtentRef node) { - assert(leaf_has_children); for (auto i : *node) { CachedExtentRef child_node; Transaction::get_extent_ret ret; @@ -572,12 +571,10 @@ public: { assert(!c.cache.test_query_cache(i->get_val())); } else { - if constexpr (leaf_has_children) { - assert(i->get_val().pladdr.is_paddr() - ? (bool)!c.cache.test_query_cache( - i->get_val().pladdr.get_paddr()) - : true); - } + assert(i->get_val().pladdr.is_paddr() + ? (bool)!c.cache.test_query_cache( + i->get_val().pladdr.get_paddr()) + : true); } if (is_reserved_ptr(child)) { if constexpr( @@ -615,10 +612,8 @@ public: depth_t depth, extent_types_t, iterator& iter) { - if constexpr (!leaf_has_children) { - if (depth == 1) { - return seastar::now(); - } + if (depth == 1) { + return seastar::now(); } if (depth > 1) { auto &node = iter.get_internal(depth).node; @@ -632,7 +627,7 @@ public: assert(depth == 1); auto &node = iter.leaf.node; assert(node->is_valid()); - check_node(c, node); + check_node(c, node); } return seastar::now(); }; @@ -723,8 +718,7 @@ public: op_context_t c, iterator iter, node_key_t laddr, - node_val_t val, - leaf_node_t::base_child_node_t* nextent + node_val_t val ) { LOG_PREFIX(FixedKVBtree::insert); SUBTRACET( @@ -735,10 +729,10 @@ public: iter.is_end() ? min_max_t::max : iter.get_key()); return seastar::do_with( iter, - [this, c, laddr, val, nextent](auto &ret) { + [this, c, laddr, val](auto &ret) { return find_insertion( c, laddr, ret - ).si_then([this, c, laddr, val, &ret, nextent] { + ).si_then([this, c, laddr, val, &ret] { if (!ret.at_boundary() && ret.get_key() == laddr) { return insert_ret( interruptible::ready_future_marker{}, @@ -747,7 +741,7 @@ public: ++(get_tree_stats(c.trans).num_inserts); return handle_split( c, ret - ).si_then([c, laddr, val, &ret, nextent] { + ).si_then([c, laddr, val, &ret] { if (!ret.leaf.node->is_mutable()) { CachedExtentRef mut = c.cache.duplicate_for_write( c.trans, ret.leaf.node @@ -760,7 +754,7 @@ public: assert(iter == ret.leaf.node->end() || iter->get_key() > laddr); assert(laddr >= ret.leaf.node->get_meta().begin && laddr < ret.leaf.node->get_meta().end); - ret.leaf.node->insert(iter, laddr, val, nextent); + ret.leaf.node->insert(iter, laddr, val); return insert_ret( interruptible::ready_future_marker{}, std::make_pair(ret, true)); @@ -773,12 +767,11 @@ public: insert_ret insert( op_context_t c, node_key_t laddr, - node_val_t val, - leaf_node_t::base_child_node_t* nextent) { + node_val_t val) { return lower_bound( c, laddr - ).si_then([this, c, laddr, val, nextent](auto iter) { - return this->insert(c, iter, laddr, val, nextent); + ).si_then([this, c, laddr, val](auto iter) { + return this->insert(c, iter, laddr, val); }); } @@ -797,8 +790,7 @@ public: update_ret update( op_context_t c, iterator iter, - node_val_t val, - leaf_node_t::base_child_node_t* nextent) + node_val_t val) { LOG_PREFIX(FixedKVBtree::update); SUBTRACET( @@ -815,8 +807,7 @@ public: ++(get_tree_stats(c.trans).num_updates); iter.leaf.node->update( iter.leaf.node->iter_idx(iter.leaf.pos), - val, - nextent); + val); return update_ret( interruptible::ready_future_marker{}, iter); @@ -2168,8 +2159,7 @@ template < typename internal_node_t, typename leaf_node_t, typename pin_t, - size_t node_size, - bool leaf_has_children> + size_t node_size> struct is_fixed_kv_tree< FixedKVBtree< node_key_t, @@ -2177,8 +2167,7 @@ struct is_fixed_kv_tree< internal_node_t, leaf_node_t, pin_t, - node_size, - leaf_has_children>> : std::true_type {}; + node_size>> : std::true_type {}; template < typename tree_type_t, diff --git a/src/crimson/os/seastore/btree/fixed_kv_node.h b/src/crimson/os/seastore/btree/fixed_kv_node.h index f593df3bbaa..f381fe2b466 100644 --- a/src/crimson/os/seastore/btree/fixed_kv_node.h +++ b/src/crimson/os/seastore/btree/fixed_kv_node.h @@ -167,10 +167,6 @@ struct FixedKVInternalNode this->set_layout_buf(this->get_bptr().c_str()); } - bool is_leaf_and_has_children() const { - return false; - } - uint16_t get_node_split_pivot() const final{ return this->get_split_pivot().get_offset(); } @@ -485,9 +481,7 @@ template < typename VAL_LE, size_t node_size, typename internal_node_type_t, - typename node_type_t, - typename child_t, - bool has_children> + typename node_type_t> struct FixedKVLeafNode : FixedKVNode, common::FixedKVNodeLayout< @@ -497,7 +491,6 @@ struct FixedKVLeafNode NODE_KEY, NODE_KEY_LE, VAL, VAL_LE>, RootChildNode, - ParentNode, ChildNode { using Ref = TCachedExtentRef; using node_layout_t = @@ -518,26 +511,19 @@ struct FixedKVLeafNode VAL_LE, node_size, internal_node_type_t, - node_type_t, - child_t, - has_children>; + node_type_t>; using base_t = FixedKVNode; - using parent_node_t = ParentNode; - using base_child_node_t = child_t; using child_node_t = ChildNode; using root_node_t = RootChildNode; explicit FixedKVLeafNode(ceph::bufferptr &&ptr) - : FixedKVNode(std::move(ptr)), - ParentNode(has_children ? CAPACITY : 0){ + : FixedKVNode(std::move(ptr)) { this->set_layout_buf(this->get_bptr().c_str()); } // Must be identical with FixedKVLeafNode(ptr) after on_fully_loaded() explicit FixedKVLeafNode(extent_len_t length) - : FixedKVNode(length), - ParentNode(has_children ? CAPACITY : 0) {} + : FixedKVNode(length) {} FixedKVLeafNode(const FixedKVLeafNode &rhs) : FixedKVNode(rhs), - ParentNode(rhs), modifications(rhs.modifications) { this->set_layout_buf(this->get_bptr().c_str()); } @@ -547,7 +533,6 @@ struct FixedKVLeafNode (this->is_btree_root() && this->has_root_parent()); } - static constexpr bool do_has_children = has_children; // for the stable extent, modifications is always 0; // it will increase for each transaction-local change, so that // modifications can be detected (see BtreeLBAMapping.parent_modifications) @@ -565,12 +550,6 @@ struct FixedKVLeafNode } } - void _on_rewrite(Transaction &t, CachedExtent &extent) final { - if (do_has_children) { - this->parent_node_t::on_rewrite(t, static_cast(extent)); - } - } - void on_modify() { modifications++; } @@ -580,27 +559,10 @@ struct FixedKVLeafNode return v != modifications; } - bool is_leaf_and_has_children() const { - return has_children; - } - uint16_t get_node_split_pivot() const final{ return this->get_split_pivot().get_offset(); } - bool is_child_stable( - op_context_t c, - uint16_t pos, - NODE_KEY key) const { - return parent_node_t::_is_child_stable(c.trans, pos, key); - } - bool is_child_data_stable( - op_context_t c, - uint16_t pos, - NODE_KEY key) const { - return parent_node_t::_is_child_stable(c.trans, pos, key, true); - } - virtual ~FixedKVLeafNode() { if (this->is_valid() && !this->is_pending()) { if (this->is_btree_root()) { @@ -615,18 +577,16 @@ struct FixedKVLeafNode this->set_layout_buf(this->get_bptr().c_str()); } + virtual void do_prepare_commit() = 0; void prepare_commit() final { - if constexpr (has_children) { - parent_node_t::prepare_commit(); - } + do_prepare_commit(); modifications = 0; } + virtual void do_on_replace_prior() = 0; void on_replace_prior() final { ceph_assert(!this->is_rewrite()); - if constexpr (has_children) { - this->parent_node_t::on_replace_prior(); - } + do_on_replace_prior(); if (this->is_btree_root()) { this->root_node_t::on_replace_prior(); } else { @@ -658,64 +618,80 @@ struct FixedKVLeafNode CachedExtentRef duplicate_for_write(Transaction&) override { assert(delta_buffer.empty()); - auto extent = new node_type_t(*this); - extent->set_cache_proxy(this->get_cache_proxy()); - return CachedExtentRef(extent); + return CachedExtentRef(new node_type_t(*static_cast(this))); }; virtual void update( internal_const_iterator_t iter, - VAL val, - base_child_node_t* nextent) = 0; + VAL val) = 0; virtual internal_const_iterator_t insert( internal_const_iterator_t iter, NODE_KEY addr, - VAL val, - base_child_node_t* nextent) = 0; + VAL val) = 0; virtual void remove(internal_const_iterator_t iter) = 0; + virtual void on_split( + Transaction &t, + node_type_t &left, + node_type_t &right) = 0; + virtual void adjust_copy_src_dest_on_split( + Transaction &t, + node_type_t &left, + node_type_t &right) = 0; std::tuple make_split_children(op_context_t c) { auto left = c.cache.template alloc_new_non_data_extent( c.trans, node_size, placement_hint_t::HOT, INIT_GENERATION); - left->set_cache_proxy(this->get_cache_proxy()); auto right = c.cache.template alloc_new_non_data_extent( c.trans, node_size, placement_hint_t::HOT, INIT_GENERATION); - if constexpr (has_children) { - this->split_child_ptrs(c.trans, *left, *right); - } - right->set_cache_proxy(this->get_cache_proxy()); + this->on_split(c.trans, *left, *right); auto pivot = this->split_into(*left, *right); left->range = left->get_meta(); right->range = right->get_meta(); - if constexpr (has_children) { - this->adjust_copy_src_dest_on_split(c.trans, *left, *right); - } + this->adjust_copy_src_dest_on_split(c.trans, *left, *right); return std::make_tuple( left, right, pivot); } + virtual void on_merge( + Transaction &t, + node_type_t &left, + node_type_t &right) = 0; + virtual void adjust_copy_src_dest_on_merge( + Transaction &t, + node_type_t &left, + node_type_t &right) = 0; + Ref make_full_merge( op_context_t c, Ref &right) { auto replacement = c.cache.template alloc_new_non_data_extent( c.trans, node_size, placement_hint_t::HOT, INIT_GENERATION); - if constexpr (has_children) { - replacement->merge_child_ptrs( - c.trans, static_cast(*this), *right); - } - replacement->set_cache_proxy(this->get_cache_proxy()); + replacement->on_merge(c.trans, static_cast(*this), *right); replacement->merge_from(*this, *right->template cast()); replacement->range = replacement->get_meta(); - if constexpr (has_children) { - replacement->adjust_copy_src_dest_on_merge( - c.trans, static_cast(*this), *right); - } + replacement->adjust_copy_src_dest_on_merge( + c.trans, static_cast(*this), *right); return replacement; } + virtual void on_balance( + Transaction &t, + node_type_t &left, + node_type_t &right, + bool prefer_left, + node_type_t &replacement_left, + node_type_t &replacement_right) = 0; + virtual void adjust_copy_src_dest_on_balance( + Transaction &t, + node_type_t &left, + node_type_t &right, + bool prefer_left, + node_type_t &replacement_left, + node_type_t &replacement_right) = 0; + std::tuple make_balanced( op_context_t c, @@ -725,20 +701,16 @@ struct FixedKVLeafNode auto &right = *_right->template cast(); auto replacement_left = c.cache.template alloc_new_non_data_extent( c.trans, node_size, placement_hint_t::HOT, INIT_GENERATION); - replacement_left->set_cache_proxy(this->get_cache_proxy()); auto replacement_right = c.cache.template alloc_new_non_data_extent( c.trans, node_size, placement_hint_t::HOT, INIT_GENERATION); - replacement_right->set_cache_proxy(this->get_cache_proxy()); - - if constexpr (has_children) { - this->balance_child_ptrs( - c.trans, - static_cast(*this), - right, - prefer_left, - *replacement_left, - *replacement_right); - } + + this->on_balance( + c.trans, + static_cast(*this), + right, + prefer_left, + *replacement_left, + *replacement_right); auto pivot = this->balance_into_new_nodes( *this, right, @@ -747,15 +719,13 @@ struct FixedKVLeafNode *replacement_right); replacement_left->range = replacement_left->get_meta(); replacement_right->range = replacement_right->get_meta(); - if constexpr (has_children) { - this->adjust_copy_src_dest_on_balance( - c.trans, - static_cast(*this), - right, - prefer_left, - *replacement_left, - *replacement_right); - } + this->adjust_copy_src_dest_on_balance( + c.trans, + static_cast(*this), + right, + prefer_left, + *replacement_left, + *replacement_right); return std::make_tuple( replacement_left, replacement_right, diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index 63e9670b8ce..a916e13e311 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -26,15 +26,6 @@ class BtreeBackrefManager; namespace crimson::os::seastore { -template < - typename node_key_t, - typename node_val_t, - typename internal_node_t, - typename leaf_node_t, - typename pin_t, - size_t node_size, - bool leaf_has_children> -class FixedKVBtree; class BackrefManager; class SegmentProvider; 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 363919f6eed..68008c8b04e 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 @@ -436,14 +436,18 @@ BtreeLBAManager::_alloc_extents( alloc_info.len, pladdr_t(alloc_info.val), refcount, - alloc_info.checksum}, - alloc_info.extent + alloc_info.checksum} ).si_then([&state, c, addr, total_len, hint, FNAME, &alloc_info, &rets](auto &&p) { auto [iter, inserted] = std::move(p); + auto &leaf_node = *iter.get_leaf_node(); + leaf_node.insert_child_ptr( + iter.get_leaf_pos(), + alloc_info.extent, + leaf_node.get_size() - 1 /*the size before the insert*/); TRACET("{}~{}, hint={}, inserted at {}", c.trans, addr, total_len, hint, state.last_end); - if (alloc_info.extent) { + if (is_valid_child_ptr(alloc_info.extent)) { ceph_assert(alloc_info.val.is_paddr()); assert(alloc_info.val == iter.get_val().pladdr); assert(alloc_info.len == iter.get_val().len); @@ -748,7 +752,7 @@ BtreeLBAManager::_decref_intermediate( std::make_optional(res)); }); } else { - return btree.update(c, iter, val, nullptr + return btree.update(c, iter, val ).si_then([](auto) { return ref_iertr::make_ready_future< std::optional>(std::nullopt); @@ -850,9 +854,17 @@ BtreeLBAManager::_update_mapping( return btree.update( c, iter, - ret, - nextent - ).si_then([c, ret](auto iter) { + ret + ).si_then([c, ret, nextent](auto iter) { + // child-ptr may already be correct, + // see LBAManager::update_mappings() + if (nextent && !nextent->has_parent_tracker()) { + iter.get_leaf_node()->update_child_ptr( + iter.get_leaf_pos(), nextent); + } + assert(!nextent || + (nextent->has_parent_tracker() + && nextent->get_parent_node().get() == iter.get_leaf_node().get())); return update_mapping_ret_bare_t{ std::move(ret), iter.get_pin(c) 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 22232b9d97c..50bf2ca5e0c 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 @@ -235,7 +235,7 @@ using BtreeLBAMappingRef = std::unique_ptr; using LBABtree = FixedKVBtree< laddr_t, lba_map_val_t, LBAInternalNode, - LBALeafNode, BtreeLBAMapping, LBA_BLOCK_SIZE, true>; + LBALeafNode, BtreeLBAMapping, LBA_BLOCK_SIZE>; /** * BtreeLBAManager @@ -287,7 +287,12 @@ public: LogicalChildNode* extent = nullptr; static alloc_mapping_info_t create_zero(extent_len_t len) { - return {L_ADDR_NULL, len, P_ADDR_ZERO, 0, nullptr}; + return { + L_ADDR_NULL, + len, + P_ADDR_ZERO, + 0, + static_cast(get_reserved_ptr())}; } static alloc_mapping_info_t create_indirect( laddr_t laddr, @@ -299,7 +304,7 @@ public: intermediate_key, 0, // crc will only be used and checked with LBA direct mappings // also see pin_to_extent(_by_type) - nullptr}; + static_cast(get_reserved_ptr())}; } static alloc_mapping_info_t create_direct( laddr_t laddr, @@ -409,6 +414,7 @@ public: { std::vector alloc_infos; for (auto &extent : extents) { + assert(extent); alloc_infos.emplace_back( alloc_mapping_info_t::create_direct( extent->has_laddr() ? extent->get_laddr() : L_ADDR_NULL, diff --git a/src/crimson/os/seastore/lba_manager/btree/lba_btree_node.cc b/src/crimson/os/seastore/lba_manager/btree/lba_btree_node.cc index fc1f3710041..54439420032 100644 --- a/src/crimson/os/seastore/lba_manager/btree/lba_btree_node.cc +++ b/src/crimson/os/seastore/lba_manager/btree/lba_btree_node.cc @@ -89,22 +89,12 @@ BtreeLBAMappingRef LBALeafNode::get_mapping( void LBALeafNode::update( internal_const_iterator_t iter, - lba_map_val_t val, - LogicalChildNode* nextent) + lba_map_val_t val) { LOG_PREFIX(LBALeafNode::update); - if (nextent) { - SUBTRACE(seastore_fixedkv_tree, "trans.{}, pos {}, {}", - this->pending_for_transaction, - iter.get_offset(), - *nextent); - // child-ptr may already be correct, see LBAManager::update_mappings() - if (!nextent->has_parent_tracker()) { - this->update_child_ptr(iter.get_offset(), nextent); - } - assert(nextent->has_parent_tracker() - && nextent->get_parent_node().get() == this); - } + SUBTRACE(seastore_fixedkv_tree, "trans.{}, pos {}", + this->pending_for_transaction, + iter.get_offset()); this->on_modify(); if (val.pladdr.is_paddr()) { val.pladdr = maybe_generate_relative(val.pladdr.get_paddr()); @@ -118,17 +108,14 @@ void LBALeafNode::update( LBALeafNode::internal_const_iterator_t LBALeafNode::insert( internal_const_iterator_t iter, laddr_t addr, - lba_map_val_t val, - LogicalChildNode* nextent) + lba_map_val_t val) { LOG_PREFIX(LBALeafNode::insert); - SUBTRACE(seastore_fixedkv_tree, "trans.{}, pos {}, key {}, extent {}", + SUBTRACE(seastore_fixedkv_tree, "trans.{}, pos {}, key {}", this->pending_for_transaction, iter.get_offset(), - addr, - (void*)nextent); + addr); this->on_modify(); - this->insert_child_ptr(iter.get_offset(), nextent); if (val.pladdr.is_paddr()) { val.pladdr = maybe_generate_relative(val.pladdr.get_paddr()); } 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 7d8a567435d..c865f6b84a9 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 @@ -153,9 +153,8 @@ struct LBALeafNode lba_map_val_t, lba_map_val_le_t, LBA_BLOCK_SIZE, LBAInternalNode, - LBALeafNode, - LogicalChildNode, - true> { + LBALeafNode>, + ParentNode { static_assert( check_capacity(LBA_BLOCK_SIZE), "LEAF_NODE_CAPACITY doesn't fit in LBA_BLOCK_SIZE"); @@ -166,30 +165,34 @@ struct LBALeafNode lba_map_val_t, lba_map_val_le_t, LBA_BLOCK_SIZE, LBAInternalNode, - LBALeafNode, - LogicalChildNode, - true>; + LBALeafNode>; using internal_const_iterator_t = typename parent_type_t::node_layout_t::const_iterator; using internal_iterator_t = typename parent_type_t::node_layout_t::iterator; using key_type = laddr_t; - template - LBALeafNode(T&&... t) : - parent_type_t(std::forward(t)...) {} + using parent_node_t = ParentNode; + using child_t = LogicalChildNode; + LBALeafNode(ceph::bufferptr &&ptr) + : parent_type_t(std::move(ptr)), + parent_node_t(LEAF_NODE_CAPACITY) {} + explicit LBALeafNode(extent_len_t length) + : parent_type_t(length), + parent_node_t(LEAF_NODE_CAPACITY) {} + LBALeafNode(const LBALeafNode &rhs) + : parent_type_t(rhs), + parent_node_t(rhs) {} static constexpr extent_types_t TYPE = extent_types_t::LADDR_LEAF; void update( internal_const_iterator_t iter, - lba_map_val_t val, - LogicalChildNode* nextent) final; + lba_map_val_t val) final; internal_const_iterator_t insert( internal_const_iterator_t iter, laddr_t addr, - lba_map_val_t val, - LogicalChildNode* nextent) final; + lba_map_val_t val) final; void remove(internal_const_iterator_t iter) final { LOG_PREFIX(LBALeafNode::remove); @@ -244,6 +247,82 @@ struct LBALeafNode return TYPE; } + void do_on_rewrite(Transaction &t, CachedExtent &extent) final { + this->parent_node_t::on_rewrite(t, static_cast(extent)); + } + + void do_on_replace_prior() final { + this->parent_node_t::on_replace_prior(); + } + + void do_prepare_commit() final { + this->parent_node_t::prepare_commit(); + } + + bool is_child_stable( + op_context_t c, + uint16_t pos, + laddr_t key) const { + return parent_node_t::_is_child_stable(c.trans, c.cache, pos, key); + } + bool is_child_data_stable( + op_context_t c, + uint16_t pos, + laddr_t key) const { + return parent_node_t::_is_child_stable(c.trans, c.cache, pos, key, true); + } + + void on_split( + Transaction &t, + LBALeafNode &left, + LBALeafNode &right) final { + this->split_child_ptrs(t, left, right); + } + void adjust_copy_src_dest_on_split( + Transaction &t, + LBALeafNode &left, + LBALeafNode &right) final { + this->parent_node_t::adjust_copy_src_dest_on_split(t, left, right); + } + + void on_merge( + Transaction &t, + LBALeafNode &left, + LBALeafNode &right) final { + this->merge_child_ptrs(t, left, right); + } + void adjust_copy_src_dest_on_merge( + Transaction &t, + LBALeafNode &left, + LBALeafNode &right) final { + this->parent_node_t::adjust_copy_src_dest_on_merge(t, left, right); + } + + void on_balance( + Transaction &t, + LBALeafNode &left, + LBALeafNode &right, + bool prefer_left, + LBALeafNode &replacement_left, + LBALeafNode &replacement_right) final { + this->balance_child_ptrs( + t, left, right, prefer_left, replacement_left, replacement_right); + } + void adjust_copy_src_dest_on_balance( + Transaction &t, + LBALeafNode &left, + LBALeafNode &right, + bool prefer_left, + LBALeafNode &replacement_left, + LBALeafNode &replacement_right) final { + this->parent_node_t::adjust_copy_src_dest_on_balance( + t, left, right, prefer_left, replacement_left, replacement_right); + } + + CachedExtentRef duplicate_for_write(Transaction&) final { + return CachedExtentRef(new LBALeafNode(*this)); + } + std::ostream &print_detail(std::ostream &out) const final; void maybe_fix_mapping_pos(BtreeLBAMapping &mapping); diff --git a/src/crimson/os/seastore/linked_tree_node.h b/src/crimson/os/seastore/linked_tree_node.h index ecda72a11bb..4f8e52bea17 100644 --- a/src/crimson/os/seastore/linked_tree_node.h +++ b/src/crimson/os/seastore/linked_tree_node.h @@ -208,6 +208,13 @@ protected: friend class ParentNode; }; +// this is to avoid mistakenly copying pointers from +// copy sources when committing this lba node, because +// we rely on pointers' "nullness" to avoid copying +// pointers for updated values, and some lba mappings' +// aren't supposed to have children. At present, mappings +// that don't have children are reserved regions and +// indirect mapping. template inline BaseChildNode* get_reserved_ptr() { return (BaseChildNode*)0x1; @@ -355,6 +362,32 @@ public: update_child_ptr(pos, child); } + void insert_child_ptr( + btreenode_pos_t offset, + BaseChildNode* child, + btreenode_pos_t size = 0) + { + assert(child); + auto &me = down_cast(); + if (size == 0) { + size = me.get_size(); + } + auto raw_children = children.data(); + std::memmove( + &raw_children[offset + 1], + &raw_children[offset], + (size - offset) * sizeof(BaseChildNode*)); + children[offset] = child; + if (!is_reserved_ptr(child)) { + set_child_ptracker(child); + } + } + + void update_child_ptr(btreenode_pos_t pos, BaseChildNode* child) { + children[pos] = child; + set_child_ptracker(child); + } + protected: ParentNode(btreenode_pos_t capacity) : children(capacity, nullptr), @@ -388,11 +421,6 @@ protected: copy_dests.dests_by_key.erase(dest); } - void update_child_ptr(btreenode_pos_t pos, BaseChildNode* child) { - children[pos] = child; - set_child_ptracker(child); - } - void set_child_ptracker(BaseChildNode *child) { if (!this->my_tracker) { auto &me = down_cast(); @@ -401,27 +429,6 @@ protected: child->reset_parent_tracker(this->my_tracker); } - void insert_child_ptr(btreenode_pos_t offset, BaseChildNode* child) { - auto &me = down_cast(); - auto raw_children = children.data(); - std::memmove( - &raw_children[offset + 1], - &raw_children[offset], - (me.get_size() - offset) * sizeof(BaseChildNode*)); - if (child) { - children[offset] = child; - set_child_ptracker(child); - } else { - // this can happen when reserving lba spaces and cloning mappings - ceph_assert(me.is_leaf_and_has_children()); - // this is to avoid mistakenly copying pointers from - // copy sources when committing this lba node, because - // we rely on pointers' "nullness" to avoid copying - // pointers for updated values - children[offset] = get_reserved_ptr(); - } - } - void remove_child_ptr(btreenode_pos_t offset) { auto &me = down_cast(); LOG_PREFIX(ParentNode::remove_child_ptr); @@ -943,7 +950,7 @@ private: template friend class child_pos_t; #ifdef UNIT_TESTS_BUILT - template + template friend class FixedKVBtree; #endif }; diff --git a/src/test/crimson/seastore/test_btree_lba_manager.cc b/src/test/crimson/seastore/test_btree_lba_manager.cc index 96965d8a435..2ec8b7b0162 100644 --- a/src/test/crimson/seastore/test_btree_lba_manager.cc +++ b/src/test/crimson/seastore/test_btree_lba_manager.cc @@ -335,9 +335,13 @@ struct lba_btree_test : btree_test_base { extents, [this, addr, len, &t, &btree](auto &extent) { return btree.insert( - get_op_context(t), addr, get_map_val(len), extent.get() + get_op_context(t), addr, get_map_val(len) ).si_then([addr, extent](auto p){ auto& [iter, inserted] = p; + iter.get_leaf_node()->insert_child_ptr( + iter.get_leaf_pos(), + extent.get(), + iter.get_leaf_node()->get_size() - 1); assert(inserted); extent->set_laddr(addr); });