From: Samuel Just Date: Wed, 8 Oct 2025 01:47:52 +0000 (-0700) Subject: crimson/.../transaction_manager: rework _remove in terms of LBACursor interfaces X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=f31f55dd836c753c8494c9220780da4b66bf46a4;p=ceph-ci.git crimson/.../transaction_manager: rework _remove in terms of LBACursor interfaces Removes the need for _remove_direct_mapping and _remove_indirect_mapping. Signed-off-by: Samuel Just --- diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc index 0cc6e7e24bb..d46fb8f5005 100644 --- a/src/crimson/os/seastore/transaction_manager.cc +++ b/src/crimson/os/seastore/transaction_manager.cc @@ -263,131 +263,65 @@ TransactionManager::_remove_indirect_mapping_only( } TransactionManager::ref_iertr::future -TransactionManager::_remove_indirect_mapping( +TransactionManager::_remove( Transaction &t, LBAMapping mapping) { - LOG_PREFIX(TransactionManager::_remove_indirect_mapping); - mapping = co_await complete_mapping(t, std::move(mapping)); - 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; - ceph_assert(primary_result.refcount == 0); - auto &direct_result = *result.direct_result; - ceph_assert(direct_result.addr.is_paddr()); - ceph_assert(!direct_result.addr.get_paddr().is_zero()); - ceph_assert(extent); - if (direct_result.refcount == 0) { - cache->retire_extent(t, extent); - } - DEBUGT("removed indirect mapping {}~0x{:x} refcount={} offset={} " - "with direct mapping {}~0x{:x} refcount={} offset={}", - t, primary_result.addr, - primary_result.length, - primary_result.refcount, - primary_result.key, - direct_result.addr, - direct_result.length, - direct_result.refcount, - direct_result.key); - co_return result.result.mapping; - } else { - auto remove_direct = mapping.would_cascade_remove(); - if (remove_direct) { + LOG_PREFIX(TransactionManager::_remove); + DEBUGT("{}", t, mapping); + mapping = co_await complete_mapping(t, mapping); + + if (!mapping.is_zero_reserved() && + mapping.direct_cursor->get_refcount() == 1) { + auto maybe_mapped_extent = get_extent_if_linked(t, *(mapping.direct_cursor)); + if (maybe_mapped_extent.has_child()) { + DEBUGT("waiting for child fut for {}", t, mapping); + auto extent = co_await maybe_mapped_extent.get_child_fut_as< + LogicalChildNode + >(); + ceph_assert(extent); + cache->retire_extent(t, std::move(extent)); + } else { auto retired_placeholder = cache->retire_absent_extent_addr( - t, mapping.get_intermediate_base(), - mapping.get_val(), - mapping.get_intermediate_length() + t, mapping.get_key(), mapping.get_val(), mapping.get_length() )->template cast(); - ret.get_child_pos().link_child(retired_placeholder.get()); + maybe_mapped_extent.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); - auto &primary_result = result.result; - ceph_assert(primary_result.refcount == 0); - auto &direct_result = *result.direct_result; - ceph_assert(direct_result.addr.is_paddr()); - ceph_assert(!direct_result.addr.get_paddr().is_zero()); - ceph_assert(remove_direct == (direct_result.refcount == 0)); - DEBUGT("removed indirect mapping {}~0x{:x} refcount={} offset={} " - "with direct mapping {}~0x{:x} refcount={} offset={}", - t, primary_result.addr, - primary_result.length, - primary_result.refcount, - primary_result.key, - direct_result.addr, - direct_result.length, - direct_result.refcount, - direct_result.key); - co_return result.result.mapping; - } -} - -TransactionManager::ref_iertr::future -TransactionManager::_remove_direct_mapping( - Transaction &t, - LBAMapping mapping) -{ - LOG_PREFIX(TransactionManager::_remove_direct_mapping); - 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); - ceph_assert(primary_result.addr.is_paddr()); - ceph_assert(!primary_result.addr.get_paddr().is_zero()); - ceph_assert(extent); - cache->retire_extent(t, extent); - DEBUGT("removed {}~0x{:x} refcount={} -- offset={}", - t, primary_result.addr, primary_result.length, - primary_result.refcount, primary_result.key); - co_return result.result.mapping; - } else { - auto retired_placeholder = cache->retire_absent_extent_addr( - t, mapping.get_key(), mapping.get_val(), mapping.get_length() - )->template cast(); - 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); - ceph_assert(primary_result.addr.is_paddr()); - ceph_assert(!primary_result.addr.get_paddr().is_zero()); - DEBUGT("removed {}~0x{:x} refcount={} -- offset={}", - t, primary_result.addr, primary_result.length, - primary_result.refcount, primary_result.key); - co_return result.result.mapping; } -} -TransactionManager::ref_iertr::future -TransactionManager::_remove( - Transaction &t, - LBAMapping mapping) -{ - LOG_PREFIX(TransactionManager::_remove); + LBACursorRef indirect_cursor; if (mapping.is_indirect()) { - return _remove_indirect_mapping(t, std::move(mapping)); - } else if (mapping.get_val().is_real_location()) { - return _remove_direct_mapping(t, std::move(mapping)); - } else { - return lba_manager->remove_mapping(t, std::move(mapping) - ).si_then([&t, FNAME](auto result) { - auto &primary_result = result.result; - ceph_assert(primary_result.refcount == 0); - ceph_assert(primary_result.addr.is_paddr()); - ceph_assert(primary_result.addr.get_paddr().is_zero()); - DEBUGT("removed {}~0x{:x} refcount={} -- offset={}", - t, primary_result.addr, - primary_result.length, - primary_result.refcount, - primary_result.key); - return result.result.mapping; - }); + DEBUGT("removing indirect mapping {}~0x{:x} refcount={} -- offset={}", + t, + mapping.indirect_cursor->get_intermediate_key(), + mapping.indirect_cursor->get_length(), + mapping.indirect_cursor->get_refcount(), + mapping.indirect_cursor->get_laddr()); + ceph_assert(mapping.indirect_cursor->get_refcount() == 1); + indirect_cursor = co_await lba_manager->update_mapping_refcount( + t, mapping.indirect_cursor, -1); + co_await mapping.direct_cursor->refresh(); } + + DEBUGT("removing direct mapping {}~0x{:x} refcount={} -- offset={}", + t, + mapping.direct_cursor->get_paddr(), + mapping.direct_cursor->get_length(), + mapping.direct_cursor->get_refcount(), + mapping.direct_cursor->get_laddr()); + + ceph_assert(mapping.direct_cursor->get_refcount() >= 1); + + LBACursorRef direct_cursor = co_await lba_manager->update_mapping_refcount( + t, mapping.direct_cursor, -1); + + auto ret = co_await resolve_cursor_to_mapping( + t, + indirect_cursor ? std::move(indirect_cursor) : std::move(direct_cursor) + ); + DEBUGT("returning {}", t, ret); + ceph_assert(ret.is_viewable()); + co_return ret; } using resolve_cursor_to_mapping_iertr = base_iertr; diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index a7572e11cfb..6b4de7e121b 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -1364,14 +1364,7 @@ private: ref_iertr::future _remove( Transaction &t, LBAMapping mapping); - ref_iertr::future - _remove_indirect_mapping( - Transaction &t, - LBAMapping mapping); - ref_iertr::future - _remove_direct_mapping( - Transaction &t, - LBAMapping mapping); + ref_iertr::future _remove_indirect_mapping_only( Transaction &t,