From: Zhang Song Date: Wed, 30 Jul 2025 06:38:51 +0000 (+0800) Subject: crimson/os/seastore: consolidate parameters on extent allocation path X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=b847b58da2d2f4864d64efca38976508877fc0d1;p=ceph-ci.git crimson/os/seastore: consolidate parameters on extent allocation path Signed-off-by: Zhang Song Signed-off-by: Xuehan Xu --- diff --git a/src/crimson/os/seastore/btree/fixed_kv_btree.h b/src/crimson/os/seastore/btree/fixed_kv_btree.h index fdfa31230ab..0cc4b1e2a70 100644 --- a/src/crimson/os/seastore/btree/fixed_kv_btree.h +++ b/src/crimson/os/seastore/btree/fixed_kv_btree.h @@ -491,10 +491,7 @@ public: static mkfs_ret mkfs(RootBlockRef &root_block, op_context_t c) { assert(root_block->is_mutation_pending()); auto root_leaf = c.cache.template alloc_new_non_data_extent( - c.trans, - node_size, - placement_hint_t::HOT, - INIT_GENERATION); + c.trans, node_size, {placement_hint_t::HOT, INIT_GENERATION}); root_leaf->set_size(0); fixed_kv_node_meta_t meta{min_max_t::min, min_max_t::max, 1}; root_leaf->set_meta(meta); @@ -1460,14 +1457,17 @@ public: assert(is_lba_backref_node(e->get_type())); auto do_rewrite = [&](auto &fixed_kv_extent) { + auto opt = Cache::alloc_option_t { + fixed_kv_extent.get_user_hint(), + // get target rewrite generation + fixed_kv_extent.get_rewrite_generation() + }; auto n_fixed_kv_extent = c.cache.template alloc_new_non_data_extent< std::remove_reference_t >( c.trans, fixed_kv_extent.get_length(), - fixed_kv_extent.get_user_hint(), - // get target rewrite generation - fixed_kv_extent.get_rewrite_generation()); + opt); n_fixed_kv_extent->rewrite(c.trans, fixed_kv_extent, 0); SUBTRACET( @@ -2246,7 +2246,7 @@ private: if (split_from == iter.get_depth()) { assert(iter.is_full()); auto nroot = c.cache.template alloc_new_non_data_extent( - c.trans, node_size, placement_hint_t::HOT, INIT_GENERATION); + c.trans, node_size, {placement_hint_t::HOT, INIT_GENERATION}); fixed_kv_node_meta_t meta{ min_max_t::min, min_max_t::max, iter.get_depth() + 1}; nroot->set_meta(meta); diff --git a/src/crimson/os/seastore/btree/fixed_kv_node.h b/src/crimson/os/seastore/btree/fixed_kv_node.h index 24475157938..69c40c1acad 100644 --- a/src/crimson/os/seastore/btree/fixed_kv_node.h +++ b/src/crimson/os/seastore/btree/fixed_kv_node.h @@ -329,9 +329,9 @@ struct FixedKVInternalNode std::tuple make_split_children(op_context_t c) { auto left = c.cache.template alloc_new_non_data_extent( - c.trans, node_size, placement_hint_t::HOT, INIT_GENERATION); + c.trans, node_size, {placement_hint_t::HOT, INIT_GENERATION}); auto right = c.cache.template alloc_new_non_data_extent( - c.trans, node_size, placement_hint_t::HOT, INIT_GENERATION); + c.trans, node_size, {placement_hint_t::HOT, INIT_GENERATION}); this->split_child_ptrs(c.trans, *left, *right); auto pivot = this->split_into(*left, *right); left->range = left->get_meta(); @@ -347,7 +347,7 @@ struct FixedKVInternalNode op_context_t c, Ref &right) { auto replacement = c.cache.template alloc_new_non_data_extent( - c.trans, node_size, placement_hint_t::HOT, INIT_GENERATION); + c.trans, node_size, {placement_hint_t::HOT, INIT_GENERATION}); replacement->merge_child_ptrs( c.trans, static_cast(*this), *right); replacement->merge_from(*this, *right->template cast()); @@ -365,9 +365,9 @@ struct FixedKVInternalNode ceph_assert(_right->get_type() == this->get_type()); auto &right = *_right->template cast(); auto replacement_left = c.cache.template alloc_new_non_data_extent( - c.trans, node_size, placement_hint_t::HOT, INIT_GENERATION); + c.trans, node_size, {placement_hint_t::HOT, INIT_GENERATION}); auto replacement_right = c.cache.template alloc_new_non_data_extent( - c.trans, node_size, placement_hint_t::HOT, INIT_GENERATION); + c.trans, node_size, {placement_hint_t::HOT, INIT_GENERATION}); // We should do full merge if pivot_idx == right.get_size(). ceph_assert(pivot_idx != right.get_size()); @@ -748,9 +748,9 @@ struct FixedKVLeafNode std::tuple make_split_children(op_context_t c) { auto left = c.cache.template alloc_new_non_data_extent( - c.trans, node_size, placement_hint_t::HOT, INIT_GENERATION); + c.trans, node_size, {placement_hint_t::HOT, INIT_GENERATION}); auto right = c.cache.template alloc_new_non_data_extent( - c.trans, node_size, placement_hint_t::HOT, INIT_GENERATION); + c.trans, node_size, {placement_hint_t::HOT, INIT_GENERATION}); this->on_split(c.trans, *left, *right); auto pivot = this->split_into(*left, *right); left->range = left->get_meta(); @@ -775,7 +775,7 @@ struct FixedKVLeafNode op_context_t c, Ref &right) { auto replacement = c.cache.template alloc_new_non_data_extent( - c.trans, node_size, placement_hint_t::HOT, INIT_GENERATION); + c.trans, node_size, {placement_hint_t::HOT, INIT_GENERATION}); replacement->on_merge(c.trans, static_cast(*this), *right); replacement->merge_from(*this, *right->template cast()); replacement->range = replacement->get_meta(); @@ -807,9 +807,9 @@ struct FixedKVLeafNode ceph_assert(_right->get_type() == this->get_type()); auto &right = *_right->template cast(); auto replacement_left = c.cache.template alloc_new_non_data_extent( - c.trans, node_size, placement_hint_t::HOT, INIT_GENERATION); + c.trans, node_size, {placement_hint_t::HOT, INIT_GENERATION}); auto replacement_right = c.cache.template alloc_new_non_data_extent( - c.trans, node_size, placement_hint_t::HOT, INIT_GENERATION); + c.trans, node_size, {placement_hint_t::HOT, INIT_GENERATION}); this->on_balance( c.trans, diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc index 11578c35f8e..7a3d9cf6481 100644 --- a/src/crimson/os/seastore/cache.cc +++ b/src/crimson/os/seastore/cache.cc @@ -1219,35 +1219,36 @@ CachedExtentRef Cache::alloc_new_non_data_extent_by_type( SUBDEBUGT(seastore_cache, "allocate {} 0x{:x}B, hint={}, gen={}", t, type, length, hint, rewrite_gen_printer_t{gen}); ceph_assert(get_extent_category(type) == data_category_t::METADATA); + auto opt = alloc_option_t{hint, gen}; switch (type) { case extent_types_t::ROOT: ceph_assert(0 == "ROOT is never directly alloc'd"); return CachedExtentRef(); case extent_types_t::LADDR_INTERNAL: - return alloc_new_non_data_extent(t, length, hint, gen); + return alloc_new_non_data_extent(t, length, opt); case extent_types_t::LADDR_LEAF: return alloc_new_non_data_extent( - t, length, hint, gen); + t, length, opt); case extent_types_t::ROOT_META: return alloc_new_non_data_extent( - t, length, hint, gen); + t, length, opt); case extent_types_t::ONODE_BLOCK_STAGED: return alloc_new_non_data_extent( - t, length, hint, gen); + t, length, opt); case extent_types_t::OMAP_INNER: return alloc_new_non_data_extent( - t, length, hint, gen); + t, length, opt); case extent_types_t::OMAP_LEAF: return alloc_new_non_data_extent( - t, length, hint, gen); + t, length, opt); case extent_types_t::COLL_BLOCK: return alloc_new_non_data_extent( - t, length, hint, gen); + t, length, opt); case extent_types_t::TEST_BLOCK_PHYSICAL: - return alloc_new_non_data_extent(t, length, hint, gen); + return alloc_new_non_data_extent(t, length, opt); case extent_types_t::LOG_NODE: return alloc_new_non_data_extent( - t, length, hint, gen); + t, length, opt); case extent_types_t::NONE: { ceph_assert(0 == "NONE is an invalid extent type"); return CachedExtentRef(); @@ -1275,14 +1276,14 @@ std::vector Cache::alloc_new_data_extents_by_type( case extent_types_t::OBJECT_DATA_BLOCK: { auto extents = alloc_new_data_extents< - ObjectDataBlock>(t, length, hint, gen); + ObjectDataBlock>(t, length, {hint, gen}); res.insert(res.begin(), extents.begin(), extents.end()); } return res; case extent_types_t::TEST_BLOCK: { auto extents = alloc_new_data_extents< - TestBlock>(t, length, hint, gen); + TestBlock>(t, length, {hint, gen}); res.insert(res.begin(), extents.begin(), extents.end()); } return res; diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index 72c5d174db6..290c7dcf4ea 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -1142,6 +1142,7 @@ public: } } + using alloc_option_t = ExtentPlacementManager::alloc_option_t; /** * alloc_new_non_data_extent * @@ -1152,22 +1153,12 @@ public: TCachedExtentRef alloc_new_non_data_extent( Transaction &t, ///< [in, out] current transaction extent_len_t length, ///< [in] length - placement_hint_t hint, ///< [in] user hint -#ifdef UNIT_TESTS_BUILT - rewrite_gen_t gen, ///< [in] rewrite generation - std::optional epaddr = std::nullopt ///< [in] paddr fed by callers -#else - rewrite_gen_t gen -#endif + alloc_option_t opt ) { LOG_PREFIX(Cache::alloc_new_non_data_extent); - SUBTRACET(seastore_cache, "allocate {} 0x{:x}B, hint={}, gen={}", - t, T::TYPE, length, hint, rewrite_gen_printer_t{gen}); -#ifdef UNIT_TESTS_BUILT - auto result = epm.alloc_new_non_data_extent(t, T::TYPE, length, hint, gen, epaddr); -#else - auto result = epm.alloc_new_non_data_extent(t, T::TYPE, length, hint, gen); -#endif + SUBTRACET(seastore_cache, "allocate {} 0x{:x}B, opt.hint={}, gen={}", + t, T::TYPE, length, opt.hint, rewrite_gen_printer_t{opt.gen}); + auto result = epm.alloc_new_non_data_extent(t, T::TYPE, length, opt); if (!result) { SUBERRORT(seastore_cache, "insufficient space", t); std::rethrow_exception(crimson::ct_error::enospc::exception_ptr()); @@ -1178,14 +1169,14 @@ public: epm.dynamic_max_rewrite_generation)); ret->init(CachedExtent::extent_state_t::INITIAL_WRITE_PENDING, result->paddr, - hint, + opt.hint, result->gen, t.get_trans_id()); t.add_fresh_extent(ret); SUBDEBUGT(seastore_cache, "allocated {} 0x{:x}B extent at {}, hint={}, gen={} -- {}", t, T::TYPE, length, result->paddr, - hint, rewrite_gen_printer_t{result->gen}, *ret); + opt.hint, rewrite_gen_printer_t{result->gen}, *ret); return ret; } /** @@ -1198,22 +1189,12 @@ public: std::vector> alloc_new_data_extents( Transaction &t, ///< [in, out] current transaction extent_len_t length, ///< [in] length - placement_hint_t hint, ///< [in] user hint -#ifdef UNIT_TESTS_BUILT - rewrite_gen_t gen, ///< [in] rewrite generation - std::optional epaddr = std::nullopt ///< [in] paddr fed by callers -#else - rewrite_gen_t gen -#endif + alloc_option_t opt ) { LOG_PREFIX(Cache::alloc_new_data_extents); SUBTRACET(seastore_cache, "allocate {} 0x{:x}B, hint={}, gen={}", - t, T::TYPE, length, hint, rewrite_gen_printer_t{gen}); -#ifdef UNIT_TESTS_BUILT - auto results = epm.alloc_new_data_extents(t, T::TYPE, length, hint, gen, epaddr); -#else - auto results = epm.alloc_new_data_extents(t, T::TYPE, length, hint, gen); -#endif + t, T::TYPE, length, opt.hint, rewrite_gen_printer_t{opt.gen}); + auto results = epm.alloc_new_data_extents(t, T::TYPE, length, opt); if (results.empty()) { SUBERRORT(seastore_cache, "insufficient space", t); std::rethrow_exception(crimson::ct_error::enospc::exception_ptr()); @@ -1226,14 +1207,14 @@ public: epm.dynamic_max_rewrite_generation)); ret->init(CachedExtent::extent_state_t::INITIAL_WRITE_PENDING, result.paddr, - hint, + opt.hint, result.gen, t.get_trans_id()); t.add_fresh_extent(ret); SUBDEBUGT(seastore_cache, "allocated {} 0x{:x}B extent at {}, hint={}, gen={} -- {}", t, T::TYPE, length, result.paddr, - hint, rewrite_gen_printer_t{result.gen}, *ret); + opt.hint, rewrite_gen_printer_t{result.gen}, *ret); extents.emplace_back(std::move(ret)); } return extents; diff --git a/src/crimson/os/seastore/extent_placement_manager.h b/src/crimson/os/seastore/extent_placement_manager.h index 2a48510a17f..8685a3d04a2 100644 --- a/src/crimson/os/seastore/extent_placement_manager.h +++ b/src/crimson/os/seastore/extent_placement_manager.h @@ -351,6 +351,13 @@ public: return background_process.start_background(); } + struct alloc_option_t { + placement_hint_t hint; + rewrite_gen_t gen; +#ifdef UNIT_TESTS_BUILT + std::optional external_paddr = std::nullopt; +#endif + }; struct alloc_result_t { paddr_t paddr; bufferptr bp; @@ -360,34 +367,27 @@ public: Transaction& t, extent_types_t type, extent_len_t length, - placement_hint_t hint, -#ifdef UNIT_TESTS_BUILT - rewrite_gen_t gen, - std::optional external_paddr = std::nullopt -#else - rewrite_gen_t gen -#endif + alloc_option_t opt ) { - assert(hint < placement_hint_t::NUM_HINTS); - assert(is_target_rewrite_generation(gen, dynamic_max_rewrite_generation)); - assert(gen == INIT_GENERATION || hint == placement_hint_t::REWRITE); + assert(opt.hint < placement_hint_t::NUM_HINTS); + assert(is_target_rewrite_generation(opt.gen, dynamic_max_rewrite_generation)); + assert(opt.gen == INIT_GENERATION || opt.hint == placement_hint_t::REWRITE); data_category_t category = get_extent_category(type); - gen = adjust_generation(category, type, hint, gen); + opt.gen = adjust_generation(category, type, opt.hint, opt.gen); paddr_t addr; #ifdef UNIT_TESTS_BUILT - if (unlikely(external_paddr.has_value())) { - assert(external_paddr->is_fake()); - addr = *external_paddr; - } else if (gen == INLINE_GENERATION) { -#else - if (gen == INLINE_GENERATION) { + if (unlikely(opt.external_paddr.has_value())) { + assert(opt.external_paddr->is_fake()); + addr = *opt.external_paddr; + } else #endif + if (opt.gen == INLINE_GENERATION) { addr = make_record_relative_paddr(0); } else { assert(category == data_category_t::METADATA); - addr = get_writer(hint, category, gen)->alloc_paddr(length); + addr = get_writer(opt.hint, category, opt.gen)->alloc_paddr(length); } assert(!(category == data_category_t::DATA)); @@ -399,44 +399,37 @@ public: // according to the allocator. auto bp = create_extent_ptr_zero(length); - return alloc_result_t{addr, std::move(bp), gen}; + return alloc_result_t{addr, std::move(bp), opt.gen}; } std::list alloc_new_data_extents( Transaction& t, extent_types_t type, extent_len_t length, - placement_hint_t hint, -#ifdef UNIT_TESTS_BUILT - rewrite_gen_t gen, - std::optional external_paddr = std::nullopt -#else - rewrite_gen_t gen -#endif + alloc_option_t opt ) { LOG_PREFIX(ExtentPlacementManager::alloc_new_data_extents); - assert(hint < placement_hint_t::NUM_HINTS); - assert(is_target_rewrite_generation(gen, dynamic_max_rewrite_generation)); - assert(gen == INIT_GENERATION || hint == placement_hint_t::REWRITE); + assert(opt.hint < placement_hint_t::NUM_HINTS); + assert(is_target_rewrite_generation(opt.gen, dynamic_max_rewrite_generation)); + assert(opt.gen == INIT_GENERATION || opt.hint == placement_hint_t::REWRITE); data_category_t category = get_extent_category(type); - gen = adjust_generation(category, type, hint, gen); - assert(gen != INLINE_GENERATION); + opt.gen = adjust_generation(category, type, opt.hint, opt.gen); + assert(opt.gen != INLINE_GENERATION); // XXX: bp might be extended to point to different memory (e.g. PMem) // according to the allocator. std::list allocs; #ifdef UNIT_TESTS_BUILT - if (unlikely(external_paddr.has_value())) { - assert(external_paddr->is_fake()); + if (unlikely(opt.external_paddr.has_value())) { + assert(opt.external_paddr->is_fake()); auto bp = create_extent_ptr_zero(length); - allocs.emplace_back(alloc_result_t{*external_paddr, std::move(bp), gen}); - } else { -#else - { + allocs.emplace_back(alloc_result_t{*opt.external_paddr, std::move(bp), opt.gen}); + } else #endif + { assert(category == data_category_t::DATA); - auto addrs = get_writer(hint, category, gen)->alloc_paddrs(length); + auto addrs = get_writer(opt.hint, category, opt.gen)->alloc_paddrs(length); for (auto &ext : addrs) { auto left = ext.len; while (left > 0) { @@ -448,10 +441,10 @@ public: auto start = ext.start.is_delayed() ? ext.start : ext.start + (ext.len - left); - allocs.emplace_back(alloc_result_t{start, std::move(bp), gen}); + allocs.emplace_back(alloc_result_t{start, std::move(bp), opt.gen}); SUBDEBUGT(seastore_epm, - "allocated {} 0x{:x}B extent at {}, hint={}, gen={}", - t, type, len, start, hint, gen); + "allocated {} 0x{:x}B extent at {}, opt.hint={}, opt.gen={}", + t, type, len, start, opt.hint, opt.gen); left -= len; } } diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index c21bebd23d1..1f7774fedcd 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -552,10 +552,7 @@ public: SUBDEBUGT(seastore_tm, "{} hint {}~0x{:x} phint={} ...", t, T::TYPE, laddr_hint, len, placement_hint); auto ext = cache->alloc_new_non_data_extent( - t, - len, - placement_hint, - INIT_GENERATION); + t, len, {placement_hint, INIT_GENERATION}); // user must initialize the logical extent themselves. assert(is_user_transaction(t.get_src())); ext->set_seen_by_users(); @@ -593,10 +590,7 @@ public: SUBDEBUGT(seastore_tm, "{} hint {}~0x{:x} phint={} ...", t, T::TYPE, laddr_hint, len, placement_hint); auto exts = cache->alloc_new_data_extents( - t, - len, - placement_hint, - INIT_GENERATION); + t, len, {placement_hint, INIT_GENERATION}); // user must initialize the logical extent themselves assert(is_user_transaction(t.get_src())); for (auto& ext : exts) { diff --git a/src/test/crimson/seastore/test_btree_lba_manager.cc b/src/test/crimson/seastore/test_btree_lba_manager.cc index 4fb774930e5..c79ce6e5e25 100644 --- a/src/test/crimson/seastore/test_btree_lba_manager.cc +++ b/src/test/crimson/seastore/test_btree_lba_manager.cc @@ -327,11 +327,7 @@ struct lba_btree_test : btree_test_base { check.emplace(addr, get_map_val(len, TestBlock::TYPE)); lba_btree_update([=, this](auto &btree, auto &t) { auto extents = cache->alloc_new_data_extents( - t, - TestBlock::SIZE, - placement_hint_t::HOT, - 0, - get_paddr()); + t, TestBlock::SIZE, {placement_hint_t::HOT, 0, false, get_paddr()}); return seastar::do_with( std::move(extents), [this, addr, &t, len, &btree](auto &extents) { @@ -454,8 +450,7 @@ struct btree_lba_manager_test : btree_test_base { cache->alloc_new_non_data_extent( *t.t, TestBlockPhysical::SIZE, - placement_hint_t::HOT, - 0); + {placement_hint_t::HOT, 0}); }; return t; } @@ -550,11 +545,7 @@ struct btree_lba_manager_test : btree_test_base { *t.t, [=, this](auto &t) { auto extents = cache->alloc_new_data_extents( - t, - TestBlock::SIZE, - placement_hint_t::HOT, - 0, - get_paddr()); + t, TestBlock::SIZE, {placement_hint_t::HOT, 0, false, get_paddr()}); return seastar::do_with( std::vector( extents.begin(), extents.end()), diff --git a/src/test/crimson/seastore/test_seastore_cache.cc b/src/test/crimson/seastore/test_seastore_cache.cc index 20b99e50ee5..4de65875e05 100644 --- a/src/test/crimson/seastore/test_seastore_cache.cc +++ b/src/test/crimson/seastore/test_seastore_cache.cc @@ -157,8 +157,7 @@ TEST_F(cache_test_t, test_addr_fixup) auto extent = cache->alloc_new_non_data_extent( *t, TestBlockPhysical::SIZE, - placement_hint_t::HOT, - 0); + {placement_hint_t::HOT, 0}); extent->set_contents('c'); csum = extent->calc_crc32c(); submit_transaction(std::move(t)).get(); @@ -188,8 +187,7 @@ TEST_F(cache_test_t, test_dirty_extent) auto extent = cache->alloc_new_non_data_extent( *t, TestBlockPhysical::SIZE, - placement_hint_t::HOT, - 0); + {placement_hint_t::HOT, 0}); extent->set_contents('c'); csum = extent->calc_crc32c(); auto reladdr = extent->get_paddr();