From 7d980e2f4dc69bbee3d8f25c3bade3fe1aebd0d3 Mon Sep 17 00:00:00 2001 From: Xuehan Xu Date: Mon, 2 Feb 2026 15:42:49 +0800 Subject: [PATCH] crimson/os/seastore/lba: set extent type for ZERO lba mappings Signed-off-by: Zhang Song Signed-off-by: Xuehan Xu --- .../seastore/backref/btree_backref_manager.cc | 16 +++++++---- .../seastore/backref/btree_backref_manager.h | 3 +- src/crimson/os/seastore/backref_manager.h | 3 +- src/crimson/os/seastore/btree/btree_types.cc | 1 + src/crimson/os/seastore/btree/btree_types.h | 6 ++-- .../os/seastore/btree/fixed_kv_btree.h | 4 +++ .../os/seastore/lba/btree_lba_manager.cc | 7 +++-- .../os/seastore/lba/btree_lba_manager.h | 28 ++++++++++--------- src/crimson/os/seastore/lba/lba_btree_node.h | 7 +++-- src/crimson/os/seastore/lba_manager.h | 6 ++-- .../os/seastore/object_data_handler.cc | 7 +++-- src/crimson/os/seastore/transaction_manager.h | 25 +++++++++++------ .../seastore/test_transaction_manager.cc | 4 +-- 13 files changed, 74 insertions(+), 43 deletions(-) diff --git a/src/crimson/os/seastore/backref/btree_backref_manager.cc b/src/crimson/os/seastore/backref/btree_backref_manager.cc index 9645fe1773ed..0e1d17e8e571 100644 --- a/src/crimson/os/seastore/backref/btree_backref_manager.cc +++ b/src/crimson/os/seastore/backref/btree_backref_manager.cc @@ -297,7 +297,8 @@ BtreeBackrefManager::merge_cached_backrefs( DEBUGT("remove mapping: {}", t, backref_entry.paddr); return remove_mapping( t, - backref_entry.paddr + backref_entry.paddr, + backref_entry.type ).si_then([](auto&&) { return seastar::now(); }).handle_error_interruptible( @@ -501,16 +502,17 @@ BtreeBackrefManager::rewrite_extent( BtreeBackrefManager::remove_mapping_ret BtreeBackrefManager::remove_mapping( Transaction &t, - paddr_t addr) + paddr_t addr, + extent_types_t type) { auto c = get_context(t); return with_btree( cache, c, - [c, addr](auto &btree) mutable { + [c, addr, type](auto &btree) mutable { return btree.lower_bound( c, addr - ).si_then([&btree, c, addr](auto iter) + ).si_then([&btree, c, addr, type](auto iter) -> remove_mapping_ret { if (iter.is_end() || iter.get_key() != addr) { LOG_PREFIX(BtreeBackrefManager::remove_mapping); @@ -520,10 +522,12 @@ BtreeBackrefManager::remove_mapping( remove_mapping_result_t>(remove_mapping_result_t()); } + auto val = iter.get_val(); + ceph_assert(type == val.type); auto ret = remove_mapping_result_t{ iter.get_key(), - iter.get_val().len, - iter.get_val().laddr}; + val.len, + val.laddr}; return btree.remove( c, iter diff --git a/src/crimson/os/seastore/backref/btree_backref_manager.h b/src/crimson/os/seastore/backref/btree_backref_manager.h index 59392f2c7461..b8b69af77d96 100644 --- a/src/crimson/os/seastore/backref/btree_backref_manager.h +++ b/src/crimson/os/seastore/backref/btree_backref_manager.h @@ -48,7 +48,8 @@ public: remove_mapping_ret remove_mapping( Transaction &t, - paddr_t offset) final; + paddr_t offset, + extent_types_t type) final; scan_mapped_space_ret scan_mapped_space( Transaction &t, diff --git a/src/crimson/os/seastore/backref_manager.h b/src/crimson/os/seastore/backref_manager.h index 9ffec10f78c8..6b6f16761896 100644 --- a/src/crimson/os/seastore/backref_manager.h +++ b/src/crimson/os/seastore/backref_manager.h @@ -123,7 +123,8 @@ public: using remove_mapping_ret = remove_mapping_iertr::future; virtual remove_mapping_ret remove_mapping( Transaction &t, - paddr_t offset) = 0; + paddr_t offset, + extent_types_t type) = 0; /** * scan all extents in both tree and cache, diff --git a/src/crimson/os/seastore/btree/btree_types.cc b/src/crimson/os/seastore/btree/btree_types.cc index 410af029b370..bc5d0cafab75 100644 --- a/src/crimson/os/seastore/btree/btree_types.cc +++ b/src/crimson/os/seastore/btree/btree_types.cc @@ -18,6 +18,7 @@ std::ostream& operator<<(std::ostream& out, const lba_map_val_t& v) << ", type=" << (extent_types_t)v.type << ", checksum=0x" << v.checksum << ", refcount=" << std::dec << v.refcount + << ", type=" << v.type << ")"; } diff --git a/src/crimson/os/seastore/btree/btree_types.h b/src/crimson/os/seastore/btree/btree_types.h index 32ee7701fece..ade1e759899b 100644 --- a/src/crimson/os/seastore/btree/btree_types.h +++ b/src/crimson/os/seastore/btree/btree_types.h @@ -138,7 +138,7 @@ struct __attribute__((packed)) lba_map_val_le_t { pladdr_le_t pladdr; extent_ref_count_le_t refcount{0}; checksum_le_t checksum{0}; - extent_types_le_t type = 0; + extent_types_le_t type{EXTENT_TYPES_MAX}; lba_map_val_le_t() = default; lba_map_val_le_t(const lba_map_val_le_t &) = default; @@ -147,7 +147,7 @@ struct __attribute__((packed)) lba_map_val_le_t { pladdr(pladdr_le_t(val.pladdr)), refcount(val.refcount), checksum(val.checksum), - type((extent_types_le_t)val.type) {} + type(static_cast(val.type)) {} operator lba_map_val_t() const { return lba_map_val_t{ @@ -155,7 +155,7 @@ struct __attribute__((packed)) lba_map_val_le_t { pladdr, refcount, checksum, - (extent_types_t)type}; + static_cast(type)}; } }; diff --git a/src/crimson/os/seastore/btree/fixed_kv_btree.h b/src/crimson/os/seastore/btree/fixed_kv_btree.h index 3aa841f3b090..8d70ad3e866f 100644 --- a/src/crimson/os/seastore/btree/fixed_kv_btree.h +++ b/src/crimson/os/seastore/btree/fixed_kv_btree.h @@ -890,6 +890,10 @@ public: c.trans, laddr, iter.is_end() ? min_max_t::max : iter.get_key()); + if constexpr (std::is_same_v) { + // avoid unexpect default extent type for lba btree + assert(val.type != extent_types_t::ROOT); + } return seastar::do_with( iter, [this, c, laddr, val, child](auto &ret) { diff --git a/src/crimson/os/seastore/lba/btree_lba_manager.cc b/src/crimson/os/seastore/lba/btree_lba_manager.cc index 5226c7b95542..d2c35cfeb401 100644 --- a/src/crimson/os/seastore/lba/btree_lba_manager.cc +++ b/src/crimson/os/seastore/lba/btree_lba_manager.cc @@ -242,7 +242,8 @@ BtreeLBAManager::reserve_region( Transaction &t, LBACursorRef cursor, laddr_t addr, - extent_len_t len) + extent_len_t len, + extent_types_t type) { LOG_PREFIX(BtreeLBAManager::reserve_region); DEBUGT("{} {}~{}", t, *cursor, addr, len); @@ -255,7 +256,7 @@ BtreeLBAManager::reserve_region( pladdr_t{P_ADDR_ZERO}, EXTENT_DEFAULT_REF_COUNT, 0, - extent_types_t::NONE}; + type}; auto p = co_await btree.insert( c, iter, addr, val, get_reserved_ptr() @@ -1054,6 +1055,7 @@ BtreeLBAManager::remap_mappings( assert(orig_indirect || (orig_val.pladdr.is_paddr() && orig_val.pladdr.get_paddr().is_absolute())); + auto type = cursor->get_extent_type(); cursor = co_await update_mapping_refcount( c.trans, cursor, -1); iter = btree.make_partial_iter(c, *cursor); @@ -1075,6 +1077,7 @@ BtreeLBAManager::remap_mappings( val.refcount = EXTENT_DEFAULT_REF_COUNT; // Checksum will be updated when the committing the transaction val.checksum = CRC_NULL; + val.type = type; // committing the transaction auto p = co_await btree.insert( c, iter, new_key, std::move(val), diff --git a/src/crimson/os/seastore/lba/btree_lba_manager.h b/src/crimson/os/seastore/lba/btree_lba_manager.h index 4dc9fd8378cb..04628f7ee26f 100644 --- a/src/crimson/os/seastore/lba/btree_lba_manager.h +++ b/src/crimson/os/seastore/lba/btree_lba_manager.h @@ -79,15 +79,17 @@ public: Transaction &t, LBACursorRef pos, laddr_t laddr, - extent_len_t len) final; + extent_len_t len, + extent_types_t type) final; alloc_extent_ret reserve_region( Transaction &t, laddr_hint_t hint, - extent_len_t len) final + extent_len_t len, + extent_types_t type) final { std::vector alloc_infos = { - alloc_mapping_info_t::create_zero(len)}; + alloc_mapping_info_t::create_zero(len, type)}; auto cursors = co_await alloc_contiguous_mappings( t, hint, alloc_infos); assert(cursors.size() == 1); @@ -271,7 +273,9 @@ private: return value.pladdr.is_laddr(); } - static alloc_mapping_info_t create_zero(extent_len_t len) { + static alloc_mapping_info_t create_zero( + extent_len_t len, + extent_types_t type) { return { L_ADDR_NULL, { @@ -279,7 +283,7 @@ private: pladdr_t(P_ADDR_ZERO), EXTENT_DEFAULT_REF_COUNT, 0, - extent_types_t::NONE + type }}; } static alloc_mapping_info_t create_indirect( @@ -294,7 +298,8 @@ private: EXTENT_DEFAULT_REF_COUNT, 0, // crc will only be used and checked with LBA direct mappings // also see pin_to_extent(_by_type) - extent_types_t::NONE + // only OBJECT_DATA_BLOCK support indirect mapping for now + extent_types_t::OBJECT_DATA_BLOCK }}; } static alloc_mapping_info_t create_direct( @@ -305,13 +310,10 @@ private: checksum_t checksum, LogicalChildNode& extent) { return { - laddr, - {len, - pladdr_t(paddr), - refcount, - checksum, - extent.get_type()}, - &extent}; + laddr, + {len, pladdr_t(paddr), refcount, checksum, extent.get_type()}, + &extent + }; } }; diff --git a/src/crimson/os/seastore/lba/lba_btree_node.h b/src/crimson/os/seastore/lba/lba_btree_node.h index e242061d3f45..9aaf30bda84f 100644 --- a/src/crimson/os/seastore/lba/lba_btree_node.h +++ b/src/crimson/os/seastore/lba/lba_btree_node.h @@ -90,14 +90,14 @@ using LBAInternalNodeRef = LBAInternalNode::Ref; * checksum : ceph_le32[1] 4B * size : ceph_le32[1] 4B * meta : lba_node_meta_le_t[1] 36B - * keys : laddr_le_t[CAPACITY] (109*16)B - * values : lba_map_val_le_t[CAPACITY] (109*21)B + * keys : laddr_le_t[CAPACITY] (106*16)B + * values : lba_map_val_le_t[CAPACITY] (106*21)B * = 4077B * * TODO: update FixedKVNodeLayout to handle the above calculation * TODO: the above alignment probably isn't portable without further work */ -constexpr size_t LEAF_NODE_CAPACITY = 109; +constexpr size_t LEAF_NODE_CAPACITY = 106; struct LBALeafNode : FixedKVLeafNode< @@ -445,6 +445,7 @@ struct LBACursor : BtreeCursor { extent_types_t get_extent_type() const { assert(is_viewable()); assert(!is_end()); + assert(iter.get_val().type != extent_types_t::NONE); return iter.get_val().type; } diff --git a/src/crimson/os/seastore/lba_manager.h b/src/crimson/os/seastore/lba_manager.h index 1b066b575ed5..a0e4dcec518a 100644 --- a/src/crimson/os/seastore/lba_manager.h +++ b/src/crimson/os/seastore/lba_manager.h @@ -117,7 +117,8 @@ public: virtual alloc_extent_ret reserve_region( Transaction &t, laddr_hint_t hint, - extent_len_t len) = 0; + extent_len_t len, + extent_types_t type) = 0; /* * Inserts a zero mapping at the position "pos" with @@ -127,7 +128,8 @@ public: Transaction &t, LBACursorRef cursor, laddr_t hint, - extent_len_t len) = 0; + extent_len_t len, + extent_types_t type) = 0; using ref_iertr = base_iertr::extend< crimson::ct_error::enoent>; diff --git a/src/crimson/os/seastore/object_data_handler.cc b/src/crimson/os/seastore/object_data_handler.cc index 7cca29f93be1..1167c8f6a1d9 100644 --- a/src/crimson/os/seastore/object_data_handler.cc +++ b/src/crimson/os/seastore/object_data_handler.cc @@ -124,7 +124,8 @@ ObjectDataHandler::prepare_data_reservation( return ctx.tm.reserve_region( ctx.t, ctx.onode.get_data_hint(), - max_object_size + max_object_size, + extent_types_t::OBJECT_DATA_BLOCK ).si_then([max_object_size=max_object_size, &object_data](auto pin) { ceph_assert(pin.get_length() == max_object_size); object_data.update_reserved( @@ -303,7 +304,9 @@ ObjectDataHandler::write_ret do_zero( ).checked_to_laddr(); auto len = end.get_byte_distance(laddr); if (len != 0) { - zero_pos = co_await ctx.tm.reserve_region(ctx.t, std::move(zero_pos), laddr, len + zero_pos = co_await ctx.tm.reserve_region( + ctx.t, std::move(zero_pos), laddr, len, + extent_types_t::OBJECT_DATA_BLOCK ).handle_error_interruptible( crimson::ct_error::enospc::assert_failure{"unexpected enospc"}, TransactionManager::get_pin_iertr::pass_further{} diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index 5a834ea4b0dc..945f9063e1fa 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -641,13 +641,15 @@ public: reserve_extent_ret reserve_region( Transaction &t, laddr_hint_t hint, - extent_len_t len) { + extent_len_t len, + extent_types_t type) { LOG_PREFIX(TransactionManager::reserve_region); - SUBDEBUGT(seastore_tm, "hint {}~0x{:x} ...", t, hint, len); + SUBDEBUGT(seastore_tm, "hint {}~0x{:x} {} ...", t, hint, len, type); auto pin = co_await lba_manager->reserve_region( t, hint, - len + len, + type ); SUBDEBUGT(seastore_tm, "reserved {}", t, *pin); co_return LBAMapping::create_direct(std::move(pin)); @@ -657,15 +659,17 @@ public: Transaction &t, LBAMapping pos, laddr_t hint, - extent_len_t len) { + extent_len_t len, + extent_types_t type) { LOG_PREFIX(TransactionManager::reserve_region); - SUBDEBUGT(seastore_tm, "hint {}~0x{:x} ...", t, hint, len); + SUBDEBUGT(seastore_tm, "hint {}~0x{:x} {} ...", t, hint, len, type); pos = co_await pos.refresh(); auto pin = co_await lba_manager->reserve_region( t, pos.get_effective_cursor_ref(), hint, - len + len, + type ); co_return LBAMapping::create_direct(std::move(pin)); } @@ -760,7 +764,8 @@ public: t, std::move(pos), (dst_base + cloned_to).checked_to_laddr(), - clone_len + clone_len, + mapping.get_extent_type() ).handle_error_interruptible( clone_iertr::pass_further{}, crimson::ct_error::assert_all{"unexpected error"} @@ -1060,6 +1065,9 @@ public: if (!mapping.is_indirect() && mapping.is_zero_reserved()) { SUBDEBUGT(seastore_tm, "zero reserved, mapping {}, {} remaps", t, mapping, remaps); + //TODO: drop this assert + assert(mapping.get_extent_type() == extent_types_t::OBJECT_DATA_BLOCK); + auto type = mapping.get_extent_type(); std::vector ret; auto orig_laddr = mapping.get_key(); auto pos = co_await remove( @@ -1074,7 +1082,8 @@ public: t, std::move(pos), laddr, - remap.len + remap.len, + type ).handle_error_interruptible( remap_mappings_iertr::pass_further{}, crimson::ct_error::assert_all{"unexpected error"} diff --git a/src/test/crimson/seastore/test_transaction_manager.cc b/src/test/crimson/seastore/test_transaction_manager.cc index 040beb967bf5..8461d151f891 100644 --- a/src/test/crimson/seastore/test_transaction_manager.cc +++ b/src/test/crimson/seastore/test_transaction_manager.cc @@ -1892,10 +1892,10 @@ TEST_P(tm_random_block_device_test_t, scatter_allocation) laddr_t ADDR = get_laddr_hint(0xFF * 4096); epm->prefill_fragmented_devices(); auto t = create_transaction(); - for (int i = 0; i < 1975; i++) { + for (int i = 0; i < 1974; i++) { auto extents = alloc_extents(t, (ADDR + i * 16384).checked_to_laddr(), 16384, 'a'); } - alloc_extents_deemed_fail(t, (ADDR + 1975 * 16384).checked_to_laddr(), 16384, 'a'); + alloc_extents_deemed_fail(t, (ADDR + 1974 * 16384).checked_to_laddr(), 16384, 'a'); check_mappings(t); check(); submit_transaction(std::move(t)); -- 2.47.3