From: Yingxin Cheng <yingxin.cheng@intel.com>
Date: Mon, 19 May 2025 07:01:18 +0000 (+0800)
Subject: crimson/os/seastore: cleanup CachedExtent::prior_instance
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=c53b97131ca3666bd147112d446c559b21dc448e;p=ceph.git

crimson/os/seastore: cleanup CachedExtent::prior_instance

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 8a373791c9cf..03cc55781383 100644
--- a/src/crimson/os/seastore/cache.cc
+++ b/src/crimson/os/seastore/cache.cc
@@ -1188,12 +1188,13 @@ CachedExtentRef Cache::duplicate_for_write(
 
     t.add_mutated_extent(i);
     DEBUGT("duplicate existing extent {}", t, *i);
+    assert(!i->prior_instance);
     return i;
   }
 
   auto ret = i->duplicate_for_write(t);
   ret->pending_for_transaction = t.get_trans_id();
-  ret->prior_instance = i;
+  ret->set_prior_instance(i);
   // duplicate_for_write won't occur after ool write finished
   assert(!i->prior_poffset);
   auto [iter, inserted] = i->mutation_pending_extents.insert(*ret);
@@ -1790,7 +1791,7 @@ void Cache::complete_commit(
     i->on_initial_write();
 
     i->state = CachedExtent::extent_state_t::CLEAN;
-    i->prior_instance.reset();
+    i->reset_prior_instance();
     DEBUGT("add extent as fresh, inline={} -- {}",
 	   t, is_inline, *i);
     i->invalidate_hints();
@@ -1842,7 +1843,7 @@ void Cache::complete_commit(
 	   i->prior_instance);
     i->on_delta_write(final_block_start);
     i->pending_for_transaction = TRANS_ID_NULL;
-    i->prior_instance = CachedExtentRef();
+    i->reset_prior_instance();
     i->state = CachedExtent::extent_state_t::DIRTY;
     assert(i->version > 0);
     if (i->version == 1 || is_root_type(i->get_type())) {
diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h
index 747952333128..22082a48360a 100644
--- a/src/crimson/os/seastore/cached_extent.h
+++ b/src/crimson/os/seastore/cached_extent.h
@@ -270,8 +270,10 @@ class CachedExtent
       CachedExtent, boost::thread_unsafe_counter>,
     public trans_spec_view_t {
   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
+    INITIAL_WRITE_PENDING, // In Transaction::write_set and fresh_block_list,
+                           // has prior_instance under rewrite
+    MUTATION_PENDING,      // In Transaction::write_set and mutated_block_list,
+                           // has prior_instance
     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,
@@ -296,7 +298,8 @@ class CachedExtent
 
   uint32_t last_committed_crc = 0;
 
-  // Points at current version while in state MUTATION_PENDING
+  // Points at the prior stable version while in state MUTATION_PENDING
+  // or is rewriting (in state INITIAL_PENDING).
   CachedExtentRef prior_instance;
 
   // time of the last modification
@@ -434,10 +437,10 @@ public:
   void rewrite(Transaction &t, CachedExtent &e, extent_len_t o) {
     assert(is_initial_pending());
     if (!e.is_pending()) {
-      prior_instance = &e;
+      set_prior_instance(&e);
     } else {
       assert(e.is_mutation_pending());
-      prior_instance = e.get_prior_instance();
+      set_prior_instance(e.prior_instance);
     }
     e.get_bptr().copy_out(
       o,
@@ -1030,6 +1033,7 @@ protected:
   virtual void update_in_extent_chksum_field(uint32_t) {}
 
   void set_prior_instance(CachedExtentRef p) {
+    assert(!prior_instance);
     prior_instance = p;
   }
 
@@ -1465,8 +1469,7 @@ protected:
   virtual void logical_on_delta_write() {}
 
   void on_delta_write(paddr_t record_block_offset) final {
-    assert(is_exist_mutation_pending() ||
-	   get_prior_instance());
+    assert(is_exist_mutation_pending() || get_prior_instance());
     logical_on_delta_write();
   }
 
diff --git a/src/crimson/os/seastore/transaction.h b/src/crimson/os/seastore/transaction.h
index b8505493fffa..3a1899b41f93 100644
--- a/src/crimson/os/seastore/transaction.h
+++ b/src/crimson/os/seastore/transaction.h
@@ -149,7 +149,7 @@ public:
       assert(ref->prior_instance);
       retired_set.emplace(ref->prior_instance, trans_id);
       assert(read_set.count(ref->prior_instance->get_paddr(), extent_cmp_t{}));
-      ref->prior_instance.reset();
+      ref->reset_prior_instance();
     } else {
       // && retired_set.count(ref->get_paddr()) == 0
       // If it's already in the set, insert here will be a noop,