crimson/os/seastore: consolidate Cache::init_cached_extent with drop_from_cache

So Cache::drop_from_cache() can be removed, and make it possible to add
related logs within the Cache component.

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 0b4a50afed76569e87a17f11dfbb69c899eccf48..2c50c0e9316b3c1f67989dc407629777332f7919 100644 (file)
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -126,17 +126,6 @@ public:
     SUBDEBUGT(seastore_cache, "reset", t);
   }
 
-  /**
-   * drop_from_cache
-   *
-   * Drop extent from cache.  Intended for use when
-   * ref refers to a logically dead extent as during
-   * replay.
-   */
-  void drop_from_cache(CachedExtentRef ref) {
-    remove_extent(ref);
-  }
-
   /// Declare ref retired in t
   void retire_extent(Transaction &t, CachedExtentRef ref) {
     t.add_to_retired_set(ref);
@@ -608,16 +597,25 @@ public:
     return seastar::do_with(
       std::forward<F>(f),
       std::move(dirty),
-      [&t](auto &f, auto &refs) mutable {
-       return trans_intr::do_for_each(
-         refs,
-         [&t, &f](auto &e) { return f(t, e); });
-      }).handle_error_interruptible(
-       init_cached_extents_iertr::pass_further{},
-       crimson::ct_error::assert_all{
-         "Invalid error in Cache::init_cached_extents"
-       }
-      );
+      [this, &t](auto &f, auto &refs) mutable
+    {
+      return trans_intr::do_for_each(
+        refs,
+        [this, &t, &f](auto &e)
+      {
+        return f(t, e
+        ).si_then([this, &t, e](bool is_alive) {
+          if (!is_alive) {
+            remove_extent(e);
+          }
+        });
+      });
+    }).handle_error_interruptible(
+      init_cached_extents_iertr::pass_further{},
+      crimson::ct_error::assert_all{
+        "Invalid error in Cache::init_cached_extents"
+      }
+    );
   }
 
   /**

diff --git a/src/crimson/os/seastore/lba_manager.h b/src/crimson/os/seastore/lba_manager.h
index c98302371ed84d903dadefbfc1d6a2d83913ce3d..69aa0954a9cdb495a7bf6e7e150fd92c8ab986b1 100644 (file)
--- a/src/crimson/os/seastore/lba_manager.h
+++ b/src/crimson/os/seastore/lba_manager.h
@@ -118,9 +118,11 @@ public:
    * Implementation must initialize the LBAPin on any
    * LogicalCachedExtent's and may also read in any dependent
    * structures, etc.
+   *
+   * @return returns whether the extent is alive
    */
   using init_cached_extent_iertr = base_iertr;
-  using init_cached_extent_ret = init_cached_extent_iertr::future<>;
+  using init_cached_extent_ret = init_cached_extent_iertr::future<bool>;
   virtual init_cached_extent_ret init_cached_extent(
     Transaction &t,
     CachedExtentRef e) = 0;

