crimson/os/seastore/cache: cleanup add_extent()

Move add_to_dirty() and touch_extent() out of add_extent(), this removes
duplicated calls to touch_extent() from the on_cache callback.

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 6963e3d351df9cc93ea3ecd9fdd028850d73e2ee..6f795392e332ad835329d13a5c8f180873015808 100644 (file)
--- a/src/crimson/os/seastore/cache.cc
+++ b/src/crimson/os/seastore/cache.cc
@@ -97,8 +97,7 @@ Cache::retire_extent_ret Cache::retire_extent_addr(
              TRANS_ID_NULL);
     DEBUGT("retire {}~{} as placeholder, add extent -- {}",
            t, addr, length, *ext);
-    const auto t_src = t.get_src();
-    add_extent(ext, &t_src);
+    add_extent(ext);
   }
   t.add_to_read_set(ext);
   t.add_to_retired_set(ext);
@@ -126,8 +125,7 @@ void Cache::retire_absent_extent_addr(
            TRANS_ID_NULL);
   DEBUGT("retire {}~{} as placeholder, add extent -- {}",
         t, addr, length, *ext);
-  const auto t_src = t.get_src();
-  add_extent(ext, &t_src);
+  add_extent(ext);
   t.add_to_read_set(ext);
   t.add_to_retired_set(ext);
 }
@@ -727,19 +725,12 @@ void Cache::register_metrics()
   );
 }
 
-void Cache::add_extent(
-    CachedExtentRef ref,
-    const Transaction::src_t* p_src=nullptr)
+void Cache::add_extent(CachedExtentRef ref)
 {
   assert(ref->is_valid());
   assert(ref->user_hint == PLACEMENT_HINT_NULL);
   assert(ref->rewrite_generation == NULL_GENERATION);
   extents.insert(*ref);
-  if (ref->is_dirty()) {
-    add_to_dirty(ref);
-  } else {
-    touch_extent(*ref, p_src);
-  }
 }
 
 void Cache::mark_dirty(CachedExtentRef ref)
@@ -1573,9 +1564,11 @@ void Cache::complete_commit(
     i->prior_instance.reset();
     DEBUGT("add extent as fresh, inline={} -- {}",
           t, is_inline, *i);
-    const auto t_src = t.get_src();
     i->invalidate_hints();
-    add_extent(i, &t_src);
+    add_extent(i);
+    assert(!i->is_dirty());
+    const auto t_src = t.get_src();
+    touch_extent(*i, &t_src);
     epm.commit_space_used(i->get_paddr(), i->get_length());
     if (is_backref_mapped_extent_node(i)) {
       DEBUGT("backref_list new {} len {}",
@@ -1691,8 +1684,13 @@ void Cache::complete_commit(
          i->get_length(),
          i->get_type(),
          start_seq));
-      const auto t_src = t.get_src();
-      add_extent(i, &t_src);
+      add_extent(i);
+      if (i->is_dirty()) {
+        add_to_dirty(i);
+      } else {
+        const auto t_src = t.get_src();
+        touch_extent(*i, &t_src);
+      }
     }
   }
   if (!backref_list.empty()) {
@@ -1868,6 +1866,7 @@ Cache::replay_delta(
           journal_seq, record_base, delta, *root);
     root->set_modify_time(modify_time);
     add_extent(root);
+    add_to_dirty(root);
     return replay_delta_ertr::make_ready_future<std::pair<bool, CachedExtentRef>>(
       std::make_pair(true, root));
   } else {
@@ -1895,7 +1894,9 @@ Cache::replay_delta(
         delta.length,
         nullptr,
         [](CachedExtent &) {},
-        [](CachedExtent &) {}) :
+        [this](CachedExtent &ext) {
+          touch_extent(ext);
+        }) :
       _get_extent_if_cached(
        delta.paddr)
     ).handle_error(

diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index 0807e83d371e30bb76053a8e555670cc1ea1d3de..df908582a5890c3516889973a897b1050c46f1b3 100644 (file)
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -585,8 +585,8 @@ private:
       SUBDEBUG(seastore_cache,
           "{} {}~{} is absent, add extent and reading ... -- {}",
           T::TYPE, offset, length, *ret);
-      const auto p_src = p_src_ext ? &p_src_ext->first : nullptr;
-      add_extent(ret, p_src);
+      add_extent(ret);
+      // touch_extent() should be included in on_cache
       on_cache(*ret);
       extent_init_func(*ret);
       return read_extent<T>(
@@ -1642,7 +1642,10 @@ private:
     const journal_seq_t &);
 
   /// Add extent to extents handling dirty and refcounting
-  void add_extent(CachedExtentRef ref, const Transaction::src_t* t_src);
+  ///
+  /// Note, it must follows with add_to_dirty() or touch_extent().
+  /// The only exception is RetiredExtentPlaceholder.
+  void add_extent(CachedExtentRef ref);
 
   /// Mark exising extent ref dirty -- mainly for replay
   void mark_dirty(CachedExtentRef ref);