From dcf86e779f0b32bdb4bb9bb10b59db26976666ff Mon Sep 17 00:00:00 2001 From: chunmei-liu Date: Fri, 21 Jan 2022 08:22:59 -0800 Subject: [PATCH 1/2] crimson: fix split_pin_left assert_aligned hint passed to lba alloc_extent may not be block size aligned in a certain case, this hint insert lba tree as key, so make laddr in lba tree is not aligned. Signed-off-by: chunmei-liu --- .../lba_manager/btree/btree_lba_manager.cc | 1 + src/crimson/os/seastore/onode.h | 5 +++-- .../onode_manager/staged-fltree/tree_utils.h | 5 +++-- src/crimson/os/seastore/seastore.cc | 18 +++++++++++------- src/crimson/os/seastore/seastore_types.cc | 5 +++++ src/crimson/os/seastore/seastore_types.h | 2 ++ .../onode_tree/test_fltree_onode_manager.cc | 18 +++++++++++------- 7 files changed, 36 insertions(+), 18 deletions(-) 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 c9b9ee865b324..b699a5ed88fda 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 @@ -126,6 +126,7 @@ BtreeLBAManager::alloc_extent( extent_len_t len, paddr_t addr) { + ceph_assert(is_aligned(hint, (uint64_t)segment_manager.get_block_size())); struct state_t { laddr_t last_end; diff --git a/src/crimson/os/seastore/onode.h b/src/crimson/os/seastore/onode.h index c9c31c3a0b512..0d8be372776ae 100644 --- a/src/crimson/os/seastore/onode.h +++ b/src/crimson/os/seastore/onode.h @@ -66,11 +66,12 @@ class Onode : public boost::intrusive_ref_counter< virtual onode_layout_t &get_mutable_layout(Transaction &t) = 0; virtual ~Onode() = default; - laddr_t get_metadata_hint() const { + laddr_t get_metadata_hint(uint64_t block_size) const { assert(default_metadata_offset); assert(default_metadata_range); + uint64_t range_blocks = default_metadata_range / block_size; return get_hint() + default_metadata_offset + - ((uint32_t)std::rand() % default_metadata_range); + (((uint32_t)std::rand() % range_blocks) * block_size); } laddr_t get_data_hint() const { return get_hint(); diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/tree_utils.h b/src/crimson/os/seastore/onode_manager/staged-fltree/tree_utils.h index e061cf8c53dae..ad60f5c6e2d4d 100644 --- a/src/crimson/os/seastore/onode_manager/staged-fltree/tree_utils.h +++ b/src/crimson/os/seastore/onode_manager/staged-fltree/tree_utils.h @@ -196,13 +196,14 @@ class KVPool { static KVPool create_range( const std::pair& range_i, - const std::vector& value_sizes) { + const std::vector& value_sizes, + const uint64_t block_size) { kv_vector_t kvs; std::random_device rd; for (index_t i = range_i.first; i < range_i.second; ++i) { auto value_size = value_sizes[rd() % value_sizes.size()]; kvs.emplace_back( - kv_t{make_oid(i), ValueItem::create(value_size, i)} + kv_t{make_oid(i), ValueItem::create(value_size, i, block_size)} ); } return KVPool(std::move(kvs)); diff --git a/src/crimson/os/seastore/seastore.cc b/src/crimson/os/seastore/seastore.cc index 1a89fa79a44a4..2702e49730970 100644 --- a/src/crimson/os/seastore/seastore.cc +++ b/src/crimson/os/seastore/seastore.cc @@ -517,7 +517,8 @@ SeaStore::get_attr_errorator::future SeaStore::get_attr( } return _omap_get_value( t, - layout.xattr_root.get(onode.get_metadata_hint()), + layout.xattr_root.get( + onode.get_metadata_hint(segment_manager->get_block_size())), name); } ).handle_error(crimson::ct_error::input_output_error::handle([FNAME] { @@ -616,7 +617,7 @@ SeaStore::omap_get_values( op_type_t::OMAP_GET_VALUES, [this, keys](auto &t, auto &onode) { omap_root_t omap_root = onode.get_layout().omap_root.get( - onode.get_metadata_hint()); + onode.get_metadata_hint(segment_manager->get_block_size())); return _omap_get_values( t, std::move(omap_root), @@ -694,7 +695,8 @@ SeaStore::_omap_list_ret SeaStore::_omap_list( const std::optional& start, OMapManager::omap_list_config_t config) const { - auto root = omap_root.get(onode.get_metadata_hint()); + auto root = omap_root.get( + onode.get_metadata_hint(segment_manager->get_block_size())); if (root.is_null()) { return seastar::make_ready_future<_omap_list_bare_ret>( true, omap_values_t{} @@ -1109,13 +1111,13 @@ SeaStore::_omap_set_kvs( { return seastar::do_with( BtreeOMapManager(*transaction_manager), - omap_root.get(onode->get_metadata_hint()), + omap_root.get(onode->get_metadata_hint(segment_manager->get_block_size())), [&, keys=std::move(kvs)](auto &omap_manager, auto &root) { tm_iertr::future<> maybe_create_root = !root.is_null() ? tm_iertr::now() : omap_manager.initialize_omap( - t, onode->get_metadata_hint() + t, onode->get_metadata_hint(segment_manager->get_block_size()) ).si_then([&root](auto new_root) { root = new_root; }); @@ -1166,13 +1168,15 @@ SeaStore::tm_ret SeaStore::_omap_rmkeys( { LOG_PREFIX(SeaStore::_omap_rmkeys); DEBUGT("{} {} keys", *ctx.transaction, *onode, keys.size()); - auto omap_root = onode->get_layout().omap_root.get(onode->get_metadata_hint()); + auto omap_root = onode->get_layout().omap_root.get( + onode->get_metadata_hint(segment_manager->get_block_size())); if (omap_root.is_null()) { return seastar::now(); } else { return seastar::do_with( BtreeOMapManager(*transaction_manager), - onode->get_layout().omap_root.get(onode->get_metadata_hint()), + onode->get_layout().omap_root.get( + onode->get_metadata_hint(segment_manager->get_block_size())), std::move(keys), [&ctx, &onode]( auto &omap_manager, diff --git a/src/crimson/os/seastore/seastore_types.cc b/src/crimson/os/seastore/seastore_types.cc index f738870544cf6..f25ce7cce3485 100644 --- a/src/crimson/os/seastore/seastore_types.cc +++ b/src/crimson/os/seastore/seastore_types.cc @@ -14,6 +14,11 @@ seastar::logger& journal_logger() { namespace crimson::os::seastore { +bool is_aligned(uint64_t offset, uint64_t alignment) +{ + return (offset % alignment) == 0; +} + std::ostream& operator<<(std::ostream& out, const seastore_meta_t& meta) { return out << meta.seastore_id; diff --git a/src/crimson/os/seastore/seastore_types.h b/src/crimson/os/seastore/seastore_types.h index db7c907a29a98..c42345076840a 100644 --- a/src/crimson/os/seastore/seastore_types.h +++ b/src/crimson/os/seastore/seastore_types.h @@ -38,6 +38,8 @@ struct seastore_meta_t { } }; +bool is_aligned(uint64_t offset, uint64_t alignment); + std::ostream& operator<<(std::ostream& out, const seastore_meta_t& meta); // identifies a specific physical device within seastore diff --git a/src/test/crimson/seastore/onode_tree/test_fltree_onode_manager.cc b/src/test/crimson/seastore/onode_tree/test_fltree_onode_manager.cc index c91e13e511b27..047e695a4100a 100644 --- a/src/test/crimson/seastore/onode_tree/test_fltree_onode_manager.cc +++ b/src/test/crimson/seastore/onode_tree/test_fltree_onode_manager.cc @@ -26,20 +26,22 @@ namespace { struct onode_item_t { uint32_t size; uint64_t id; + uint64_t block_size; uint32_t cnt_modify = 0; void initialize(Transaction& t, Onode& value) const { auto& layout = value.get_mutable_layout(t); layout.size = size; - layout.omap_root.update(omap_root_t(id, cnt_modify, value.get_metadata_hint())); + layout.omap_root.update(omap_root_t(id, cnt_modify, + value.get_metadata_hint(block_size))); validate(value); } void validate(Onode& value) const { auto& layout = value.get_layout(); ceph_assert(laddr_t(layout.size) == laddr_t{size}); - ceph_assert(layout.omap_root.get(value.get_metadata_hint()).addr == id); - ceph_assert(layout.omap_root.get(value.get_metadata_hint()).depth == cnt_modify); + ceph_assert(layout.omap_root.get(value.get_metadata_hint(block_size)).addr == id); + ceph_assert(layout.omap_root.get(value.get_metadata_hint(block_size)).depth == cnt_modify); } void modify(Transaction& t, Onode& value) { @@ -48,9 +50,9 @@ struct onode_item_t { initialize(t, value); } - static onode_item_t create(std::size_t size, std::size_t id) { + static onode_item_t create(std::size_t size, std::size_t id, uint64_t block_size) { ceph_assert(size <= std::numeric_limits::max()); - return {(uint32_t)size, id}; + return {(uint32_t)size, id, block_size}; } }; @@ -239,7 +241,8 @@ struct fltree_onode_manager_test_t TEST_F(fltree_onode_manager_test_t, 1_single) { run_async([this] { - auto pool = KVPool::create_range({0, 1}, {128, 256}); + uint64_t block_size = tm->get_block_size(); + auto pool = KVPool::create_range({0, 1}, {128, 256}, block_size); auto iter = pool.begin(); with_onode_write(iter, [](auto& t, auto& onode, auto& item) { item.initialize(t, onode); @@ -266,8 +269,9 @@ TEST_F(fltree_onode_manager_test_t, 1_single) TEST_F(fltree_onode_manager_test_t, 2_synthetic) { run_async([this] { + uint64_t block_size = tm->get_block_size(); auto pool = KVPool::create_range( - {0, 100}, {32, 64, 128, 256, 512}); + {0, 100}, {32, 64, 128, 256, 512}, block_size); auto start = pool.begin(); auto end = pool.end(); with_onodes_write(start, end, From b398698e37079a5ce1e6ef6a5a38e11b1203e3cf Mon Sep 17 00:00:00 2001 From: chunmei-liu Date: Mon, 24 Jan 2022 17:36:23 -0800 Subject: [PATCH 2/2] crimson: eliminate warning for unused variable Signed-off-by: chunmei-liu --- src/test/crimson/seastar_runner.h | 2 +- src/test/crimson/seastore/onode_tree/test_staged_fltree.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/crimson/seastar_runner.h b/src/test/crimson/seastar_runner.h index 79f1387179067..58d3f8119e669 100644 --- a/src/test/crimson/seastar_runner.h +++ b/src/test/crimson/seastar_runner.h @@ -70,7 +70,7 @@ struct SeastarRunner { on_end.reset(new seastar::readable_eventfd); return seastar::now().then([this] { begin_signaled = true; - auto r = ::eventfd_write(begin_fd.get(), APP_RUNNING); + [[maybe_unused]] auto r = ::eventfd_write(begin_fd.get(), APP_RUNNING); assert(r == 0); return seastar::now(); }).then([this] { diff --git a/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc b/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc index 2ba456192f8cb..3d890c27683a5 100644 --- a/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc +++ b/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc @@ -1191,7 +1191,7 @@ class DummyChildPool { EXPECT_EQ(pool_clone.p_dummy->size(), 3); // erase and merge - auto pivot_key = node_to_split->get_pivot_key(); + [[maybe_unused]] auto pivot_key = node_to_split->get_pivot_key(); logger().info("\n\nERASE-MERGE {}:", node_to_split->get_name()); assert(pivot_key.compare_to(key_hobj_t(key)) == MatchKindCMP::EQ); with_trans_intr(pool_clone.get_context().t, [&] (auto &t) {