diff --git a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc
index f15582733f7910e8cb160c49995fed415a06bdf4..c9b9ee865b324a8c6759c0817d4fa5aefd218467 100644 (file)
--- a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc
+++ b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc
@@ -266,14 +266,17 @@ BtreeLBAManager::init_cached_extent_ret BtreeLBAManager::init_cached_extent(
 {
   LOG_PREFIX(BtreeLBAManager::init_cached_extent);
   DEBUGT("extent {}", t, *e);
-  auto c = get_context(t);
-  return with_btree(
-    c,
-    [c, e](auto &btree) {
-      return btree.init_cached_extent(
-       c, e
-      ).si_then([](auto) {});
+  return seastar::do_with(bool(), [this, e, &t](bool& ret) {
+    auto c = get_context(t);
+    return with_btree(c, [c, e, &ret](auto &btree) {
+      return btree.init_cached_extent(c, e
+      ).si_then([&ret](bool is_alive) {
+        ret = is_alive;
+      });
+    }).si_then([&ret] {
+      return ret;
     });
+  });
 }
 
 BtreeLBAManager::scan_mappings_ret BtreeLBAManager::scan_mappings(

diff --git a/src/crimson/os/seastore/lba_manager/btree/lba_btree.cc b/src/crimson/os/seastore/lba_manager/btree/lba_btree.cc
index 84358df08da1132f5a6a1f43287bba72318ee3b1..9bea6b596ab9631c747b5b603a81ec4b37f31641 100644 (file)
--- a/src/crimson/os/seastore/lba_manager/btree/lba_btree.cc
+++ b/src/crimson/os/seastore/lba_manager/btree/lba_btree.cc
@@ -286,12 +286,11 @@ LBABtree::init_cached_extent_ret LBABtree::init_cached_extent(
          c.pins->add_pin(
            static_cast<BtreeLBAPin&>(logn->get_pin()).pin);
        }
-       DEBUGT("logical extent {} live, initialized", c.trans, *logn);
-       return e;
+       DEBUGT("logical extent {} live", c.trans, *logn);
+       return true;
       } else {
-       DEBUGT("logical extent {} not live, dropping", c.trans, *logn);
-       c.cache.drop_from_cache(logn);
-       return CachedExtentRef();
+       DEBUGT("logical extent {} not live", c.trans, *logn);
+       return false;
       }
     });
   } else if (e->get_type() == extent_types_t::LADDR_INTERNAL) {
@@ -304,11 +303,10 @@ LBABtree::init_cached_extent_ret LBABtree::init_cached_extent(
       if (cand_depth <= iter.get_depth() &&
          &*iter.get_internal(cand_depth).node == &*eint) {
        DEBUGT("extent {} is live", c.trans, *eint);
-       return e;
+       return true;
       } else {
        DEBUGT("extent {} is not live", c.trans, *eint);
-       c.cache.drop_from_cache(eint);
-       return CachedExtentRef();
+       return false;
       }
     });
   } else if (e->get_type() == extent_types_t::LADDR_LEAF) {
@@ -319,11 +317,10 @@ LBABtree::init_cached_extent_ret LBABtree::init_cached_extent(
       // Note, this check is valid even if iter.is_end()
       if (iter.leaf.node == &*eleaf) {
        DEBUGT("extent {} is live", c.trans, *eleaf);
-       return e;
+       return true;
       } else {
        DEBUGT("extent {} is not live", c.trans, *eleaf);
-       c.cache.drop_from_cache(eleaf);
-       return CachedExtentRef();
+       return false;
       }
     });
   } else {
@@ -334,7 +331,7 @@ LBABtree::init_cached_extent_ret LBABtree::init_cached_extent(
       e->get_type());
     return init_cached_extent_ret(
       interruptible::ready_future_marker{},
-      e);
+      true);
   }
 }
 

diff --git a/src/crimson/os/seastore/lba_manager/btree/lba_btree.h b/src/crimson/os/seastore/lba_manager/btree/lba_btree.h
index 5caf094047482c18ccb17cae3b97ddd365d0ea0b..163f7b11d9c8a4d9cacfcfd295c0ed7923950621 100644 (file)
--- a/src/crimson/os/seastore/lba_manager/btree/lba_btree.h
+++ b/src/crimson/os/seastore/lba_manager/btree/lba_btree.h
@@ -378,10 +378,10 @@ public:
    * Checks whether e is live (reachable from lba tree) and drops or initializes
    * accordingly.
    *
-   * Returns e if live and a null CachedExtentRef otherwise.
+   * Returns if e is live.
    */
   using init_cached_extent_iertr = base_iertr;
-  using init_cached_extent_ret = init_cached_extent_iertr::future<CachedExtentRef>;
+  using init_cached_extent_ret = init_cached_extent_iertr::future<bool>;
   init_cached_extent_ret init_cached_extent(op_context_t c, CachedExtentRef e);
 
   /// get_leaf_if_live: get leaf node at laddr/addr if still live