From: Yingxin Cheng <yingxin.cheng@intel.com>
Date: Fri, 25 Apr 2025 06:47:16 +0000 (+0800)
Subject: crimson/os/seastore/cached_extent: drop CLEAN_PENDING state
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=9c9e4e61238af8353b734398ec1ae54a80931e0f;p=ceph.git

crimson/os/seastore/cached_extent: drop CLEAN_PENDING state

It is only used to prevent printing details when the buffer is still
loading, and is a special case of CLEAN.

It doesn't worth a dedicated state that complicates state-related
decisions.

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

diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index d83165e27b1..121ac34c39c 100644
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -703,7 +703,7 @@ private:
     if (!cached) {
       // partial read
       TCachedExtentRef<T> ret = CachedExtent::make_cached_extent_ref<T>(length);
-      ret->init(CachedExtent::extent_state_t::CLEAN_PENDING,
+      ret->init(CachedExtent::extent_state_t::CLEAN,
                 offset,
                 PLACEMENT_HINT_NULL,
                 NULL_GENERATION,
@@ -724,7 +724,7 @@ private:
     if (is_retired_placeholder_type(cached->get_type())) {
       // partial read
       TCachedExtentRef<T> ret = CachedExtent::make_cached_extent_ref<T>(length);
-      ret->init(CachedExtent::extent_state_t::CLEAN_PENDING,
+      ret->init(CachedExtent::extent_state_t::CLEAN,
                 offset,
                 PLACEMENT_HINT_NULL,
                 NULL_GENERATION,
@@ -1935,8 +1935,7 @@ private:
     const Transaction::src_t* p_src
   ) {
     LOG_PREFIX(Cache::read_extent);
-    assert(extent->state == CachedExtent::extent_state_t::CLEAN_PENDING ||
-           extent->state == CachedExtent::extent_state_t::EXIST_CLEAN ||
+    assert(extent->state == CachedExtent::extent_state_t::EXIST_CLEAN ||
            extent->state == CachedExtent::extent_state_t::CLEAN);
     assert(!extent->is_range_loaded(offset, length));
     assert(is_aligned(offset, get_block_size()));
@@ -1962,9 +1961,6 @@ private:
       });
     }).safe_then(
       [this, FNAME, extent=std::move(extent), offset, length]() mutable {
-        if (likely(extent->state == CachedExtent::extent_state_t::CLEAN_PENDING)) {
-          extent->state = CachedExtent::extent_state_t::CLEAN;
-        }
         ceph_assert(extent->state == CachedExtent::extent_state_t::EXIST_CLEAN
           || extent->state == CachedExtent::extent_state_t::CLEAN
           || !extent->is_valid());
diff --git a/src/crimson/os/seastore/cached_extent.cc b/src/crimson/os/seastore/cached_extent.cc
index 44127df625a..7bb33949d96 100644
--- a/src/crimson/os/seastore/cached_extent.cc
+++ b/src/crimson/os/seastore/cached_extent.cc
@@ -47,8 +47,6 @@ std::ostream &operator<<(std::ostream &out, CachedExtent::extent_state_t state)
     return out << "INITIAL_WRITE_PENDING";
   case CachedExtent::extent_state_t::MUTATION_PENDING:
     return out << "MUTATION_PENDING";
-  case CachedExtent::extent_state_t::CLEAN_PENDING:
-    return out << "CLEAN_PENDING";
   case CachedExtent::extent_state_t::CLEAN:
     return out << "CLEAN";
   case CachedExtent::extent_state_t::DIRTY:
diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h
index 50b696d59ca..598afd7543a 100644
--- a/src/crimson/os/seastore/cached_extent.h
+++ b/src/crimson/os/seastore/cached_extent.h
@@ -272,7 +272,6 @@ class CachedExtent
   enum class extent_state_t : uint8_t {
     INITIAL_WRITE_PENDING, // In Transaction::write_set and fresh_block_list
     MUTATION_PENDING,      // In Transaction::write_set and mutated_block_list
-    CLEAN_PENDING,         // CLEAN, but not yet read out
     CLEAN,                 // In Cache::extent_index, Transaction::read_set
                            //  during write, contents match disk, version == 0
     DIRTY,                 // Same as CLEAN, but contents do not match disk,
@@ -479,8 +478,7 @@ public:
 	<< ", refcount=" << use_count()
 	<< ", user_hint=" << user_hint
 	<< ", rewrite_gen=" << rewrite_gen_printer_t{rewrite_generation};
-    if (state != extent_state_t::INVALID &&
-        state != extent_state_t::CLEAN_PENDING) {
+    if (is_valid() && is_fully_loaded() && !is_stable_clean_pending()) {
       print_detail(out);
     }
     return out << ")";
@@ -559,9 +557,8 @@ public:
 
   /// Returns true if extent is stable, written and shared among transactions
   bool is_stable_written() const {
-    return state == extent_state_t::CLEAN_PENDING ||
-      state == extent_state_t::CLEAN ||
-      state == extent_state_t::DIRTY;
+    return state == extent_state_t::CLEAN
+           || state == extent_state_t::DIRTY;
   }
 
   bool is_stable_writting() const {
@@ -598,15 +595,18 @@ public:
     ceph_assert(is_valid());
     return state == extent_state_t::INITIAL_WRITE_PENDING ||
            state == extent_state_t::CLEAN ||
-           state == extent_state_t::CLEAN_PENDING ||
            state == extent_state_t::EXIST_CLEAN;
   }
 
   // Returs true if extent is stable and clean
   bool is_stable_clean() const {
     ceph_assert(is_valid());
-    return state == extent_state_t::CLEAN ||
-           state == extent_state_t::CLEAN_PENDING;
+    return state == extent_state_t::CLEAN;
+  }
+
+  // Returns true if the buffer is still loading
+  bool is_stable_clean_pending() const {
+    return is_stable_clean() && is_pending_io();
   }
 
   /// Ruturns true if data is persisted while metadata isn't