From 513369cd66c8e2035670ac9f1cf63a970e9a3ff8 Mon Sep 17 00:00:00 2001 From: Yingxin Cheng Date: Wed, 21 May 2025 14:21:10 +0800 Subject: [PATCH] crimson/os/seastore: more accurate usages to is_pending() vs is_stable() Generally, prefer is_pending() than !is_stable(), and is_stable() than !is_pending(). Signed-off-by: Yingxin Cheng --- .../os/seastore/btree/fixed_kv_btree.h | 12 +++++------ src/crimson/os/seastore/btree/fixed_kv_node.h | 4 ++-- src/crimson/os/seastore/cached_extent.h | 9 ++++++--- .../os/seastore/lba/btree_lba_manager.cc | 2 +- src/crimson/os/seastore/linked_tree_node.h | 20 +++++++++---------- src/crimson/os/seastore/logical_child_node.h | 3 +-- .../omap_manager/btree/omap_btree_node_impl.h | 6 ++---- 7 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/crimson/os/seastore/btree/fixed_kv_btree.h b/src/crimson/os/seastore/btree/fixed_kv_btree.h index ab12ab3ee6a..fef9a03114d 100644 --- a/src/crimson/os/seastore/btree/fixed_kv_btree.h +++ b/src/crimson/os/seastore/btree/fixed_kv_btree.h @@ -1192,7 +1192,7 @@ private: parent_pos=std::move(parent_pos)] (internal_node_t &node) { using tree_root_linker_t = TreeRootLinker; - assert(!node.is_pending()); + assert(node.is_stable()); assert(!node.is_linked()); node.range = fixed_kv_node_meta_t{begin, end, depth}; if (parent_pos) { @@ -1205,7 +1205,7 @@ private: auto &stable_root = (RootBlockRef&)*root_block->get_prior_instance(); tree_root_linker_t::link_root(stable_root, &node); } else { - assert(!root_block->is_pending()); + assert(root_block->is_stable()); tree_root_linker_t::link_root(root_block, &node); } } @@ -1236,7 +1236,7 @@ private: *ret); // This can only happen during init_cached_extent // or when backref extent being rewritten by gc space reclaiming - if (!ret->is_pending() && !ret->is_linked()) { + if (ret->is_stable() && !ret->is_linked()) { assert(ret->has_delta() || is_backref_node(ret->get_type())); init_internal(*ret); } @@ -1276,7 +1276,7 @@ private: parent_pos=std::move(parent_pos)] (leaf_node_t &node) { using tree_root_linker_t = TreeRootLinker; - assert(!node.is_pending()); + assert(node.is_stable()); assert(!node.is_linked()); node.range = fixed_kv_node_meta_t{begin, end, 1}; if (parent_pos) { @@ -1289,7 +1289,7 @@ private: auto &stable_root = (RootBlockRef&)*root_block->get_prior_instance(); tree_root_linker_t::link_root(stable_root, &node); } else { - assert(!root_block->is_pending()); + assert(root_block->is_stable()); tree_root_linker_t::link_root(root_block, &node); } } @@ -1319,7 +1319,7 @@ private: *ret); // This can only happen during init_cached_extent // or when backref extent being rewritten by gc space reclaiming - if (!ret->is_pending() && !ret->is_linked()) { + if (ret->is_stable() && !ret->is_linked()) { assert(ret->has_delta() || is_backref_node(ret->get_type())); init_leaf(*ret); } diff --git a/src/crimson/os/seastore/btree/fixed_kv_node.h b/src/crimson/os/seastore/btree/fixed_kv_node.h index 6bfb825fa8c..2cd3b47a58f 100644 --- a/src/crimson/os/seastore/btree/fixed_kv_node.h +++ b/src/crimson/os/seastore/btree/fixed_kv_node.h @@ -182,7 +182,7 @@ struct FixedKVInternalNode } virtual ~FixedKVInternalNode() { - if (this->is_valid() && !this->is_pending()) { + if (this->is_stable()) { if (this->is_btree_root()) { this->root_node_t::destroy(); } else { @@ -571,7 +571,7 @@ struct FixedKVLeafNode } virtual ~FixedKVLeafNode() { - if (this->is_valid() && !this->is_pending()) { + if (this->is_stable()) { if (this->is_btree_root()) { this->root_node_t::destroy(); } else { diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h index 38f44502db8..371c7d204fc 100644 --- a/src/crimson/os/seastore/cached_extent.h +++ b/src/crimson/os/seastore/cached_extent.h @@ -436,7 +436,8 @@ public: void rewrite(Transaction &t, CachedExtent &e, extent_len_t o) { assert(is_initial_pending()); - if (!e.is_pending()) { + assert(e.is_valid()); + if (e.is_stable()) { set_prior_instance(&e); } else { assert(e.has_mutation()); @@ -550,7 +551,8 @@ public: state == extent_state_t::EXIST_MUTATION_PENDING; } - /// Returns true if extent is part of an open transaction + /// Returns true if extent is part of an open transaction, + /// normally equivalent to !is_stable. bool is_pending() const { return is_mutable() || state == extent_state_t::EXIST_CLEAN; } @@ -574,7 +576,8 @@ public: return (has_mutation() || is_initial_pending()) && is_pending_io(); } - /// Returns true if extent is stable and shared among transactions + /// Returns true if extent is stable and shared among transactions, + /// normally equivalent to !is_pending bool is_stable() const { return is_stable_written() || is_stable_writting(); } diff --git a/src/crimson/os/seastore/lba/btree_lba_manager.cc b/src/crimson/os/seastore/lba/btree_lba_manager.cc index 8eace23652e..3fe901f8768 100644 --- a/src/crimson/os/seastore/lba/btree_lba_manager.cc +++ b/src/crimson/os/seastore/lba/btree_lba_manager.cc @@ -490,7 +490,7 @@ _init_cached_extent( iter.get_key() == logn->get_laddr() && iter.get_val().pladdr.is_paddr() && iter.get_val().pladdr.get_paddr() == logn->get_paddr()) { - assert(!iter.get_leaf_node()->is_pending()); + assert(iter.get_leaf_node()->is_stable()); iter.get_leaf_node()->link_child(logn.get(), iter.get_leaf_pos()); logn->set_laddr(iter.get_key()); ceph_assert(iter.get_val().len == e->get_length()); diff --git a/src/crimson/os/seastore/linked_tree_node.h b/src/crimson/os/seastore/linked_tree_node.h index eb5708483f7..4887a186775 100644 --- a/src/crimson/os/seastore/linked_tree_node.h +++ b/src/crimson/os/seastore/linked_tree_node.h @@ -206,8 +206,8 @@ public: virtual key_t node_begin() const = 0; protected: parent_tracker_ref parent_tracker; - virtual bool valid() const = 0; - virtual bool pending() const = 0; + virtual bool _is_valid() const = 0; + virtual bool _is_stable() const = 0; template friend class ParentNode; }; @@ -360,8 +360,8 @@ public: assert(pos < me.get_size()); assert(pos < children.capacity()); assert(child); - ceph_assert(!me.is_pending()); - assert(child->valid() && !child->pending()); + ceph_assert(me.is_stable()); + assert(child->_is_stable()); assert(!children[pos]); ceph_assert(is_valid_child_ptr(child)); update_child_ptr(pos, child); @@ -460,7 +460,7 @@ protected: void on_rewrite(Transaction &t, T &foreign_extent) { auto &me = down_cast(); - if (!foreign_extent.is_pending()) { + if (foreign_extent.is_stable()) { foreign_extent.add_copy_dest(t, &me); copy_sources.emplace(&foreign_extent); } else { @@ -508,7 +508,7 @@ protected: T &src) { ceph_assert(dest.is_initial_pending()); - if (!src.is_pending()) { + if (src.is_stable()) { src.add_copy_dest(t, &dest); dest.copy_sources.emplace(&src); } else if (src.is_mutation_pending()) { @@ -713,7 +713,7 @@ protected: for (auto it = children.begin(); it != children.begin() + down_cast().get_size(); it++) { - if (is_valid_child_ptr(*it) && (*it)->valid()) { + if (is_valid_child_ptr(*it) && (*it)->_is_valid()) { return false; } } @@ -1041,11 +1041,11 @@ private: assert(iter.get_key() == me.get_begin()); return iter.get_offset(); } - bool valid() const final { + bool _is_valid() const final { return down_cast().is_valid(); } - bool pending() const final { - return down_cast().is_pending(); + bool _is_stable() const final { + return down_cast().is_stable(); } key_t node_begin() const final { return down_cast().get_begin(); diff --git a/src/crimson/os/seastore/logical_child_node.h b/src/crimson/os/seastore/logical_child_node.h index b17d5c17bc4..0394a88bda8 100644 --- a/src/crimson/os/seastore/logical_child_node.h +++ b/src/crimson/os/seastore/logical_child_node.h @@ -21,8 +21,7 @@ public: LogicalChildNode(T&&... t) : LogicalCachedExtent(std::forward(t)...) {} virtual ~LogicalChildNode() { - if (this->is_valid() && - !this->is_pending()) { + if (this->is_stable()) { lba_child_node_t::destroy(); } } diff --git a/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.h b/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.h index c8b6fae475b..c42030a6576 100644 --- a/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.h +++ b/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.h @@ -250,8 +250,7 @@ struct OMapInnerNode } ~OMapInnerNode() { - if (this->is_valid() - && !this->is_pending() + if (this->is_stable() && !this->is_btree_root() // dirty omap extent may not be accessed/linked yet && this->base_child_t::has_parent_tracker()) { @@ -352,8 +351,7 @@ struct OMapLeafNode } ~OMapLeafNode() { - if (this->is_valid() - && !this->is_pending() + if (this->is_stable() && !this->is_btree_root() // dirty omap extent may not be accessed/linked yet && this->base_child_t::has_parent_tracker()) { -- 2.39.5