From: Samuel Just Date: Tue, 7 Oct 2025 00:27:18 +0000 (+0000) Subject: crimson/.../transaction_manager: simplify get_extent_if_linked X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=872175ae242f0d2dea6941116a9b6f94b550d5d4;p=ceph-ci.git crimson/.../transaction_manager: simplify get_extent_if_linked Signed-off-by: Samuel Just --- diff --git a/src/crimson/os/seastore/linked_tree_node.h b/src/crimson/os/seastore/linked_tree_node.h index 308f53ff8dc..e6452eda428 100644 --- a/src/crimson/os/seastore/linked_tree_node.h +++ b/src/crimson/os/seastore/linked_tree_node.h @@ -82,6 +82,13 @@ struct get_child_ret_t { ceph_assert(ret.index() == 1); return std::get<1>(ret); } + + template + get_child_ifut get_child_fut_as() { + return std::move(get_child_fut()).si_then([](auto e) { + return e->template cast(); + }); + } }; template diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc index 9d84fa6069f..59812078171 100644 --- a/src/crimson/os/seastore/transaction_manager.cc +++ b/src/crimson/os/seastore/transaction_manager.cc @@ -269,9 +269,9 @@ TransactionManager::_remove_indirect_mapping( LOG_PREFIX(TransactionManager::_remove_indirect_mapping); mapping = co_await lba_manager->complete_indirect_lba_mapping(t, std::move(mapping) ); - auto ret = get_extent_if_linked(t, mapping); - if (ret.index() == 1) { - auto extent = co_await std::move(std::get<1>(ret)); + auto ret = get_extent_if_linked(t, *(mapping.direct_cursor)); + if (ret.has_child()) { + auto extent = co_await ret.template get_child_fut_as(); auto result = co_await lba_manager->remove_mapping(t, std::move(mapping)); ceph_assert(result.direct_result); auto &primary_result = result.result; @@ -297,13 +297,12 @@ TransactionManager::_remove_indirect_mapping( } else { auto remove_direct = mapping.would_cascade_remove(); if (remove_direct) { - auto unlinked_child = std::move(std::get<0>(ret)); auto retired_placeholder = cache->retire_absent_extent_addr( t, mapping.get_intermediate_base(), mapping.get_val(), mapping.get_intermediate_length() )->template cast(); - unlinked_child.child_pos.link_child(retired_placeholder.get()); + ret.get_child_pos().link_child(retired_placeholder.get()); } auto result = co_await lba_manager->remove_mapping(t, std::move(mapping)); ceph_assert(result.direct_result); @@ -334,9 +333,9 @@ TransactionManager::_remove_direct_mapping( LBAMapping mapping) { LOG_PREFIX(TransactionManager::_remove_direct_mapping); - auto ret = get_extent_if_linked(t, mapping); - if (ret.index() == 1) { - auto extent = co_await std::move(std::get<1>(ret)); + auto ret = get_extent_if_linked(t, *(mapping.direct_cursor)); + if (ret.has_child()) { + auto extent = co_await ret.template get_child_fut_as(); auto result = co_await lba_manager->remove_mapping(t, std::move(mapping)); auto &primary_result = result.result; ceph_assert(primary_result.refcount == 0); @@ -349,11 +348,10 @@ TransactionManager::_remove_direct_mapping( primary_result.refcount, primary_result.key); co_return result; } else { - auto unlinked_child = std::move(std::get<0>(ret)); auto retired_placeholder = cache->retire_absent_extent_addr( t, mapping.get_key(), mapping.get_val(), mapping.get_length() )->template cast(); - unlinked_child.child_pos.link_child(retired_placeholder.get()); + ret.get_child_pos().link_child(retired_placeholder.get()); auto result = co_await lba_manager->remove_mapping(t, std::move(mapping)); auto &primary_result = result.result; ceph_assert(primary_result.refcount == 0); diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index a0ea3e41e12..8bea388dfaf 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -303,10 +303,10 @@ public: // checking the lba child must be atomic with creating // and linking the absent child - auto ret = get_extent_if_linked(t, std::move(pin)); + auto ret = get_extent_if_linked(t, *(pin.direct_cursor)); TCachedExtentRef extent; - if (ret.index() == 1) { - extent = co_await std::move(std::get<1>(ret)); + if (ret.has_child()) { + extent = co_await ret.template get_child_fut_as(); extent = co_await cache->read_extent_maybe_partial( t, std::move(extent), direct_partial_off, partial_len); if (!extent->is_seen_by_users()) { @@ -314,9 +314,8 @@ public: extent->set_seen_by_users(); } } else { - auto &r = std::get<0>(ret); extent = co_await this->pin_to_extent( - t, std::move(r.mapping), std::move(r.child_pos), + t, std::move(pin), std::move(ret.get_child_pos()), direct_partial_off, partial_len, std::move(maybe_init)); } @@ -1160,35 +1159,19 @@ private: LBACursorRef cursor); using LBALeafNode = lba::LBALeafNode; - struct unlinked_child_t { - LBAMapping mapping; - child_pos_t child_pos; - }; - template - std::variant> - get_extent_if_linked( + auto get_extent_if_linked( Transaction &t, - LBAMapping pin) + LBACursor &cursor) { - ceph_assert(pin.is_viewable()); - // checking the lba child must be atomic with creating - // and linking the absent child - auto v = pin.get_logical_extent(t); - if (v.has_child()) { - return v.get_child_fut( - ).si_then([pin](auto extent) { -#ifndef NDEBUG - auto lextent = extent->template cast(); - auto pin_laddr = pin.get_intermediate_base(); - assert(lextent->get_laddr() == pin_laddr); -#endif - return extent->template cast(); - }); - } else { - return unlinked_child_t{ - std::move(const_cast(pin)), - v.get_child_pos()}; - } + ceph_assert(cursor.is_viewable()); + ceph_assert(cursor.ctx.trans.get_trans_id() + == t.get_trans_id()); + assert(!cursor.is_end()); + assert(cursor.pos != BTREENODE_POS_NULL); + ceph_assert(t.get_trans_id() == cursor.ctx.trans.get_trans_id()); + auto p = cursor.parent->cast(); + return p->template get_child( + t, cursor.ctx.cache, cursor.pos, cursor.key); } base_iertr::future read_pin_by_type( @@ -1203,7 +1186,7 @@ private: // checking the lba child must be atomic with creating // and linking the absent child if (v.has_child()) { - auto extent = co_await std::move(v.get_child_fut()); + auto extent = co_await v.get_child_fut_as(); auto len = extent->get_length(); auto ext = co_await cache->read_extent_maybe_partial( t, std::move(extent), 0, len); @@ -1286,20 +1269,19 @@ private: assert(!maybe_indirect_extent.is_clone); ext = maybe_indirect_extent.extent; } else { - auto ret = get_extent_if_linked(t, pin); - if (ret.index() == 1) { - ext = co_await std::move(std::get<1>(ret)); + auto ret = get_extent_if_linked(t, *(pin.direct_cursor)); + if (ret.has_child()) { + ext = co_await ret.template get_child_fut_as(); if (!ext->is_seen_by_users()) { // Note, no maybe_init available for data extents ext->set_seen_by_users(); } } else { // absent - auto unlinked_child = std::move(std::get<0>(ret)); auto retired_placeholder = cache->retire_absent_extent_addr( t, pin.get_key(), original_paddr, original_len )->template cast(); - unlinked_child.child_pos.link_child(retired_placeholder.get()); + ret.get_child_pos().link_child(retired_placeholder.get()); // leave ext null } }