From 2a42e2047ba96699796cae9b07f81b77fd6373e9 Mon Sep 17 00:00:00 2001 From: Xuehan Xu Date: Tue, 11 Jun 2024 13:00:49 +0800 Subject: [PATCH] crimson/os/seastore/btree: add interfaces to check whether the mappings' parents have been modified Signed-off-by: Xuehan Xu (cherry picked from commit 10c0f2ab0af4aff8ef182b95830e25d036fe2e3f) --- .../os/seastore/btree/btree_range_pin.cc | 4 ++-- src/crimson/os/seastore/btree/fixed_kv_node.h | 19 ++++++++++++++++++- src/crimson/os/seastore/cached_extent.h | 4 ++++ .../lba_manager/btree/btree_lba_manager.h | 18 ++++++++++++++++-- .../lba_manager/btree/lba_btree_node.cc | 1 + .../lba_manager/btree/lba_btree_node.h | 3 +++ src/crimson/os/seastore/transaction_manager.h | 2 +- 7 files changed, 45 insertions(+), 6 deletions(-) diff --git a/src/crimson/os/seastore/btree/btree_range_pin.cc b/src/crimson/os/seastore/btree/btree_range_pin.cc index ac04705aaf067..ad6abdf4ee80f 100644 --- a/src/crimson/os/seastore/btree/btree_range_pin.cc +++ b/src/crimson/os/seastore/btree/btree_range_pin.cc @@ -28,7 +28,7 @@ BtreeNodeMapping::get_logical_extent( template bool BtreeNodeMapping::is_stable() const { - assert(is_parent_valid()); + assert(!this->parent_modified()); assert(pos != std::numeric_limits::max()); auto &p = (FixedKVNode&)*parent; return p.is_child_stable(ctx, pos); @@ -37,7 +37,7 @@ bool BtreeNodeMapping::is_stable() const template bool BtreeNodeMapping::is_data_stable() const { - assert(is_parent_valid()); + assert(!this->parent_modified()); assert(pos != std::numeric_limits::max()); auto &p = (FixedKVNode&)*parent; return p.is_child_data_stable(ctx, pos); diff --git a/src/crimson/os/seastore/btree/fixed_kv_node.h b/src/crimson/os/seastore/btree/fixed_kv_node.h index 79495cb35d129..ea61d6b9933d2 100644 --- a/src/crimson/os/seastore/btree/fixed_kv_node.h +++ b/src/crimson/os/seastore/btree/fixed_kv_node.h @@ -1004,14 +1004,29 @@ struct FixedKVLeafNode node_layout_t(this->get_bptr().c_str()) {} FixedKVLeafNode(const FixedKVLeafNode &rhs) : FixedKVNode(rhs), - node_layout_t(this->get_bptr().c_str()) {} + node_layout_t(this->get_bptr().c_str()), + modifications(rhs.modifications) {} 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) + uint64_t modifications = 0; + bool have_children() const final { return do_has_children; } + void on_modify() { + modifications++; + } + + bool modified_since(uint64_t v) const { + ceph_assert(v <= modifications); + return v != modifications; + } + bool is_leaf_and_has_children() const final { return has_children; } @@ -1108,6 +1123,7 @@ struct FixedKVLeafNode this->copy_sources.clear(); } } + modifications = 0; assert(this->is_initial_pending() ? this->copy_sources.empty(): true); @@ -1129,6 +1145,7 @@ struct FixedKVLeafNode } else { this->set_parent_tracker_from_prior_instance(); } + modifications = 0; } uint16_t lower_bound_offset(NODE_KEY key) const final { diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h index 5c68207e80ca6..d26d61f25a229 100644 --- a/src/crimson/os/seastore/cached_extent.h +++ b/src/crimson/os/seastore/cached_extent.h @@ -1152,6 +1152,10 @@ public: return !get_val().is_real(); } virtual bool is_parent_valid() const = 0; + virtual bool parent_modified() const { + ceph_abort("impossible"); + return false; + }; virtual ~PhysicalNodeMapping() {} protected: 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 43807efb5fcf9..ac2274676db59 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 @@ -62,7 +62,7 @@ public: : BtreeNodeMapping(ctx) {} BtreeLBAMapping( op_context_t c, - CachedExtentRef parent, + LBALeafNodeRef parent, uint16_t pos, lba_map_val_t &val, lba_node_meta_t meta) @@ -78,7 +78,8 @@ public: intermediate_key(indirect ? val.pladdr.get_laddr() : L_ADDR_NULL), intermediate_length(indirect ? val.len : 0), raw_val(val.pladdr), - map_val(val) + map_val(val), + parent_modifications(parent->modifications) {} lba_map_val_t get_map_val() const { @@ -154,6 +155,17 @@ public: len = length; } + uint64_t get_parent_modifications() const { + return parent_modifications; + } + + bool parent_modified() const final { + ceph_assert(parent); + ceph_assert(is_parent_valid()); + auto &p = static_cast(*parent); + return p.modified_since(parent_modifications); + } + protected: std::unique_ptr> _duplicate( op_context_t ctx) const final { @@ -165,6 +177,7 @@ protected: pin->indirect = indirect; pin->raw_val = raw_val; pin->map_val = map_val; + pin->parent_modifications = parent_modifications; return pin; } private: @@ -175,6 +188,7 @@ private: extent_len_t intermediate_length = 0; pladdr_t raw_val; lba_map_val_t map_val; + uint64_t parent_modifications = 0; }; using BtreeLBAMappingRef = std::unique_ptr; 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 66dc94394a99e..730da546cb0e2 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 @@ -31,6 +31,7 @@ std::ostream &LBALeafNode::_print_detail(std::ostream &out) const { out << ", size=" << this->get_size() << ", meta=" << this->get_meta() + << ", modifications=" << this->modifications << ", my_tracker=" << (void*)this->my_tracker; if (this->my_tracker) { out << ", my_tracker->parent=" << (void*)this->my_tracker->get_parent().get(); 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 c5da860e24ff8..ff06bf81039d4 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 @@ -202,6 +202,7 @@ struct LBALeafNode assert(nextent->has_parent_tracker() && nextent->get_parent_node().get() == this); } + this->on_modify(); if (val.pladdr.is_paddr()) { val.pladdr = maybe_generate_relative(val.pladdr.get_paddr()); } @@ -222,6 +223,7 @@ struct LBALeafNode iter.get_offset(), addr, (void*)nextent); + this->on_modify(); this->insert_child_ptr(iter, nextent); if (val.pladdr.is_paddr()) { val.pladdr = maybe_generate_relative(val.pladdr.get_paddr()); @@ -241,6 +243,7 @@ struct LBALeafNode iter.get_offset(), iter.get_key()); assert(iter != this->end()); + this->on_modify(); this->remove_child_ptr(iter); return this->journal_remove( iter, diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index f6bbc8aa4152b..1fbdcd21fd065 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -221,7 +221,7 @@ public: LBAMappingRef pin, extent_types_t type) { - ceph_assert(pin->is_parent_valid()); + ceph_assert(!pin->parent_modified()); auto v = pin->get_logical_extent(t); // checking the lba child must be atomic with creating // and linking the absent child -- 2.39.5