From: Zhang Song Date: Thu, 24 Apr 2025 09:42:10 +0000 (+0800) Subject: crimson/os/seastore/BtreeLBAManager: support refresh LBAMapping X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=89d10652333e415eef9a1ab9a37b710296d98e00;p=ceph.git crimson/os/seastore/BtreeLBAManager: support refresh LBAMapping Signed-off-by: Zhang Song (cherry picked from commit dca4e562f2d92c1a481256518e5999b8f3e3ec4a) --- diff --git a/src/crimson/os/seastore/lba_manager.h b/src/crimson/os/seastore/lba_manager.h index 120e3b9b962c3..286496f3c71e9 100644 --- a/src/crimson/os/seastore/lba_manager.h +++ b/src/crimson/os/seastore/lba_manager.h @@ -230,6 +230,12 @@ public: laddr_t laddr, extent_len_t len) = 0; + using refresh_lba_mapping_iertr = base_iertr; + using refresh_lba_mapping_ret = refresh_lba_mapping_iertr::future; + virtual refresh_lba_mapping_ret refresh_lba_mapping( + Transaction &t, + LBAMapping mapping) = 0; + virtual ~LBAManager() {} }; using LBAManagerRef = std::unique_ptr; diff --git a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc index 8914e8d5b12bf..3f0700fd701cb 100644 --- a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc +++ b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc @@ -721,6 +721,100 @@ BtreeLBAManager::get_physical_extent_if_live( }); } +BtreeLBAManager::refresh_lba_mapping_ret +BtreeLBAManager::refresh_lba_mapping(Transaction &t, LBAMapping mapping) +{ + assert(mapping.is_linked_direct()); + if (mapping.is_viewable()) { + return refresh_lba_mapping_iertr::make_ready_future< + LBAMapping>(std::move(mapping)); + } + auto c = get_context(t); + return with_btree_state( + cache, + c, + std::move(mapping), + [c, this](LBABtree &btree, LBAMapping &mapping) mutable + { + return refresh_lba_cursor(c, btree, *mapping.direct_cursor + ).si_then([c, this, &btree, &mapping] { + if (mapping.indirect_cursor) { + return refresh_lba_cursor(c, btree, *mapping.indirect_cursor); + } + return refresh_lba_cursor_iertr::make_ready_future(); +#ifndef NDEBUG + }).si_then([&mapping] { + assert(mapping.is_viewable()); +#endif + }); + }); +} + +BtreeLBAManager::refresh_lba_cursor_ret +BtreeLBAManager::refresh_lba_cursor( + op_context_t c, + LBABtree &btree, + LBACursor &cursor) +{ + LOG_PREFIX(BtreeLBAManager::refresh_lba_cursor); + stats.num_refresh_parent_total++; + + if (!cursor.parent->is_valid()) { + stats.num_refresh_invalid_parent++; + TRACET("cursor {} parent is invalid, re-search from scratch", + c.trans, cursor); + return btree.lower_bound(c, cursor.get_laddr() + ).si_then([&cursor](LBABtree::iterator iter) { + auto leaf = iter.get_leaf_node(); + cursor.parent = leaf; + cursor.modifications = leaf->modifications; + cursor.pos = iter.get_leaf_pos(); + if (!cursor.is_end()) { + ceph_assert(!iter.is_end()); + ceph_assert(iter.get_key() == cursor.get_laddr()); + cursor.val = iter.get_val(); + assert(cursor.is_viewable()); + } + }); + } + + auto [viewable, state] = cursor.parent->is_viewable_by_trans(c.trans); + auto leaf = cursor.parent->cast(); + + TRACET("cursor: {} viewable: {} state: {}", + c.trans, cursor, viewable, state); + + if (!viewable) { + stats.num_refresh_unviewable_parent++; + leaf = leaf->find_pending_version(c.trans, cursor.get_laddr()); + cursor.parent = leaf; + } + + if (!viewable || + leaf->modified_since(cursor.modifications)) { + if (viewable) { + stats.num_refresh_modified_viewable_parent++; + } + + cursor.modifications = leaf->modifications; + if (cursor.is_end()) { + cursor.pos = leaf->get_size(); + assert(!cursor.val); + } else { + auto i = leaf->lower_bound(cursor.get_laddr()); + cursor.pos = i.get_offset(); + cursor.val = i.get_val(); + + auto iter = LBALeafNode::iterator(leaf.get(), cursor.pos); + ceph_assert(iter.get_key() == cursor.key); + ceph_assert(iter.get_val() == cursor.val); + assert(cursor.is_viewable()); + } + } + + return refresh_lba_cursor_iertr::make_ready_future(); +} + void BtreeLBAManager::register_metrics() { LOG_PREFIX(BtreeLBAManager::register_metrics); @@ -740,6 +834,26 @@ void BtreeLBAManager::register_metrics() stats.num_alloc_extents_iter_nexts, sm::description("total number of iterator next operations during extent allocation") ), + sm::make_counter( + "refresh_parent_total", + stats.num_refresh_parent_total, + sm::description("total number of refreshed cursors") + ), + sm::make_counter( + "refresh_invalid_parent", + stats.num_refresh_invalid_parent, + sm::description("total number of refreshed cursors with invalid parents") + ), + sm::make_counter( + "refresh_unviewable_parent", + stats.num_refresh_unviewable_parent, + sm::description("total number of refreshed cursors with unviewable parents") + ), + sm::make_counter( + "refresh_modified_viewable_parent", + stats.num_refresh_modified_viewable_parent, + sm::description("total number of refreshed cursors with viewable but modified parents") + ), } ); } diff --git a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.h b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.h index 33cdcee8e5a24..8a6698072b389 100644 --- a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.h +++ b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.h @@ -263,12 +263,21 @@ public: paddr_t addr, laddr_t laddr, extent_len_t len) final; + + refresh_lba_mapping_ret refresh_lba_mapping( + Transaction &t, + LBAMapping mapping) final; + private: Cache &cache; struct { uint64_t num_alloc_extents = 0; uint64_t num_alloc_extents_iter_nexts = 0; + uint64_t num_refresh_parent_total = 0; + uint64_t num_refresh_invalid_parent = 0; + uint64_t num_refresh_unviewable_parent = 0; + uint64_t num_refresh_modified_viewable_parent = 0; } stats; struct alloc_mapping_info_t { @@ -510,6 +519,13 @@ private: Transaction &t, laddr_t addr, extent_len_t len); + + using refresh_lba_cursor_iertr = base_iertr; + using refresh_lba_cursor_ret = refresh_lba_cursor_iertr::future<>; + refresh_lba_cursor_ret refresh_lba_cursor( + op_context_t c, + LBABtree &btree, + LBACursor &cursor); }; using BtreeLBAManagerRef = std::unique_ptr; diff --git a/src/crimson/os/seastore/lba_manager/btree/lba_btree_node.cc b/src/crimson/os/seastore/lba_manager/btree/lba_btree_node.cc index 429a07afcb8c0..9cb62db9a4ef0 100644 --- a/src/crimson/os/seastore/lba_manager/btree/lba_btree_node.cc +++ b/src/crimson/os/seastore/lba_manager/btree/lba_btree_node.cc @@ -44,39 +44,6 @@ void LBALeafNode::resolve_relative_addrs(paddr_t base) } } -void LBALeafNode::maybe_fix_mapping_pos(BtreeLBAMapping &mapping) -{ - assert(mapping.get_parent() == this); - auto key = mapping.is_indirect() - ? mapping.get_intermediate_base() - : mapping.get_key(); - if (key != iter_idx(mapping.get_pos()).get_key()) { - auto iter = lower_bound(key); - { - // a mapping that no longer exist or has its value - // modified is considered an outdated one, and - // shouldn't be used anymore - ceph_assert(iter != end()); - assert(iter.get_val() == mapping.get_map_val()); - } - mapping._new_pos(iter.get_offset()); - } -} - -BtreeLBAMappingRef LBALeafNode::get_mapping( - op_context_t c, laddr_t laddr) -{ - auto iter = lower_bound(laddr); - ceph_assert(iter != end() && iter->get_key() == laddr); - auto val = iter.get_val(); - return std::make_unique( - c, - this, - iter.get_offset(), - val, - lba_node_meta_t{laddr, (laddr + val.len).checked_to_laddr(), 0}); -} - void LBALeafNode::update( internal_const_iterator_t iter, lba_map_val_t val) diff --git a/src/crimson/os/seastore/lba_manager/btree/lba_btree_node.h b/src/crimson/os/seastore/lba_manager/btree/lba_btree_node.h index 456c025cb41f2..85319466e103a 100644 --- a/src/crimson/os/seastore/lba_manager/btree/lba_btree_node.h +++ b/src/crimson/os/seastore/lba_manager/btree/lba_btree_node.h @@ -278,9 +278,6 @@ struct LBALeafNode } std::ostream &print_detail(std::ostream &out) const final; - - void maybe_fix_mapping_pos(BtreeLBAMapping &mapping); - std::unique_ptr get_mapping(op_context_t c, laddr_t laddr); }; using LBALeafNodeRef = TCachedExtentRef; diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index c506bbf1c5ff9..9d2c9dda88257 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -277,10 +277,9 @@ public: SUBDEBUGT(seastore_tm, "{} {} 0x{:x}~0x{:x} direct_off=0x{:x} ...", t, T::TYPE, pin, partial_off, partial_len, direct_partial_off); - auto fut = base_iertr::make_ready_future(); - // TODO: refresh pin - return fut.si_then([&t, this, direct_partial_off, partial_len, - maybe_init=std::move(maybe_init)](auto npin) mutable { + return lba_manager->refresh_lba_mapping(t, std::move(pin) + ).si_then([&t, this, direct_partial_off, partial_len, + maybe_init=std::move(maybe_init)](auto npin) mutable { // checking the lba child must be atomic with creating // and linking the absent child auto ret = get_extent_if_linked(t, std::move(npin)); @@ -525,12 +524,12 @@ public: t, original_laddr, original_len, original_paddr, remaps.size(), pin); // The according extent might be stable or pending. auto fut = base_iertr::now(); - if (!pin->is_indirect()) { - ceph_assert(!pin->is_clone()); - - // TODO: refresh pin - - fut = fut.si_then([this, &t, &pin] { + if (!pin.is_indirect()) { + ceph_assert(!pin.is_clone()); + fut = fut.si_then([this, &t, &pin]() mutable { + return lba_manager->refresh_lba_mapping(t, std::move(pin)); + }).si_then([this, &t, &pin](auto newpin) { + pin = std::move(newpin); if (full_extent_integrity_check) { return read_pin(t, pin.duplicate() ).si_then([](auto maybe_indirect_extent) { diff --git a/src/test/crimson/seastore/test_transaction_manager.cc b/src/test/crimson/seastore/test_transaction_manager.cc index 334bf1ac106b1..6244f6e80d9a6 100644 --- a/src/test/crimson/seastore/test_transaction_manager.cc +++ b/src/test/crimson/seastore/test_transaction_manager.cc @@ -726,6 +726,12 @@ struct transaction_manager_test_t : }).unsafe_get(); } + LBAMapping refresh_lba_mapping(test_transaction_t &t, LBAMapping mapping) { + return with_trans_intr(*t.t, [this, mapping=std::move(mapping)](auto &t) mutable { + return lba_manager->refresh_lba_mapping(t, std::move(mapping)); + }).unsafe_get(); + } + bool try_submit_transaction(test_transaction_t t) { using ertr = with_trans_ertr; using ret = ertr::future; @@ -2135,8 +2141,7 @@ TEST_P(tm_single_device_test_t, invalid_lba_mapping_detect) assert(pin.is_viewable()); std::ignore = alloc_extent(t, get_laddr_hint((LEAF_NODE_CAPACITY + 1) * 4096), 4096, 'a'); assert(!pin.is_viewable()); - // TODO: refresh pin - // pin->maybe_fix_pos(); + pin = refresh_lba_mapping(t, pin.duplicate()); auto extent2 = with_trans_intr(*(t.t), [&pin](auto& trans) { auto v = pin.get_logical_extent(trans); assert(v.has_child());