From 1dbdc3e5c0ceea24db7566981c765af06c64ffea Mon Sep 17 00:00:00 2001 From: Zhang Song Date: Thu, 24 Apr 2025 17:39:50 +0800 Subject: [PATCH] crimson/os/seastore: simplify the processing of indirect LBAMapping Signed-off-by: Zhang Song (cherry picked from commit f2a804b64f928e878a6d30c39efc853163e88e7f) --- src/crimson/os/seastore/cached_extent.cc | 6 ---- src/crimson/os/seastore/cached_extent.h | 2 -- src/crimson/os/seastore/transaction_manager.h | 31 ++++++------------- .../seastore/test_transaction_manager.cc | 14 +++------ 4 files changed, 13 insertions(+), 40 deletions(-) diff --git a/src/crimson/os/seastore/cached_extent.cc b/src/crimson/os/seastore/cached_extent.cc index 23e221392df2d..44127df625a03 100644 --- a/src/crimson/os/seastore/cached_extent.cc +++ b/src/crimson/os/seastore/cached_extent.cc @@ -104,12 +104,6 @@ void CachedExtent::set_invalid(Transaction &t) { on_invalidated(t); } -void LogicalCachedExtent::maybe_set_intermediate_laddr(LBAMapping &mapping) { - laddr = mapping.is_indirect() - ? mapping.get_intermediate_base() - : mapping.get_key(); -} - std::pair CachedExtent::is_viewable_by_trans(Transaction &t) { if (!is_valid()) { diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h index 02730f9ad625d..dd34c8fbdabf6 100644 --- a/src/crimson/os/seastore/cached_extent.h +++ b/src/crimson/os/seastore/cached_extent.h @@ -1415,8 +1415,6 @@ public: laddr = nladdr; } - void maybe_set_intermediate_laddr(LBAMapping &mapping); - void apply_delta_and_adjust_crc( paddr_t base, const ceph::bufferlist &bl) final { apply_delta(bl); diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index 348f27b8366dd..c506bbf1c5ff9 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -972,10 +972,7 @@ private: ).si_then([pin=std::move(pin)](auto extent) { #ifndef NDEBUG auto lextent = extent->template cast(); - auto pin_laddr = pin.get_key(); - if (pin.is_indirect()) { - pin_laddr = pin.get_intermediate_base(); - } + auto pin_laddr = pin.get_intermediate_base(); assert(lextent->get_laddr() == pin_laddr); #endif return extent->template cast(); @@ -1045,9 +1042,7 @@ private: // must be user-oriented required by maybe_init assert(is_user_transaction(t.get_src())); using ret = pin_to_extent_ret; - auto direct_length = pin.is_indirect() ? - pin.get_intermediate_length() : - pin.get_length(); + auto direct_length = pin.get_intermediate_length(); if (full_extent_integrity_check) { direct_partial_off = 0; partial_len = direct_length; @@ -1061,15 +1056,15 @@ private: direct_length, direct_partial_off, partial_len, - [pin=pin.duplicate(), maybe_init=std::move(maybe_init), + [laddr=pin.get_intermediate_base(), + maybe_init=std::move(maybe_init), child_pos=std::move(child_pos)] (T &extent) mutable { assert(extent.is_logical()); assert(!extent.has_laddr()); assert(!extent.has_been_invalidated()); - assert(pin.is_valid()); child_pos.link_child(&extent); - extent.maybe_set_intermediate_laddr(pin); + extent.set_laddr(laddr); maybe_init(extent); extent.set_seen_by_users(); } @@ -1126,29 +1121,21 @@ private: t, pin, type); assert(is_logical_type(type)); assert(is_background_transaction(t.get_src())); - laddr_t direct_key; - extent_len_t direct_length; - if (pin.is_indirect()) { - direct_key = pin.get_intermediate_base(); - direct_length = pin.get_intermediate_length(); - } else { - direct_key = pin.get_key(); - direct_length = pin.get_length(); - } + laddr_t direct_key = pin.get_intermediate_base(); + extent_len_t direct_length = pin.get_intermediate_length(); return cache->get_absent_extent_by_type( t, type, pin.get_val(), direct_key, direct_length, - [pin=pin.duplicate(), child_pos=std::move(child_pos)](CachedExtent &extent) mutable { + [direct_key, child_pos=std::move(child_pos)](CachedExtent &extent) mutable { assert(extent.is_logical()); auto &lextent = static_cast(extent); assert(!lextent.has_laddr()); assert(!lextent.has_been_invalidated()); - assert(pin.is_valid()); child_pos.link_child(&lextent); - lextent.maybe_set_intermediate_laddr(pref); + lextent.set_laddr(direct_key); // No change to extent::seen_by_user because this path is only // for background cleaning. } diff --git a/src/test/crimson/seastore/test_transaction_manager.cc b/src/test/crimson/seastore/test_transaction_manager.cc index ba0c5c0691360..334bf1ac106b1 100644 --- a/src/test/crimson/seastore/test_transaction_manager.cc +++ b/src/test/crimson/seastore/test_transaction_manager.cc @@ -497,12 +497,8 @@ struct transaction_manager_test_t : TestBlockRef read_pin( test_transaction_t &t, LBAMapping pin) { - auto addr = pin.is_indirect() - ? pin.get_intermediate_base() - : pin.get_key(); - auto len = pin.is_indirect() - ? pin.get_intermediate_length() - : pin.get_length(); + auto addr = pin.get_intermediate_base(); + auto len = pin.get_intermediate_length(); ceph_assert(test_mappings.contains(addr, t.mapping_delta)); ceph_assert(test_mappings.get(addr, t.mapping_delta).desc.len == len); @@ -585,7 +581,7 @@ struct transaction_manager_test_t : using ertr = with_trans_ertr; bool indirect = pin.is_indirect(); auto addr = pin.get_key(); - auto im_addr = indirect ? pin.get_intermediate_base() : L_ADDR_NULL; + auto im_addr = pin.get_intermediate_base(); auto ext = with_trans_intr(*(t.t), [&](auto& trans) { return tm->read_pin(trans, std::move(pin)); }).safe_then([](auto ret) { @@ -1108,9 +1104,7 @@ struct transaction_manager_test_t : } auto o_laddr = opin.get_key(); bool indirect_opin = opin.is_indirect(); - auto data_laddr = indirect_opin - ? opin.get_intermediate_base() - : o_laddr; + auto data_laddr = opin.get_intermediate_base(); auto pin = with_trans_intr(*(t.t), [&](auto& trans) { return tm->remap_pin( trans, std::move(opin), std::array{ -- 2.39.5