From ef39863b97b1bf3820a3f60f81a2ad6dbeda52cc Mon Sep 17 00:00:00 2001 From: Xuehan Xu Date: Sat, 11 Mar 2023 03:46:14 +0000 Subject: [PATCH] test/crimson/seastore: complement lba test with logical extents Signed-off-by: Xuehan Xu (cherry picked from commit 3c4f8c761333dcc1a4e24e73f37808720c8f684f) --- src/crimson/os/seastore/cache.cc | 2 +- src/crimson/os/seastore/cache.h | 14 ++++- src/crimson/os/seastore/cached_extent.h | 2 + .../os/seastore/extent_placement_manager.h | 12 ++++ src/crimson/os/seastore/seastore_types.h | 4 ++ src/crimson/os/seastore/transaction.h | 7 ++- .../seastore/test_btree_lba_manager.cc | 60 ++++++++++++++----- 7 files changed, 80 insertions(+), 21 deletions(-) diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc index 65f7f1d400f..d6c9fdce3aa 100644 --- a/src/crimson/os/seastore/cache.cc +++ b/src/crimson/os/seastore/cache.cc @@ -1196,7 +1196,7 @@ record_t Cache::prepare_record( fresh_stat.increment(i->get_length()); get_by_ext(efforts.fresh_inline_by_ext, i->get_type()).increment(i->get_length()); - assert(i->is_inline()); + assert(i->is_inline() || i->get_paddr().is_fake()); bufferlist bl; i->prepare_write(); diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index 2b2b66fd923..3abb7bb9360 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -804,19 +804,29 @@ public: /** * alloc_new_extent * - * Allocates a fresh extent. if delayed is true, addr will be alloc'd later + * Allocates a fresh extent. if delayed is true, addr will be alloc'd later. + * Note that epaddr can only be fed by the btree lba unittest for now */ template TCachedExtentRef alloc_new_extent( Transaction &t, ///< [in, out] current transaction extent_len_t length, ///< [in] length placement_hint_t hint, ///< [in] user hint - rewrite_gen_t gen ///< [in] rewrite generation +#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 ) { LOG_PREFIX(Cache::alloc_new_extent); SUBTRACET(seastore_cache, "allocate {} {}B, hint={}, gen={}", t, T::TYPE, length, hint, rewrite_gen_printer_t{gen}); +#ifdef UNIT_TESTS_BUILT + auto result = epm.alloc_new_extent(t, T::TYPE, length, hint, gen, epaddr); +#else auto result = epm.alloc_new_extent(t, T::TYPE, length, hint, gen); +#endif auto ret = CachedExtent::make_cached_extent_ref(std::move(result.bp)); ret->init(CachedExtent::extent_state_t::INITIAL_WRITE_PENDING, result.paddr, diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h index 3d7af9fcbdc..464f34d79fd 100644 --- a/src/crimson/os/seastore/cached_extent.h +++ b/src/crimson/os/seastore/cached_extent.h @@ -16,6 +16,8 @@ #include "crimson/common/interruptible_future.h" #include "crimson/os/seastore/seastore_types.h" +struct btree_lba_manager_test; + namespace crimson::os::seastore { class Transaction; diff --git a/src/crimson/os/seastore/extent_placement_manager.h b/src/crimson/os/seastore/extent_placement_manager.h index 9ab9ce7fe9f..b94c03ec34a 100644 --- a/src/crimson/os/seastore/extent_placement_manager.h +++ b/src/crimson/os/seastore/extent_placement_manager.h @@ -246,7 +246,12 @@ public: 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 ) { assert(hint < placement_hint_t::NUM_HINTS); assert(is_target_rewrite_generation(gen)); @@ -261,7 +266,14 @@ public: buffer::create_page_aligned(length)); bp.zero(); 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) { +#endif addr = make_record_relative_paddr(0); } else if (category == data_category_t::DATA) { assert(data_writers_by_gen[generation_to_writer(gen)]); diff --git a/src/crimson/os/seastore/seastore_types.h b/src/crimson/os/seastore/seastore_types.h index 7ec75775644..fffa24f74ab 100644 --- a/src/crimson/os/seastore/seastore_types.h +++ b/src/crimson/os/seastore/seastore_types.h @@ -625,6 +625,10 @@ public: return get_addr_type() != paddr_types_t::RESERVED; } + bool is_fake() const { + return get_device_id() == DEVICE_ID_FAKE; + } + auto operator<=>(const paddr_t &) const = default; DENC(paddr_t, v, p) { diff --git a/src/crimson/os/seastore/transaction.h b/src/crimson/os/seastore/transaction.h index ed9d1d1a0c4..d423196feba 100644 --- a/src/crimson/os/seastore/transaction.h +++ b/src/crimson/os/seastore/transaction.h @@ -169,8 +169,11 @@ public: pre_alloc_list.emplace_back(ref->cast()); fresh_block_stats.increment(ref->get_length()); } else { - assert(ref->get_paddr() == make_record_relative_paddr(0)); - ref->set_paddr(make_record_relative_paddr(offset)); + if (likely(ref->get_paddr() == make_record_relative_paddr(0))) { + ref->set_paddr(make_record_relative_paddr(offset)); + } else { + ceph_assert(ref->get_paddr().is_fake()); + } offset += ref->get_length(); inline_block_list.push_back(ref); fresh_block_stats.increment(ref->get_length()); diff --git a/src/test/crimson/seastore/test_btree_lba_manager.cc b/src/test/crimson/seastore/test_btree_lba_manager.cc index 67e18746561..0635358463a 100644 --- a/src/test/crimson/seastore/test_btree_lba_manager.cc +++ b/src/test/crimson/seastore/test_btree_lba_manager.cc @@ -252,13 +252,29 @@ struct lba_btree_test : btree_test_base { return lba_map_val_t{0, P_ADDR_NULL, len, 0}; } + device_off_t next_off = 0; + paddr_t get_paddr() { + next_off += block_size; + return make_fake_paddr(next_off); + } + void insert(laddr_t addr, extent_len_t len) { ceph_assert(check.count(addr) == 0); check.emplace(addr, get_map_val(len)); lba_btree_update([=, this](auto &btree, auto &t) { + auto extent = cache->alloc_new_extent( + t, + TestBlock::SIZE, + placement_hint_t::HOT, + 0, + get_paddr()); return btree.insert( - get_op_context(t), addr, get_map_val(len), nullptr - ).si_then([](auto){}); + get_op_context(t), addr, get_map_val(len), extent.get() + ).si_then([addr, extent](auto p){ + auto& [iter, inserted] = p; + assert(inserted); + extent->set_laddr(addr); + }); }); } @@ -405,12 +421,18 @@ struct btree_lba_manager_test : btree_test_base { auto alloc_mapping( test_transaction_t &t, laddr_t hint, - size_t len, - paddr_t paddr) { + size_t len) { auto ret = with_trans_intr( *t.t, [=, this](auto &t) { - return lba_manager->alloc_extent(t, hint, len, paddr, nullptr); + auto extent = cache->alloc_new_extent( + t, + TestBlock::SIZE, + placement_hint_t::HOT, + 0, + get_paddr()); + return lba_manager->alloc_extent( + t, hint, len, extent->get_paddr(), extent.get()); }).unsafe_get0(); logger().debug("alloc'd: {}", *ret); EXPECT_EQ(len, ret->get_length()); @@ -441,14 +463,20 @@ struct btree_lba_manager_test : btree_test_base { ceph_assert(target->second.refcount > 0); target->second.refcount--; - auto refcnt = with_trans_intr( + (void) with_trans_intr( *t.t, [=, this](auto &t) { return lba_manager->decref_extent( t, - target->first); - }).unsafe_get0().refcount; - EXPECT_EQ(refcnt, target->second.refcount); + target->first + ).si_then([this, &t, target](auto result) { + EXPECT_EQ(result.refcount, target->second.refcount); + if (result.refcount == 0) { + return cache->retire_extent_addr(t, result.addr, result.length); + } + return Cache::retire_extent_iertr::now(); + }); + }).unsafe_get0(); if (target->second.refcount == 0) { t.mappings.erase(target); } @@ -557,7 +585,7 @@ TEST_F(btree_lba_manager_test, basic) auto t = create_transaction(); check_mappings(t); // check in progress transaction sees mapping check_mappings(); // check concurrent does not - auto ret = alloc_mapping(t, laddr, block_size, get_paddr()); + auto ret = alloc_mapping(t, laddr, block_size); submit_test_transaction(std::move(t)); } check_mappings(); // check new transaction post commit sees it @@ -571,7 +599,7 @@ TEST_F(btree_lba_manager_test, force_split) auto t = create_transaction(); logger().debug("opened transaction"); for (unsigned j = 0; j < 5; ++j) { - auto ret = alloc_mapping(t, 0, block_size, get_paddr()); + auto ret = alloc_mapping(t, 0, block_size); if ((i % 10 == 0) && (j == 3)) { check_mappings(t); check_mappings(); @@ -591,7 +619,7 @@ TEST_F(btree_lba_manager_test, force_split_merge) auto t = create_transaction(); logger().debug("opened transaction"); for (unsigned j = 0; j < 5; ++j) { - auto ret = alloc_mapping(t, 0, block_size, get_paddr()); + auto ret = alloc_mapping(t, 0, block_size); // just to speed things up a bit if ((i % 100 == 0) && (j == 3)) { check_mappings(t); @@ -648,7 +676,7 @@ TEST_F(btree_lba_manager_test, single_transaction_split_merge) { auto t = create_transaction(); for (unsigned i = 0; i < 400; ++i) { - alloc_mapping(t, 0, block_size, get_paddr()); + alloc_mapping(t, 0, block_size); } check_mappings(t); submit_test_transaction(std::move(t)); @@ -671,7 +699,7 @@ TEST_F(btree_lba_manager_test, single_transaction_split_merge) { auto t = create_transaction(); for (unsigned i = 0; i < 600; ++i) { - alloc_mapping(t, 0, block_size, get_paddr()); + alloc_mapping(t, 0, block_size); } auto addresses = get_mapped_addresses(t); for (unsigned i = 0; i != addresses.size(); ++i) { @@ -699,7 +727,7 @@ TEST_F(btree_lba_manager_test, split_merge_multi) } }; iterate([&](auto &t, auto idx) { - alloc_mapping(t, idx * block_size, block_size, get_paddr()); + alloc_mapping(t, idx * block_size, block_size); }); check_mappings(); iterate([&](auto &t, auto idx) { @@ -710,7 +738,7 @@ TEST_F(btree_lba_manager_test, split_merge_multi) check_mappings(); iterate([&](auto &t, auto idx) { if ((idx % 32) > 0) { - alloc_mapping(t, idx * block_size, block_size, get_paddr()); + alloc_mapping(t, idx * block_size, block_size); } }); check_mappings(); -- 2.39.5