From: Yingxin Cheng <yingxin.cheng@intel.com>
Date: Wed, 21 May 2025 03:54:05 +0000 (+0800)
Subject: crimson/os/seastore: more accurate usages to is_mutable() vs is_data_stable()
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=6f892feb0c829aac350135e18577f26f3c407e3d;p=ceph.git

crimson/os/seastore: more accurate usages to is_mutable() vs is_data_stable()

Generally, prefer is_mutable() than !is_data_stable(), and
is_data_stable() than !is_mutable().

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
---

diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc
index db2ff1ff0a615..dacadba80314a 100644
--- a/src/crimson/os/seastore/cache.cc
+++ b/src/crimson/os/seastore/cache.cc
@@ -1162,6 +1162,7 @@ CachedExtentRef Cache::duplicate_for_write(
   Transaction &t,
   CachedExtentRef i) {
   LOG_PREFIX(Cache::duplicate_for_write);
+  ceph_assert(i->is_valid());
   assert(i->is_fully_loaded());
 
 #ifndef NDEBUG
diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index 049df6f820291..9d3193b9e9cbf 100644
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -633,7 +633,7 @@ private:
     //
     // TODO(implement fine-grained-wait)
     assert(!extent->is_range_loaded(partial_off, partial_len));
-    assert(!extent->is_mutable());
+    assert(extent->is_data_stable());
     if (extent->is_pending_io()) {
       std::optional<Transaction::src_t> src;
       if (p_src) {
@@ -1685,7 +1685,7 @@ private:
       CachedExtent &extent,
       extent_len_t increased_length,
       const Transaction::src_t* p_src) {
-      assert(!extent.is_mutable());
+      assert(extent.is_data_stable());
 
       if (extent.primary_ref_list_hook.is_linked()) {
         assert(extent.is_stable_clean() && !extent.is_placeholder());
diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h
index 8775a69444a5d..38f44502db89c 100644
--- a/src/crimson/os/seastore/cached_extent.h
+++ b/src/crimson/os/seastore/cached_extent.h
@@ -542,7 +542,8 @@ public:
     return TCachedExtentRef<const T>(static_cast<const T*>(this));
   }
 
-  /// Returns true if extent can be mutated in an open transaction
+  /// Returns true if extent can be mutated in an open transaction,
+  /// normally equivalent to !is_data_stable.
   bool is_mutable() const {
     return state == extent_state_t::INITIAL_WRITE_PENDING ||
       state == extent_state_t::MUTATION_PENDING ||
@@ -578,6 +579,8 @@ public:
     return is_stable_written() || is_stable_writting();
   }
 
+  /// Returns true if extent can not be mutated,
+  /// normally equivalent to !is_mutable.
   bool is_data_stable() const {
     return is_stable() || is_exist_clean();
   }
diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_accessor.h b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_accessor.h
index 14cd2ef3a8db1..49c29710bc29c 100644
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_accessor.h
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_accessor.h
@@ -305,7 +305,7 @@ class NodeExtentAccessorT {
       assert(p_recorder->node_type() == NODE_TYPE);
       assert(p_recorder->field_type() == FIELD_TYPE);
       recorder = static_cast<recorder_t*>(p_recorder);
-    } else if (!extent->is_mutable() && extent->is_valid()) {
+    } else if (extent->is_stable()) {
       state = nextent_state_t::READ_ONLY;
       // mut is empty
       assert(extent->get_recorder() == nullptr ||
@@ -355,7 +355,7 @@ class NodeExtentAccessorT {
   void prepare_mutate(context_t c) {
     assert(!is_retired());
     if (state == nextent_state_t::READ_ONLY) {
-      assert(!extent->is_mutable());
+      assert(extent->is_stable());
       auto ref_recorder = recorder_t::create_for_encode(c.vb);
       recorder = static_cast<recorder_t*>(ref_recorder.get());
       extent = extent->mutate(c, std::move(ref_recorder));
diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h
index 8e3fe2e9f0785..0238307a9745d 100644
--- a/src/crimson/os/seastore/transaction_manager.h
+++ b/src/crimson/os/seastore/transaction_manager.h
@@ -563,7 +563,7 @@ public:
 	  std::optional<ceph::bufferptr> original_bptr;
 	  // TODO: preserve the bufferspace if partially loaded
 	  if (ext && ext->is_fully_loaded()) {
-	    ceph_assert(!ext->is_mutable());
+	    ceph_assert(ext->is_data_stable());
 	    ceph_assert(ext->get_length() >= original_len);
 	    ceph_assert(ext->get_paddr() == original_paddr);
 	    original_bptr = ext->get_bptr();