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 <yingxin.cheng@intel.com>

diff --git a/src/crimson/os/seastore/btree/fixed_kv_btree.h b/src/crimson/os/seastore/btree/fixed_kv_btree.h
index ab12ab3ee6aeb1654e02d4f66c29b3cdda7e5c31..fef9a03114d0d74afbb0b9d487f84fafac03bd02 100644 (file)
--- 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<RootBlock, internal_node_t>;
-      assert(!node.is_pending());
+      assert(node.is_stable());
       assert(!node.is_linked());
       node.range = fixed_kv_node_meta_t<node_key_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<RootBlock, leaf_node_t>;
-      assert(!node.is_pending());
+      assert(node.is_stable());
       assert(!node.is_linked());
       node.range = fixed_kv_node_meta_t<node_key_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 6bfb825fa8c5527e4bd81c55510191441584e69f..2cd3b47a58fe75e7f649ab23ec5a7de0608de9fb 100644 (file)
--- 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 38f44502db89c30d61ed996cad5b6503555337a4..371c7d204fce080271fb210ae0144e47e5e3ccb1 100644 (file)
--- 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 8eace23652ec5d6dd107081b97e08c79276655ae..3fe901f8768bbdf58868533933d85a94360a0cb3 100644 (file)
--- 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 eb5708483f7ab5ad0cce0077949b7ed870367f9b..4887a186775355a2d300064071e3d155f5a1fc38 100644 (file)
--- 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<ParentT> 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 <typename, typename, typename>
   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 b17d5c17bc453e29ddbab12ba23c56de01aab540..0394a88bda84b389f52670e5c446717e21bddecd 100644 (file)
--- 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>(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 c8b6fae475b27541dd9fa49e9daf21211c47db1c..c42030a6576315e8630ec554f8e36add6b255d4d 100644 (file)
--- 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()) {