From 89b904f7c88f46681f77a0ac7e7e32c186a5fc95 Mon Sep 17 00:00:00 2001 From: Xuehan Xu Date: Wed, 1 Jun 2022 18:44:30 +0800 Subject: [PATCH] crimson/os/seastore/cache: make access to Transaction::read_set atomic Signed-off-by: Xuehan Xu --- src/crimson/os/seastore/cache.cc | 53 +++++++++++------------ src/crimson/os/seastore/cache.h | 74 ++++++++++++++++---------------- 2 files changed, 60 insertions(+), 67 deletions(-) diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc index 6734b30389a79..ca2ced36cb5f2 100644 --- 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( - t.root); + return t.root->wait_io().then([&t] { + return get_root_iertr::make_ready_future( + 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(); - } else { - t.root = ret; - t.add_to_read_set(ret); - return get_root_iertr::make_ready_future( - 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( + root); }); } } @@ -1739,7 +1733,8 @@ Cache::get_extent_ertr::future 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 Cache::_get_extent_by_type( return get_extent_ertr::make_ready_future(); case extent_types_t::BACKREF_INTERNAL: return get_extent( - 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( - 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( - 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( - 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( - 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( - 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( - 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( - 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( - 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 Cache::_get_extent_by_type( return get_extent_ertr::make_ready_future(); case extent_types_t::TEST_BLOCK: return get_extent( - 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( - 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 ac84df837b39e..31be85d55d165 100644 --- 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 using get_extent_ret = get_extent_ertr::future>; - template + template get_extent_ret 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( std::move(ret)); @@ -313,6 +315,7 @@ public: "{} {}~{} is present in cache -- {}", T::TYPE, offset, length, *cached); auto ret = TCachedExtentRef(static_cast(cached.get())); + on_cache(*ret); return ret->wait_io( ).then([ret=std::move(ret)]() mutable -> get_extent_ret { @@ -331,7 +334,7 @@ public: ) { return get_extent( 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>( - ret->cast()); + return ret->wait_io().then([ret] { + return seastar::make_ready_future>( + ret->cast()); + }); } 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( offset, length, &metric_key, - std::forward(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>(); - } else { - touch_extent(*ref); - t.add_to_read_set(ref); - return get_extent_iertr::make_ready_future>( - std::move(ref)); - } + std::forward(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>( + 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(ret); + return ret->wait_io().then([ret] { + return seastar::make_ready_future(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(); - } else { - touch_extent(*ret); - t.add_to_read_set(ret); - return get_extent_ertr::make_ready_future( - 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( + std::move(ret)); }); } } -- 2.39.5