From e3c9e511e8cb688db9e81fb17c3f83ca2b56bdb5 Mon Sep 17 00:00:00 2001 From: Samuel Just Date: Mon, 13 Oct 2025 22:50:05 +0000 Subject: [PATCH] crimson/.../lba_manager: remove get_mapping interfaces Signed-off-by: Samuel Just --- .../os/seastore/lba/btree_lba_manager.cc | 103 +----------------- .../os/seastore/lba/btree_lba_manager.h | 15 +-- src/crimson/os/seastore/lba_manager.h | 37 ------- src/crimson/os/seastore/transaction_manager.h | 7 +- .../seastore/test_btree_lba_manager.cc | 16 +-- 5 files changed, 14 insertions(+), 164 deletions(-) diff --git a/src/crimson/os/seastore/lba/btree_lba_manager.cc b/src/crimson/os/seastore/lba/btree_lba_manager.cc index dffed25cac7..4a0441da27b 100644 --- a/src/crimson/os/seastore/lba/btree_lba_manager.cc +++ b/src/crimson/os/seastore/lba/btree_lba_manager.cc @@ -185,37 +185,6 @@ BtreeLBAManager::get_cursor( co_return btree.get_cursor(c, leaf, extent.get_laddr()); } -BtreeLBAManager::get_mappings_ret -BtreeLBAManager::get_mappings( - Transaction &t, - laddr_t laddr, - extent_len_t length) -{ - LOG_PREFIX(BtreeLBAManager::get_mappings); - TRACET("{}~0x{:x} ...", t, laddr, length); - auto c = get_context(t); - - lba_mapping_list_t ret; - auto btree = co_await get_btree(cache, c); - - auto cursors = co_await get_cursors(c, btree, laddr, length); - for (auto &cursor: cursors) { - assert(!cursor->is_end()); - if (!cursor->is_indirect()) { - ret.emplace_back(LBAMapping::create_direct(std::move(cursor))); - } else { - assert(cursor->val->refcount == EXTENT_DEFAULT_REF_COUNT); - assert(cursor->val->checksum == 0); - auto direct = co_await this->resolve_indirect_cursor(c, btree, *cursor); - ret.emplace_back(LBAMapping::create_indirect( - std::move(direct), std::move(cursor))); - } - TRACET("{}~0x{:x} got {}", - c.trans, laddr, length, ret.back()); - } - co_return ret; -} - BtreeLBAManager::get_cursors_ret BtreeLBAManager::get_cursors( op_context_t c, @@ -268,76 +237,6 @@ BtreeLBAManager::resolve_indirect_cursor( }); } -BtreeLBAManager::get_mapping_ret -BtreeLBAManager::get_mapping( - Transaction &t, - laddr_t laddr, - bool search_containing) -{ - LOG_PREFIX(BtreeLBAManager::get_mapping); - TRACET("{} ... search_containing={}", t, laddr, search_containing); - auto c = get_context(t); - auto btree = co_await get_btree(cache, c); - - LBACursorRef cursor = co_await ( - search_containing ? - get_containing_cursor(c, btree, laddr) : - get_cursor(c, btree, laddr)); - - assert(!cursor->is_end()); - if (!cursor->is_indirect()) { - TRACET("{} got direct cursor {}", - c.trans, laddr, *cursor); - co_return LBAMapping::create_direct(std::move(cursor)); - } - - if (search_containing) { - assert(cursor->contains(laddr)); - } else { - assert(laddr == cursor->get_laddr()); - } - assert(cursor->val->refcount == EXTENT_DEFAULT_REF_COUNT); - assert(cursor->val->checksum == 0); - auto direct = co_await resolve_indirect_cursor(c, btree, *cursor); - auto mapping = LBAMapping::create_indirect( - std::move(direct), std::move(cursor)); - TRACET("{} got indirect mapping {}", - c.trans, laddr, mapping); - co_return mapping; -} - -BtreeLBAManager::get_mapping_ret -BtreeLBAManager::get_mapping( - Transaction &t, - LogicalChildNode &extent) -{ - LOG_PREFIX(BtreeLBAManager::get_mapping); - TRACET("{}", t, extent); -#ifndef NDEBUG - if (extent.is_mutation_pending()) { - auto &prior = static_cast( - *extent.get_prior_instance()); - assert(prior.peek_parent_node()->is_valid()); - } else { - assert(extent.peek_parent_node()->is_valid()); - } -#endif - auto c = get_context(t); - auto btree = co_await get_btree(cache, c); - - auto leaf = co_await extent.get_parent_node(c.trans, c.cache); - - if (leaf->is_pending()) { - TRACET("find pending extent {} for {}", - c.trans, (void*)leaf.get(), extent); - } -#ifndef NDEBUG - auto it = leaf->lower_bound(extent.get_laddr()); - assert(it != leaf->end() && it.get_key() == extent.get_laddr()); -#endif - co_return LBAMapping::create_direct(btree.get_cursor(c, leaf, extent.get_laddr())); -} - BtreeLBAManager::alloc_extent_ret BtreeLBAManager::reserve_region( Transaction &t, @@ -1162,7 +1061,7 @@ BtreeLBAManager::get_containing_cursor( } TRACET("{} got {}, {}", c.trans, laddr, iter.get_key(), iter.get_val()); - return get_mapping_iertr::make_ready_future< + return get_cursor_iertr::make_ready_future< LBACursorRef>(iter.get_cursor(c)); }); } diff --git a/src/crimson/os/seastore/lba/btree_lba_manager.h b/src/crimson/os/seastore/lba/btree_lba_manager.h index d330c8b85c2..7027430e262 100644 --- a/src/crimson/os/seastore/lba/btree_lba_manager.h +++ b/src/crimson/os/seastore/lba/btree_lba_manager.h @@ -75,19 +75,6 @@ public: Transaction &t, LogicalChildNode &extent) final; - get_mappings_ret get_mappings( - Transaction &t, - laddr_t offset, extent_len_t length) final; - - get_mapping_ret get_mapping( - Transaction &t, - laddr_t offset, - bool search_containing = false) final; - - get_mapping_ret get_mapping( - Transaction &t, - LogicalChildNode &extent) final; - alloc_extent_ret reserve_region( Transaction &t, LBAMapping pos, @@ -506,7 +493,7 @@ private: laddr_t offset, extent_len_t length); - using resolve_indirect_cursor_ret = get_mappings_iertr::future; + using resolve_indirect_cursor_ret = base_iertr::future; resolve_indirect_cursor_ret resolve_indirect_cursor( op_context_t c, LBABtree& btree, diff --git a/src/crimson/os/seastore/lba_manager.h b/src/crimson/os/seastore/lba_manager.h index 80d815dab92..cb101af592a 100644 --- a/src/crimson/os/seastore/lba_manager.h +++ b/src/crimson/os/seastore/lba_manager.h @@ -52,43 +52,6 @@ public: Transaction &t, LogicalChildNode &extent) = 0; - /** - * Fetches mappings for laddr_t in range [offset, offset + len) - * - * Future will not resolve until all pins have resolved (set_paddr called) - * For indirect lba mappings, get_mappings will always retrieve the original - * lba value. - */ - using get_mappings_iertr = base_iertr; - using get_mappings_ret = get_mappings_iertr::future; - virtual get_mappings_ret get_mappings( - Transaction &t, - laddr_t offset, extent_len_t length) = 0; - - /** - * Fetches the mapping for laddr_t - * - * Future will not resolve until the pin has resolved (set_paddr called) - * For indirect lba mappings, get_mapping will always retrieve the original - * lba value. - */ - using get_mapping_iertr = base_iertr::extend< - crimson::ct_error::enoent>; - using get_mapping_ret = get_mapping_iertr::future; - virtual get_mapping_ret get_mapping( - Transaction &t, - laddr_t offset, - bool search_containing = false) = 0; - - /* - * Fetches the mapping corresponding to the "extent" - * - */ - virtual get_mapping_ret get_mapping( - Transaction &t, - LogicalChildNode &extent) = 0; - - #ifdef UNIT_TESTS_BUILT using get_end_mapping_iertr = base_iertr; using get_end_mapping_ret = get_end_mapping_iertr::future; diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index c6f57c75ab7..3ebc12b1c83 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -109,8 +109,9 @@ public: * * Get the logical pin at offset */ - using get_pin_iertr = LBAManager::get_mapping_iertr; - using get_pin_ret = LBAManager::get_mapping_iertr::future; + using get_pin_iertr = base_iertr::extend< + crimson::ct_error::enoent>; + using get_pin_ret = get_pin_iertr::future; get_pin_ret get_pin( Transaction &t, laddr_t offset) { @@ -153,7 +154,7 @@ public: * * Get logical pins overlapping offset~length */ - using get_pins_iertr = LBAManager::get_mappings_iertr; + using get_pins_iertr = base_iertr; using get_pins_ret = get_pins_iertr::future; get_pins_ret get_pins( Transaction &t, diff --git a/src/test/crimson/seastore/test_btree_lba_manager.cc b/src/test/crimson/seastore/test_btree_lba_manager.cc index 33abb1b43f4..50e602d2ccd 100644 --- a/src/test/crimson/seastore/test_btree_lba_manager.cc +++ b/src/test/crimson/seastore/test_btree_lba_manager.cc @@ -656,24 +656,24 @@ struct btree_lba_manager_test : btree_test_base { auto ret_list = with_trans_intr( *t.t, [=, this](auto &t) { - return lba_manager->get_mappings( + return lba_manager->get_cursors( t, laddr, len); }).unsafe_get(); EXPECT_EQ(ret_list.size(), 1); auto &ret = *ret_list.begin(); - EXPECT_EQ(i.second.addr, ret.get_val()); - EXPECT_EQ(laddr, ret.get_key()); - EXPECT_EQ(len, ret.get_length()); + EXPECT_EQ(i.second.addr, ret->get_paddr()); + EXPECT_EQ(laddr, ret->get_laddr()); + EXPECT_EQ(len, ret->get_length()); auto ret_pin = with_trans_intr( *t.t, [=, this](auto &t) { - return lba_manager->get_mapping( + return lba_manager->get_cursor( t, laddr); }).unsafe_get(); - EXPECT_EQ(i.second.addr, ret_pin.get_val()); - EXPECT_EQ(laddr, ret_pin.get_key()); - EXPECT_EQ(len, ret_pin.get_length()); + EXPECT_EQ(i.second.addr, ret_pin->get_paddr()); + EXPECT_EQ(laddr, ret_pin->get_laddr()); + EXPECT_EQ(len, ret_pin->get_length()); } with_trans_intr( *t.t, -- 2.47.3