From 6bacf5e76165c0f5b33130ce91cc16c0f2f4ab41 Mon Sep 17 00:00:00 2001 From: Yingxin Cheng Date: Thu, 29 Aug 2024 15:27:23 +0800 Subject: [PATCH] crimson/os/seastore/cache: drop inaccurate cache_query_by_src Signed-off-by: Yingxin Cheng --- src/crimson/os/seastore/cache.cc | 39 ++++++++++++-------------------- src/crimson/os/seastore/cache.h | 38 +++++++------------------------ 2 files changed, 23 insertions(+), 54 deletions(-) diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc index 3ecff20feb3..09fdb2359bd 100644 --- a/src/crimson/os/seastore/cache.cc +++ b/src/crimson/os/seastore/cache.cc @@ -83,7 +83,7 @@ Cache::retire_extent_ret Cache::retire_extent_addr( // absent from transaction // retiring is not included by the cache hit metrics - ext = query_cache(addr, nullptr); + ext = query_cache(addr); if (ext) { DEBUGT("retire {}~{} in cache -- {}", t, addr, length, *ext); } else { @@ -112,7 +112,7 @@ void Cache::retire_absent_extent_addr( auto result = t.get_extent(addr, &ext); assert(result != Transaction::get_extent_ret::PRESENT && result != Transaction::get_extent_ret::RETIRED); - assert(!query_cache(addr, nullptr)); + assert(!query_cache(addr)); #endif LOG_PREFIX(Cache::retire_absent_extent_addr); // add a new placeholder to Cache @@ -1970,7 +1970,7 @@ Cache::replay_delta( auto _get_extent_if_cached = [this](paddr_t addr) -> get_extent_ertr::future { // replay is not included by the cache hit metrics - auto ret = query_cache(addr, nullptr); + auto ret = query_cache(addr); if (ret) { // no retired-placeholder should be exist yet because no transaction // has been created. @@ -1983,15 +1983,14 @@ Cache::replay_delta( } }; auto extent_fut = (delta.pversion == 0 ? - // replay is not included by the cache hit metrics do_get_caching_extent_by_type( delta.type, delta.paddr, delta.laddr, delta.length, - nullptr, [](CachedExtent &) {}, [this](CachedExtent &ext) { + // replay is not included by the cache hit metrics touch_extent(ext, nullptr); }) : _get_extent_if_cached( @@ -2155,73 +2154,65 @@ Cache::do_get_caching_extent_by_type( paddr_t offset, laddr_t laddr, extent_len_t length, - const Transaction::src_t* p_src, extent_init_func_t &&extent_init_func, extent_init_func_t &&on_cache) { return [=, this, extent_init_func=std::move(extent_init_func)]() mutable { - src_ext_t* p_metric_key = nullptr; - src_ext_t metric_key; - if (p_src) { - metric_key = std::make_pair(*p_src, type); - p_metric_key = &metric_key; - } - switch (type) { case extent_types_t::ROOT: ceph_assert(0 == "ROOT is never directly read"); return get_extent_ertr::make_ready_future(); case extent_types_t::BACKREF_INTERNAL: return do_get_caching_extent( - offset, length, p_metric_key, std::move(extent_init_func), std::move(on_cache) + offset, length, 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 do_get_caching_extent( - offset, length, p_metric_key, std::move(extent_init_func), std::move(on_cache) + offset, length, 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 do_get_caching_extent( - offset, length, p_metric_key, std::move(extent_init_func), std::move(on_cache) + offset, length, 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 do_get_caching_extent( - offset, length, p_metric_key, std::move(extent_init_func), std::move(on_cache) + offset, length, 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 do_get_caching_extent( - offset, length, p_metric_key, std::move(extent_init_func), std::move(on_cache) + offset, length, 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 do_get_caching_extent( - offset, length, p_metric_key, std::move(extent_init_func), std::move(on_cache) + offset, length, 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 do_get_caching_extent( - offset, length, p_metric_key, std::move(extent_init_func), std::move(on_cache) + offset, length, 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 do_get_caching_extent( - offset, length, p_metric_key, std::move(extent_init_func), std::move(on_cache) + offset, length, 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 do_get_caching_extent( - offset, length, p_metric_key, std::move(extent_init_func), std::move(on_cache) + offset, length, std::move(extent_init_func), std::move(on_cache) ).safe_then([](auto extent) { return CachedExtentRef(extent.detach(), false /* add_ref */); }); @@ -2230,13 +2221,13 @@ Cache::do_get_caching_extent_by_type( return get_extent_ertr::make_ready_future(); case extent_types_t::TEST_BLOCK: return do_get_caching_extent( - offset, length, p_metric_key, std::move(extent_init_func), std::move(on_cache) + offset, length, 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 do_get_caching_extent( - offset, length, p_metric_key, std::move(extent_init_func), std::move(on_cache) + offset, length, 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 b2ab0e8c0cb..01be2a82277 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -331,8 +331,7 @@ public: } // get_extent_ret::ABSENT from transaction - auto metric_key = std::make_pair(t.get_src(), type); - ret = query_cache(offset, &metric_key); + ret = query_cache(offset); if (!ret) { SUBDEBUGT(seastore_cache, "{} {} is absent", t, type, offset); account_absent_access(t_src); @@ -425,11 +424,9 @@ public: const auto t_src = t.get_src(); touch_extent(ext, &t_src); }; - auto metric_key = std::make_pair(t.get_src(), T::TYPE); return trans_intr::make_interruptible( do_get_caching_extent( - offset, length, &metric_key, - [](T &){}, std::move(f)) + offset, length, [](T &){}, std::move(f)) ); } } @@ -472,11 +469,9 @@ public: t.add_to_read_set(CachedExtentRef(&ext)); touch_extent(ext, &t_src); }; - auto metric_key = std::make_pair(t.get_src(), T::TYPE); return trans_intr::make_interruptible( do_get_caching_extent( - offset, length, &metric_key, - std::forward(extent_init_func), std::move(f)) + offset, length, std::forward(extent_init_func), std::move(f)) ); } @@ -629,7 +624,7 @@ public: // Interfaces only for tests. public: CachedExtentRef test_query_cache(paddr_t offset) { - return query_cache(offset, nullptr); + return query_cache(offset); } private: @@ -645,12 +640,11 @@ private: read_extent_ret do_get_caching_extent( paddr_t offset, ///< [in] starting addr extent_len_t length, ///< [in] length - const src_ext_t* p_src_ext, ///< [in] cache query metric key Func &&extent_init_func, ///< [in] init func for extent OnCache &&on_cache ) { LOG_PREFIX(Cache::do_get_caching_extent); - auto cached = query_cache(offset, p_src_ext); + auto cached = query_cache(offset); if (!cached) { auto ret = CachedExtent::make_cached_extent_ref( alloc_cache_buf(length)); @@ -756,7 +750,6 @@ private: paddr_t offset, laddr_t laddr, extent_len_t length, - const Transaction::src_t* p_src, extent_init_func_t &&extent_init_func, extent_init_func_t &&on_cache ); @@ -811,10 +804,9 @@ private: const auto t_src = t.get_src(); touch_extent(ext, &t_src); }; - auto src = t.get_src(); return trans_intr::make_interruptible( do_get_caching_extent_by_type( - type, offset, laddr, length, &src, + type, offset, laddr, length, std::move(extent_init_func), std::move(f)) ); } @@ -854,10 +846,9 @@ private: t.add_to_read_set(CachedExtentRef(&ext)); touch_extent(ext, &t_src); }; - auto src = t.get_src(); return trans_intr::make_interruptible( do_get_caching_extent_by_type( - type, offset, laddr, length, &src, + type, offset, laddr, length, std::move(extent_init_func), std::move(f)) ); } @@ -1686,7 +1677,6 @@ private: counter_by_src_t trans_created_by_src; counter_by_src_t committed_efforts_by_src; counter_by_src_t invalidated_efforts_by_src; - counter_by_src_t cache_query_by_src; success_read_trans_efforts_t success_read_efforts; uint64_t dirty_bytes = 0; @@ -1869,21 +1859,9 @@ private: } // Extents in cache may contain placeholders - CachedExtentRef query_cache( - paddr_t offset, - const src_ext_t* p_metric_key) { - query_counters_t* p_counters = nullptr; - if (p_metric_key) { - p_counters = &get_by_src(stats.cache_query_by_src, p_metric_key->first); - ++p_counters->access; - } + CachedExtentRef query_cache(paddr_t offset) { if (auto iter = extents_index.find_offset(offset); iter != extents_index.end()) { - if (p_metric_key && - // retired_placeholder is not really cached yet - !is_retired_placeholder_type(iter->get_type())) { - ++p_counters->hit; - } assert(iter->is_stable()); return CachedExtentRef(&*iter); } else { -- 2.39.5