From 585c387922125a828064c95c3ed9c49b21344e0c Mon Sep 17 00:00:00 2001 From: Samuel Just Date: Fri, 17 Oct 2025 17:54:08 -0700 Subject: [PATCH] crimson/.../btree_lba_manager: simplify _update_mapping_ret, remove update_mapping_ret_bare_t Signed-off-by: Samuel Just --- .../os/seastore/lba/btree_lba_manager.cc | 13 ++- .../os/seastore/lba/btree_lba_manager.h | 83 +------------------ src/crimson/os/seastore/lba_manager.h | 12 --- 3 files changed, 8 insertions(+), 100 deletions(-) diff --git a/src/crimson/os/seastore/lba/btree_lba_manager.cc b/src/crimson/os/seastore/lba/btree_lba_manager.cc index 6cddf45a0a2..cb9b8c20024 100644 --- a/src/crimson/os/seastore/lba/btree_lba_manager.cc +++ b/src/crimson/os/seastore/lba/btree_lba_manager.cc @@ -730,10 +730,9 @@ BtreeLBAManager::update_mapping( "Invalid error in BtreeLBAManager::update_mapping" } ); - assert(res.is_alive_mapping()); DEBUGT("laddr={}, paddr {}~0x{:x} => {}~0x{:x}, crc=0x{:x} done -- {}", - t, laddr, prev_addr, prev_len, addr, len, checksum, res.get_cursor()); - co_return res.get_cursor().get_refcount(); + t, laddr, prev_addr, prev_len, addr, len, checksum, *cursor); + co_return res->get_refcount(); } BtreeLBAManager::update_mappings_ret @@ -787,7 +786,7 @@ BtreeLBAManager::update_mappings( checksum, FNAME](auto res) { DEBUGT("cursor={}, paddr {}~0x{:x} => {}, crc=0x{:x} done -- {}", c.trans, *cursor, prev_addr, len, - addr, checksum, res.get_cursor()); + addr, checksum, *res); return update_mapping_iertr::make_ready_future(); }, update_mapping_iertr::pass_further{}, @@ -864,14 +863,12 @@ BtreeLBAManager::_update_mapping( auto btree = co_await get_btree(cache, c); auto iter = btree.make_partial_iter(c, cursor); auto ret = f(iter.get_val()); - auto laddr = cursor.key; if (ret.refcount == 0) { iter = co_await btree.remove( c, iter ); - co_return update_mapping_ret_bare_t{ - laddr, std::move(ret), iter.get_cursor(c)}; + co_return iter.get_cursor(c); } else { iter = co_await btree.update( c, @@ -887,7 +884,7 @@ BtreeLBAManager::_update_mapping( && nextent->peek_parent_node().get() == iter.get_leaf_node().get())); LBACursorRef cursor = iter.get_cursor(c); assert(cursor->val); - co_return update_mapping_ret_bare_t{std::move(cursor)}; + co_return cursor; } } diff --git a/src/crimson/os/seastore/lba/btree_lba_manager.h b/src/crimson/os/seastore/lba/btree_lba_manager.h index 87b60e06d31..39c288e2f7c 100644 --- a/src/crimson/os/seastore/lba/btree_lba_manager.h +++ b/src/crimson/os/seastore/lba/btree_lba_manager.h @@ -181,7 +181,7 @@ public: Transaction &t, LBACursorRef cursor, int delta) final { - co_return (co_await _update_mapping( + co_return co_await _update_mapping( t, *cursor, [delta](lba_map_val_t ret) { @@ -193,7 +193,7 @@ public: ).handle_error_interruptible( base_iertr::pass_further{}, crimson::ct_error::assert_all{} - )).take_cursor(); + ); } remap_ret remap_mappings( @@ -309,89 +309,12 @@ private: seastar::metrics::metric_group metrics; void register_metrics(); - struct update_mapping_ret_bare_t { - update_mapping_ret_bare_t() - : update_mapping_ret_bare_t(LBACursorRef(nullptr)) {} - - update_mapping_ret_bare_t(LBACursorRef cursor) - : ret(std::move(cursor)) {} - - update_mapping_ret_bare_t( - laddr_t laddr, lba_map_val_t value, LBACursorRef &&cursor) - : ret(removed_mapping_t{laddr, value, std::move(cursor)}) {} - - struct removed_mapping_t { - laddr_t laddr; - lba_map_val_t map_value; - LBACursorRef next; - }; - std::variant ret; - - bool is_removed_mapping() const { - return ret.index() == 0; - } - - bool is_alive_mapping() const { - if (ret.index() == 1) { - assert(std::get<1>(ret)); - return true; - } else { - return false; - } - } - - removed_mapping_t &get_removed_mapping() { - assert(is_removed_mapping()); - return std::get<0>(ret); - } - - const removed_mapping_t& get_removed_mapping() const { - assert(is_removed_mapping()); - return std::get<0>(ret); - } - - const LBACursor& get_cursor() const { - assert(is_alive_mapping()); - return *std::get<1>(ret); - } - - LBACursorRef take_cursor() { - assert(is_alive_mapping()); - return std::move(std::get<1>(ret)); - } - }; - - mapping_update_result_t get_mapping_update_result( - update_mapping_ret_bare_t &result) { - if (result.is_removed_mapping()) { - auto &v = result.get_removed_mapping(); - auto &val = v.map_value; - return {v.laddr, - val.refcount, - val.pladdr, - val.len, - v.next->is_indirect() - ? LBAMapping::create_indirect(nullptr, std::move(v.next)) - : LBAMapping::create_direct(std::move(v.next))}; - } else { - assert(result.is_alive_mapping()); - auto &c = result.get_cursor(); - assert(c.val); - ceph_assert(!c.is_indirect()); - return {c.get_laddr(), c.val->refcount, - c.val->pladdr, c.val->len, - LBAMapping::create_direct(result.take_cursor())}; - } - } - /** * _update_mapping * * Updates mapping, removes if f returns nullopt */ - using _update_mapping_iertr = ref_iertr; - using _update_mapping_ret = ref_iertr::future< - update_mapping_ret_bare_t>; + using _update_mapping_ret = ref_iertr::future; using update_func_t = std::function< lba_map_val_t(const lba_map_val_t &v) >; diff --git a/src/crimson/os/seastore/lba_manager.h b/src/crimson/os/seastore/lba_manager.h index c1543d4bcf0..1de8a4a3598 100644 --- a/src/crimson/os/seastore/lba_manager.h +++ b/src/crimson/os/seastore/lba_manager.h @@ -126,18 +126,6 @@ public: laddr_t hint, extent_len_t len) = 0; - struct mapping_update_result_t { - laddr_t key; - extent_ref_count_t refcount = 0; - pladdr_t addr; - extent_len_t length = 0; - LBAMapping mapping; // the mapping pointing to the updated lba entry if - // refcount is non-zero; the next lba entry or the - // end mapping otherwise. - bool need_to_remove_extent() const { - return refcount == 0 && addr.is_paddr() && !addr.get_paddr().is_zero(); - } - }; using ref_iertr = base_iertr::extend< crimson::ct_error::enoent>; -- 2.47.3