Merge pull request ceph#44718 from liu-chunmei/crimson-fix-split_pin_…

…right-assert crimson: fix split_pin_left assert_aligned Reviewed-by: Yingxin Cheng <[email protected]> Reviewed-by: Samuel Just <[email protected]>
joscollin · Jan 26, 2022 · b152c05 · b152c05
2 parents 520f29f + b398698
commit b152c05
Show file tree

Hide file tree

Showing 9 changed files with 38 additions and 20 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
@@ -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
@@ -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
@@ -196,13 +196,14 @@ class KVPool {
 
   static KVPool create_range(
       const std::pair<index_t, index_t>& range_i,
-      const std::vector<size_t>& value_sizes) {
+      const std::vector<size_t>& 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
@@ -517,7 +517,8 @@ SeaStore::get_attr_errorator::future<ceph::bufferlist> 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<std::string>& 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
@@ -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
@@ -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/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_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<uint32_t>::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<onode_item_t>::create_range({0, 1}, {128, 256});
+    uint64_t block_size = tm->get_block_size();
+    auto pool = KVPool<onode_item_t>::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<onode_item_t>::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,

diff --git 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) {
-Original file line number
+Diff line change
@@ Expand Up / @@ -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
@@ Expand Down @@