crimson/os/seastore: distinguish between is_mutation_pending() and has_mutation()

Signed-off-by: Yingxin Cheng <[email protected]>

Author: cyx1231st

src/crimson/os/seastore/cache.cc

@@ -1273,7 +1273,7 @@ record_t Cache::prepare_record(
     i->set_modify_time(commit_time);
     DEBUGT("mutated extent with {}B delta -- {}",
 	   t, delta_length, *i);
-    if (!i->is_exist_mutation_pending()) {
+    if (i->is_mutation_pending()) {
       DEBUGT("commit replace extent ... -- {}, prior={}",
 	     t, *i, *i->prior_instance);
 
@@ -1298,6 +1298,8 @@ record_t Cache::prepare_record(
       // the existing extents should be added into Cache
       // during complete_commit to sync with gc transaction.
       commit_replace_extent(t, i, i->prior_instance);
+    } else {
+      assert(i->is_exist_mutation_pending());
     }
 
     i->prepare_write();

src/crimson/os/seastore/cached_extent.h

@@ -439,7 +439,7 @@ class CachedExtent
     if (!e.is_pending()) {
       set_prior_instance(&e);
     } else {
-      assert(e.is_mutation_pending());
+      assert(e.has_mutation());
       set_prior_instance(e.prior_instance);
     }
     e.get_bptr().copy_out(
@@ -565,12 +565,12 @@ class CachedExtent
   }
 
   bool is_stable_writting() const {
-    // MUTATION_PENDING/INITIAL_WRITE_PENDING and under-io extents are already
+    // mutated/INITIAL_WRITE_PENDING and under-io extents are already
     // stable and visible, see prepare_record().
     //
     // XXX: It might be good to mark this case as DIRTY/CLEAN from the definition,
     // which probably can make things simpler.
-    return (is_mutation_pending() || is_initial_pending()) && is_pending_io();
+    return (has_mutation() || is_initial_pending()) && is_pending_io();
   }
 
   /// Returns true if extent is stable and shared among transactions
@@ -583,11 +583,15 @@ class CachedExtent
   }
 
   /// Returns true if extent has a pending delta
-  bool is_mutation_pending() const {
+  bool has_mutation() const {
     return state == extent_state_t::MUTATION_PENDING
       || state == extent_state_t::EXIST_MUTATION_PENDING;
   }
 
+  bool is_mutation_pending() const {
+    return state == extent_state_t::MUTATION_PENDING;
+  }
+
   /// Returns true if extent is a fresh extent
   bool is_initial_pending() const {
     return state == extent_state_t::INITIAL_WRITE_PENDING;
@@ -636,10 +640,10 @@ class CachedExtent
 
   /// Returns true if extent or prior_instance has been invalidated
   bool has_been_invalidated() const {
-    return !is_valid() || (is_mutation_pending() && !prior_instance->is_valid());
+    return !is_valid() || (prior_instance && !prior_instance->is_valid());
   }
 
-  /// Returns true if extent is a plcaeholder
+  /// Returns true if extent is a placeholder
   bool is_placeholder() const {
     return is_retired_placeholder_type(get_type());
   }
@@ -1077,7 +1081,7 @@ class CachedExtent
     if (is_initial_pending() && addr.is_record_relative()) {
       return addr.block_relative_to(get_paddr());
     } else {
-      ceph_assert(!addr.is_record_relative() || is_mutation_pending());
+      ceph_assert(!addr.is_record_relative() || has_mutation());
       return addr;
     }
   }

src/crimson/os/seastore/object_data_handler.h

@@ -128,7 +128,7 @@ struct ObjectDataBlock : crimson::os::seastore::LogicalChildNode {
   }
 
   void prepare_commit() final {
-    if (is_mutation_pending() || is_exist_mutation_pending()) {
+    if (has_mutation()) {
       ceph_assert(!cached_overwrites.is_empty());
       if (cached_overwrites.has_cached_bptr()) {
         set_bptr(cached_overwrites.move_cached_bptr());

src/crimson/os/seastore/transaction.h

@@ -151,6 +151,7 @@ class Transaction {
       assert(read_set.count(ref->prior_instance->get_paddr(), extent_cmp_t{}));
       ref->reset_prior_instance();
     } else {
+      assert(ref->is_stable_written());
       // && retired_set.count(ref->get_paddr()) == 0
       // If it's already in the set, insert here will be a noop,
       // which is what we want.
@@ -285,9 +286,10 @@ class Transaction {
     assert(ref->is_exist_mutation_pending() ||
 	   read_set.count(ref->prior_instance->get_paddr(), extent_cmp_t{}));
     mutated_block_list.push_back(ref);
-    if (!ref->is_exist_mutation_pending()) {
+    if (ref->is_mutation_pending()) {
       write_set.insert(*ref);
     } else {
+      assert(ref->is_exist_mutation_pending());
       // already added as fresh extent in write_set
       assert(write_set.exists(*ref));
     }

src/crimson/os/seastore/transaction_manager.cc

@@ -633,9 +633,10 @@ TransactionManager::rewrite_extent_ret TransactionManager::rewrite_extent(
     assert(extent->get_version() > 0);
     if (is_root_type(extent->get_type())) {
       // pass
-    } else if (extent->get_version() == 1 && extent->is_mutation_pending()) {
+    } else if (extent->get_version() == 1 && extent->has_mutation()) {
       t.get_rewrite_stats().account_n_dirty();
     } else {
+      // extent->get_version() > 1 or DIRTY
       t.get_rewrite_stats().account_dirty(extent->get_version());
     }
     if (epm->can_inplace_rewrite(t, extent)) {