crimson/os/seastore/cache: make access to Transaction::read_set atomic

Signed-off-by: Xuehan Xu <xxhdx1985126@gmail.com>

diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc
index 6734b30389a7967eb6275c967f4c92e85391ad05..ca2ced36cb5f20efd87d8e7a5aab4468aa0852cd 100644 (file)
--- a/src/crimson/os/seastore/cache.cc
+++ b/src/crimson/os/seastore/cache.cc
@@ -1581,6 +1581,7 @@ Cache::replay_delta(
         delta.laddr,
         delta.length,
         nullptr,
+        [](CachedExtent &) {},
         [](CachedExtent &) {}) :
       _get_extent_if_cached(
        delta.paddr)
@@ -1711,24 +1712,17 @@ Cache::get_root_ret Cache::get_root(Transaction &t)
   LOG_PREFIX(Cache::get_root);
   if (t.root) {
     TRACET("root already on t -- {}", t, *t.root);
-    return get_root_iertr::make_ready_future<RootBlockRef>(
-      t.root);
+    return t.root->wait_io().then([&t] {
+      return get_root_iertr::make_ready_future<RootBlockRef>(
+       t.root);
+    });
   } else {
-    auto ret = root;
-    DEBUGT("root not on t -- {}", t, *ret);
-    return ret->wait_io().then([this, FNAME, ret, &t] {
-      TRACET("root wait io done -- {}", t, *ret);
-      if (!ret->is_valid()) {
-       DEBUGT("root is invalid -- {}", t, *ret);
-       ++(get_by_src(stats.trans_conflicts_by_unknown, t.get_src()));
-       mark_transaction_conflicted(t, *ret);
-       return get_root_iertr::make_ready_future<RootBlockRef>();
-      } else {
-       t.root = ret;
-       t.add_to_read_set(ret);
-       return get_root_iertr::make_ready_future<RootBlockRef>(
-         ret);
-      }
+    DEBUGT("root not on t -- {}", t, *root);
+    t.root = root;
+    t.add_to_read_set(root);
+    return root->wait_io().then([this] {
+      return get_root_iertr::make_ready_future<RootBlockRef>(
+       root);
     });
   }
 }
@@ -1739,7 +1733,8 @@ Cache::get_extent_ertr::future<CachedExtentRef> Cache::_get_extent_by_type(
   laddr_t laddr,
   seastore_off_t length,
   const Transaction::src_t* p_src,
-  extent_init_func_t &&extent_init_func)
+  extent_init_func_t &&extent_init_func,
+  extent_init_func_t &&on_cache)
 {
   return [=, extent_init_func=std::move(extent_init_func)]() mutable {
     src_ext_t* p_metric_key = nullptr;
@@ -1755,55 +1750,55 @@ Cache::get_extent_ertr::future<CachedExtentRef> Cache::_get_extent_by_type(
       return get_extent_ertr::make_ready_future<CachedExtentRef>();
     case extent_types_t::BACKREF_INTERNAL:
       return get_extent<backref::BackrefInternalNode>(
-       offset, length, p_metric_key, std::move(extent_init_func)
+       offset, length, p_metric_key, std::move(extent_init_func), std::move(on_cache)
       ).safe_then([](auto extent) {
        return CachedExtentRef(extent.detach(), false /* add_ref */);
       });
     case extent_types_t::BACKREF_LEAF:
       return get_extent<backref::BackrefLeafNode>(
-       offset, length, p_metric_key, std::move(extent_init_func)
+       offset, length, p_metric_key, std::move(extent_init_func), std::move(on_cache)
       ).safe_then([](auto extent) {
        return CachedExtentRef(extent.detach(), false /* add_ref */);
       });
     case extent_types_t::LADDR_INTERNAL:
       return get_extent<lba_manager::btree::LBAInternalNode>(
-       offset, length, p_metric_key, std::move(extent_init_func)
+       offset, length, p_metric_key, std::move(extent_init_func), std::move(on_cache)
       ).safe_then([](auto extent) {
        return CachedExtentRef(extent.detach(), false /* add_ref */);
       });
     case extent_types_t::LADDR_LEAF:
       return get_extent<lba_manager::btree::LBALeafNode>(
-       offset, length, p_metric_key, std::move(extent_init_func)
+       offset, length, p_metric_key, std::move(extent_init_func), std::move(on_cache)
       ).safe_then([](auto extent) {
        return CachedExtentRef(extent.detach(), false /* add_ref */);
       });
     case extent_types_t::OMAP_INNER:
       return get_extent<omap_manager::OMapInnerNode>(
-          offset, length, p_metric_key, std::move(extent_init_func)
+          offset, length, p_metric_key, std::move(extent_init_func), std::move(on_cache)
       ).safe_then([](auto extent) {
         return CachedExtentRef(extent.detach(), false /* add_ref */);
       });
     case extent_types_t::OMAP_LEAF:
       return get_extent<omap_manager::OMapLeafNode>(
-          offset, length, p_metric_key, std::move(extent_init_func)
+          offset, length, p_metric_key, std::move(extent_init_func), std::move(on_cache)
       ).safe_then([](auto extent) {
         return CachedExtentRef(extent.detach(), false /* add_ref */);
       });
     case extent_types_t::COLL_BLOCK:
       return get_extent<collection_manager::CollectionNode>(
-          offset, length, p_metric_key, std::move(extent_init_func)
+          offset, length, p_metric_key, std::move(extent_init_func), std::move(on_cache)
       ).safe_then([](auto extent) {
         return CachedExtentRef(extent.detach(), false /* add_ref */);
       });
     case extent_types_t::ONODE_BLOCK_STAGED:
       return get_extent<onode::SeastoreNodeExtent>(
-          offset, length, p_metric_key, std::move(extent_init_func)
+          offset, length, p_metric_key, std::move(extent_init_func), std::move(on_cache)
       ).safe_then([](auto extent) {
        return CachedExtentRef(extent.detach(), false /* add_ref */);
       });
     case extent_types_t::OBJECT_DATA_BLOCK:
       return get_extent<ObjectDataBlock>(
-          offset, length, p_metric_key, std::move(extent_init_func)
+          offset, length, p_metric_key, std::move(extent_init_func), std::move(on_cache)
       ).safe_then([](auto extent) {
        return CachedExtentRef(extent.detach(), false /* add_ref */);
       });
@@ -1812,13 +1807,13 @@ Cache::get_extent_ertr::future<CachedExtentRef> Cache::_get_extent_by_type(
       return get_extent_ertr::make_ready_future<CachedExtentRef>();
     case extent_types_t::TEST_BLOCK:
       return get_extent<TestBlock>(
-          offset, length, p_metric_key, std::move(extent_init_func)
+          offset, length, p_metric_key, std::move(extent_init_func), std::move(on_cache)
       ).safe_then([](auto extent) {
        return CachedExtentRef(extent.detach(), false /* add_ref */);
       });
     case extent_types_t::TEST_BLOCK_PHYSICAL:
       return get_extent<TestBlockPhysical>(
-          offset, length, p_metric_key, std::move(extent_init_func)
+          offset, length, p_metric_key, std::move(extent_init_func), std::move(on_cache)
       ).safe_then([](auto extent) {
        return CachedExtentRef(extent.detach(), false /* add_ref */);
       });

diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index ac84df837b39e77c62d0fcd871cdb1b6dc773ee4..31be85d55d165cd07dfd9006af856eccdb6cb3bc 100644 (file)
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -264,12 +264,13 @@ public:
   using get_extent_ertr = base_ertr;
   template <typename T>
   using get_extent_ret = get_extent_ertr::future<TCachedExtentRef<T>>;
-  template <typename T, typename Func>
+  template <typename T, typename Func, typename OnCache>
   get_extent_ret<T> get_extent(
     paddr_t offset,                ///< [in] starting addr
     seastore_off_t length,          ///< [in] length
     const src_ext_t* p_metric_key, ///< [in] cache query metric key
-    Func &&extent_init_func        ///< [in] init func for extent
+    Func &&extent_init_func,       ///< [in] init func for extent
+    OnCache &&on_cache
   ) {
     LOG_PREFIX(Cache::get_extent);
     auto cached = query_cache(offset, p_metric_key);
@@ -282,6 +283,7 @@ public:
           "{} {}~{} is absent, add extent and reading ... -- {}",
           T::TYPE, offset, length, *ret);
       add_extent(ret);
+      on_cache(*ret);
       extent_init_func(*ret);
       return read_extent<T>(
        std::move(ret));
@@ -313,6 +315,7 @@ public:
           "{} {}~{} is present in cache -- {}",
           T::TYPE, offset, length, *cached);
       auto ret = TCachedExtentRef<T>(static_cast<T*>(cached.get()));
+      on_cache(*ret);
       return ret->wait_io(
       ).then([ret=std::move(ret)]() mutable
             -> get_extent_ret<T> {
@@ -331,7 +334,7 @@ public:
   ) {
     return get_extent<T>(
       offset, length, p_metric_key,
-      [](T &){});
+      [](T &){}, [](T &) {});
   }
 
   /**
@@ -357,8 +360,10 @@ public:
     } else if (result == Transaction::get_extent_ret::PRESENT) {
       SUBTRACET(seastore_cache, "{} {} is present on t -- {}",
           t, type, offset, *ret);
-      return get_extent_if_cached_iertr::make_ready_future<
-        CachedExtentRef>(ret);
+      return ret->wait_io().then([ret] {
+       return get_extent_if_cached_iertr::make_ready_future<
+         CachedExtentRef>(ret);
+      });
     }
 
     // get_extent_ret::ABSENT from transaction
@@ -413,30 +418,26 @@ public:
           result == Transaction::get_extent_ret::PRESENT ? "present" : "retired",
           *ret);
       assert(result != Transaction::get_extent_ret::RETIRED);
-      return seastar::make_ready_future<TCachedExtentRef<T>>(
-       ret->cast<T>());
+      return ret->wait_io().then([ret] {
+       return seastar::make_ready_future<TCachedExtentRef<T>>(
+         ret->cast<T>());
+      });
     } else {
       SUBTRACET(seastore_cache, "{} {}~{} is absent on t, query cache ...",
                 t, T::TYPE, offset, length);
+      auto f = [&t](CachedExtent &ext) {
+       t.add_to_read_set(CachedExtentRef(&ext));
+      };
       auto metric_key = std::make_pair(t.get_src(), T::TYPE);
       return trans_intr::make_interruptible(
        get_extent<T>(
          offset, length, &metric_key,
-         std::forward<Func>(extent_init_func))
-      ).si_then([this, FNAME, offset, length, &t](auto ref) {
-       (void)this; // silence incorrect clang warning about capture
-       if (!ref->is_valid()) {
-         SUBDEBUGT(seastore_cache, "{} {}~{} is invalid -- {}",
-                   t, T::TYPE, offset, length, *ref);
-         ++(get_by_src(stats.trans_conflicts_by_unknown, t.get_src()));
-         mark_transaction_conflicted(t, *ref);
-         return get_extent_iertr::make_ready_future<TCachedExtentRef<T>>();
-       } else {
-         touch_extent(*ref);
-         t.add_to_read_set(ref);
-         return get_extent_iertr::make_ready_future<TCachedExtentRef<T>>(
-           std::move(ref));
-       }
+         std::forward<Func>(extent_init_func), std::move(f))
+      ).si_then([this](auto ref) {
+       (void)this; // silence incorrect clang warning about captur
+       touch_extent(*ref);
+       return get_extent_iertr::make_ready_future<TCachedExtentRef<T>>(
+         std::move(ref));
       });
     }
   }
@@ -481,7 +482,8 @@ private:
     laddr_t laddr,
     seastore_off_t length,
     const Transaction::src_t* p_src,
-    extent_init_func_t &&extent_init_func
+    extent_init_func_t &&extent_init_func,
+    extent_init_func_t &&on_cache
   );
 
   using get_extent_by_type_iertr = get_extent_iertr;
@@ -505,28 +507,24 @@ private:
     } else if (status == Transaction::get_extent_ret::PRESENT) {
       SUBTRACET(seastore_cache, "{} {}~{} {} is present on t -- {}",
                 t, type, offset, length, laddr, *ret);
-      return seastar::make_ready_future<CachedExtentRef>(ret);
+      return ret->wait_io().then([ret] {
+       return seastar::make_ready_future<CachedExtentRef>(ret);
+      });
     } else {
       SUBTRACET(seastore_cache, "{} {}~{} {} is absent on t, query cache ...",
                 t, type, offset, length, laddr);
+      auto f = [&t](CachedExtent &ext) {
+       t.add_to_read_set(CachedExtentRef(&ext));
+      };
       auto src = t.get_src();
       return trans_intr::make_interruptible(
        _get_extent_by_type(
          type, offset, laddr, length, &src,
-         std::move(extent_init_func))
-      ).si_then([=, &t](CachedExtentRef ret) {
-        if (!ret->is_valid()) {
-          SUBDEBUGT(seastore_cache, "{} {}~{} {} is invalid -- {}",
-                    t, type, offset, length, laddr, *ret);
-          ++(get_by_src(stats.trans_conflicts_by_unknown, t.get_src()));
-          mark_transaction_conflicted(t, *ret.get());
-          return get_extent_ertr::make_ready_future<CachedExtentRef>();
-        } else {
-         touch_extent(*ret);
-          t.add_to_read_set(ret);
-          return get_extent_ertr::make_ready_future<CachedExtentRef>(
-            std::move(ret));
-        }
+         std::move(extent_init_func), std::move(f))
+      ).si_then([this](CachedExtentRef ret) {
+       touch_extent(*ret);
+       return get_extent_ertr::make_ready_future<CachedExtentRef>(
+         std::move(ret));
       });
     }
   }