From 2e9a34eed63b1eaa1f7ad698fe58fedd3701dc68 Mon Sep 17 00:00:00 2001
From: Yingxin Cheng <yingxin.cheng@intel.com>
Date: Mon, 16 Jun 2025 14:02:40 +0800
Subject: [PATCH] crimson/os/seastore: adjust is_stable_clean()

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
---
 src/crimson/os/seastore/cache.cc        | 12 +++++-------
 src/crimson/os/seastore/cache.h         | 17 ++++++++++++-----
 src/crimson/os/seastore/cached_extent.h |  5 ++---
 3 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc
index ef0fb233e69..070ad1414bf 100644
--- a/src/crimson/os/seastore/cache.cc
+++ b/src/crimson/os/seastore/cache.cc
@@ -901,12 +901,10 @@ void Cache::commit_replace_extent(
 
   const auto t_src = t.get_src();
   if (is_root_type(prev->get_type())) {
-    assert(prev->is_stable_clean()
-      || prev->primary_ref_list_hook.is_linked());
-    if (prev->is_stable_dirty()) {
-      // add the new dirty root to front
-      remove_from_dirty(prev, nullptr/* exclude root */);
-    }
+    assert(prev->is_stable_dirty());
+    assert(prev->primary_ref_list_hook.is_linked());
+    // add the new dirty root to front
+    remove_from_dirty(prev, nullptr/* exclude root */);
     add_to_dirty(next, nullptr/* exclude root */);
   } else if (prev->is_stable_dirty()) {
     replace_dirty(next, prev, t_src);
@@ -1795,6 +1793,7 @@ void Cache::complete_commit(
       return;
     }
 
+    assert(i->is_stable_clean_pending());
     bool is_inline = false;
     if (i->is_inline()) {
       is_inline = true;
@@ -1817,7 +1816,6 @@ void Cache::complete_commit(
     const auto t_src = t.get_src();
     touch_extent(*i, &t_src, t.get_cache_hint());
     i->complete_io();
-    assert(i->is_stable_clean());
     epm.commit_space_used(i->get_paddr(), i->get_length());
 
     // Note: commit extents and backref allocations in the same place
diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index ad4dd5c6c7b..fd9fa64a75d 100644
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -226,6 +226,7 @@ public:
           ++access_stats.trans_dirty;
           ++stats.access.trans_dirty;
         } else {
+          assert(ret->is_stable_clean());
           ++access_stats.trans_lru;
           ++stats.access.trans_lru;
         }
@@ -282,6 +283,7 @@ public:
       ++access_stats.cache_dirty;
       ++stats.access.cache_dirty;
     } else {
+      assert(ret->is_stable_clean());
       ++access_stats.cache_lru;
       ++stats.access.cache_lru;
     }
@@ -395,7 +397,7 @@ public:
 	      t, T::TYPE, offset, length);
     const auto t_src = t.get_src();
     auto f = [&t, this, t_src](CachedExtent &ext) {
-      // FIXME: assert(ext.is_stable_clean());
+      // XXX: is_stable_dirty() may not be linked in lba tree
       assert(ext.is_stable());
       assert(T::TYPE == ext.get_type());
       cache_access_stats_t& access_stats = get_by_ext(
@@ -509,6 +511,7 @@ public:
             ++access_stats.cache_dirty;
             ++stats.access.cache_dirty;
           } else {
+            assert(p_extent->is_stable_clean());
             ++access_stats.cache_lru;
             ++stats.access.cache_lru;
           }
@@ -523,6 +526,7 @@ public:
             ++access_stats.trans_dirty;
             ++stats.access.trans_dirty;
           } else {
+            assert(p_extent->is_stable_clean());
             ++access_stats.trans_lru;
             ++stats.access.trans_lru;
           }
@@ -897,7 +901,7 @@ private:
 	      t, type, offset, length, laddr);
     const auto t_src = t.get_src();
     auto f = [&t, this, t_src](CachedExtent &ext) {
-      // FIXME: assert(ext.is_stable_clean());
+      // XXX: is_stable_dirty() may not be linked in lba tree
       assert(ext.is_stable());
       cache_access_stats_t& access_stats = get_by_ext(
         get_by_src(stats.access_by_src_ext, t_src),
@@ -1641,7 +1645,8 @@ private:
         double seconds) const;
 
     void remove_from_lru(CachedExtent &extent) {
-      assert(extent.is_stable_clean() && !extent.is_placeholder());
+      assert(extent.is_stable_clean());
+      assert(!extent.is_placeholder());
 
       if (extent.primary_ref_list_hook.is_linked()) {
         do_remove_from_lru(extent, nullptr);
@@ -1651,7 +1656,8 @@ private:
     void move_to_top(
         CachedExtent &extent,
         const Transaction::src_t* p_src) {
-      assert(extent.is_stable_clean() && !extent.is_placeholder());
+      assert(extent.is_stable_clean());
+      assert(!extent.is_placeholder());
 
       auto extent_loaded_length = extent.get_loaded_length();
       if (extent.primary_ref_list_hook.is_linked()) {
@@ -1688,7 +1694,8 @@ private:
       assert(extent.is_data_stable());
 
       if (extent.primary_ref_list_hook.is_linked()) {
-        assert(extent.is_stable_clean() && !extent.is_placeholder());
+        assert(extent.is_stable_clean());
+        assert(!extent.is_placeholder());
         // present, increase size
         assert(lru.size() > 0);
         current_size += increased_length;
diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h
index 73c6c88b00a..c26d5eaff50 100644
--- a/src/crimson/os/seastore/cached_extent.h
+++ b/src/crimson/os/seastore/cached_extent.h
@@ -613,13 +613,12 @@ public:
     return state == extent_state_t::DIRTY;
   }
 
-  // Returs true if extent is stable and clean
+  /// Returns iff extent is CLEAN
   bool is_stable_clean() const {
-    ceph_assert(is_valid());
     return state == extent_state_t::CLEAN;
   }
 
-  // Returns true if the buffer is still loading
+  /// Returns iff extent is CLEAN and pending
   bool is_stable_clean_pending() const {
     return is_stable_clean() && is_pending_io();
   }
-- 
2.47.3