Merge pull request ceph#63432 from Matan-B/wip-matanb-crimson-tentacle-62071

crimson/os/seastore: tentacle: introduce BtreeCursor to simplify the implementation of LBAMapping/BackrefMapping

Reviewed-by: Yingxin Cheng <[email protected]>

Author: Matan-B

src/crimson/os/seastore/CMakeLists.txt

@@ -12,11 +12,11 @@ set(crimson_seastore_srcs
   root_block.cc
   lba_manager.cc
   async_cleaner.cc
+  btree/btree_types.cc
   backref_manager.cc
-  backref/backref_tree_node.cc
   backref/btree_backref_manager.cc
-  lba_manager/btree/btree_lba_manager.cc
-  lba_manager/btree/lba_btree_node.cc
+  lba/btree_lba_manager.cc
+  lba/lba_btree_node.cc
   omap_manager.cc
   omap_manager/btree/btree_omap_manager.cc
   omap_manager/btree/omap_btree_node_impl.cc

src/crimson/os/seastore/async_cleaner.cc

@@ -1091,7 +1091,7 @@ double SegmentCleaner::calc_gc_benefit_cost(
 SegmentCleaner::do_reclaim_space_ret
 SegmentCleaner::do_reclaim_space(
     const std::vector<CachedExtentRef> &backref_extents,
-    const backref_pin_list_t &pin_list,
+    const backref_mapping_list_t &pin_list,
     std::size_t &reclaimed,
     std::size_t &runs)
 {
@@ -1142,10 +1142,10 @@ SegmentCleaner::do_reclaim_space(
         backref_entry_query_set_t backref_entries;
         for (auto &pin : pin_list) {
           backref_entries.emplace(
-            pin->get_key(),
-            pin->get_val(),
-            pin->get_length(),
-            pin->get_type());
+            pin.get_key(),
+            pin.get_val(),
+            pin.get_length(),
+            pin.get_type());
         }
         for (auto &cached_backref : cached_backref_entries) {
           if (cached_backref.laddr == L_ADDR_NULL) {
@@ -1236,7 +1236,7 @@ SegmentCleaner::clean_space_ret SegmentCleaner::clean_space()
   // transactions.  So, concurrent transactions between trim and reclaim are
   // not allowed right now.
   return seastar::do_with(
-    std::pair<std::vector<CachedExtentRef>, backref_pin_list_t>(),
+    std::pair<std::vector<CachedExtentRef>, backref_mapping_list_t>(),
     [this](auto &weak_read_ret) {
     return repeat_eagain([this, &weak_read_ret] {
       // Note: not tracked by shard_stats_t intentionally.
@@ -1253,7 +1253,7 @@ SegmentCleaner::clean_space_ret SegmentCleaner::clean_space()
 	  if (!pin_list.empty()) {
 	    auto it = pin_list.begin();
 	    auto &first_pin = *it;
-	    if (first_pin->get_key() < reclaim_state->start_pos) {
+	    if (first_pin.get_key() < reclaim_state->start_pos) {
 	      // BackrefManager::get_mappings may include a entry before
 	      // reclaim_state->start_pos, which is semantically inconsistent
 	      // with the requirements of the cleaner

src/crimson/os/seastore/async_cleaner.h

@@ -1487,7 +1487,7 @@ class SegmentCleaner : public SegmentProvider, public AsyncCleaner {
   using do_reclaim_space_ret = do_reclaim_space_ertr::future<>;
   do_reclaim_space_ret do_reclaim_space(
     const std::vector<CachedExtentRef> &backref_extents,
-    const backref_pin_list_t &pin_list,
+    const backref_mapping_list_t &pin_list,
     std::size_t &reclaimed,
     std::size_t &runs);
 

src/crimson/os/seastore/backref/backref_tree_node.cc

@@ -44,42 +44,6 @@ constexpr size_t LEAF_NODE_CAPACITY = 193;
 
 using BackrefNode = FixedKVNode<paddr_t>;
 
-struct backref_map_val_t {
-  extent_len_t len = 0;	///< length of extents
-  laddr_t laddr = L_ADDR_MIN; ///< logical address of extents
-  extent_types_t type = extent_types_t::ROOT;
-
-  backref_map_val_t() = default;
-  backref_map_val_t(
-    extent_len_t len,
-    laddr_t laddr,
-    extent_types_t type)
-    : len(len), laddr(laddr), type(type) {}
-
-  bool operator==(const backref_map_val_t& rhs) const noexcept {
-    return len == rhs.len && laddr == rhs.laddr;
-  }
-};
-
-std::ostream& operator<<(std::ostream &out, const backref_map_val_t& val);
-
-struct __attribute__((packed)) backref_map_val_le_t {
-  extent_len_le_t len = init_extent_len_le(0);
-  laddr_le_t laddr = laddr_le_t(L_ADDR_MIN);
-  extent_types_le_t type = 0;
-
-  backref_map_val_le_t() = default;
-  backref_map_val_le_t(const backref_map_val_le_t &) = default;
-  explicit backref_map_val_le_t(const backref_map_val_t &val)
-    : len(init_extent_len_le(val.len)),
-      laddr(val.laddr),
-      type(extent_types_le_t(val.type)) {}
-
-  operator backref_map_val_t() const {
-    return backref_map_val_t{len, laddr, (extent_types_t)type};
-  }
-};
-
 class BackrefInternalNode
   : public FixedKVInternalNode<
       INTERNAL_NODE_CAPACITY,

src/crimson/os/seastore/backref/backref_tree_node.h

@@ -22,7 +22,7 @@ phy_tree_root_t& get_phy_tree_root<
 template<>
 const get_phy_tree_root_node_ret get_phy_tree_root_node<
   crimson::os::seastore::backref::BackrefBtree>(
-  const RootBlockRef &root_block, op_context_t<paddr_t> c) {
+  const RootBlockRef &root_block, op_context_t c) {
   auto backref_root = root_block->backref_root_node;
   if (backref_root) {
     ceph_assert(backref_root->is_initial_pending()
@@ -91,7 +91,7 @@ BtreeBackrefManager::get_mapping(
   LOG_PREFIX(BtreeBackrefManager::get_mapping);
   TRACET("{}", t, offset);
   auto c = get_context(t);
-  return with_btree_ret<BackrefBtree, BackrefMappingRef>(
+  return with_btree<BackrefBtree>(
     cache,
     c,
     [c, offset](auto &btree) {
@@ -107,7 +107,7 @@ BtreeBackrefManager::get_mapping(
 	       c.trans, offset, iter.get_key(), iter.get_val());
 	return get_mapping_ret(
 	  interruptible::ready_future_marker{},
-	  iter.get_pin(c));
+	  BackrefMapping::create(iter.get_cursor(c)));
       }
     });
   });
@@ -122,7 +122,7 @@ BtreeBackrefManager::get_mappings(
   LOG_PREFIX(BtreeBackrefManager::get_mappings);
   TRACET("{}~{}", t, offset, end);
   auto c = get_context(t);
-  return with_btree_state<BackrefBtree, backref_pin_list_t>(
+  return with_btree_state<BackrefBtree, backref_mapping_list_t>(
     cache,
     c,
     [c, offset, end](auto &btree, auto &ret) {
@@ -141,7 +141,7 @@ BtreeBackrefManager::get_mappings(
 	  TRACET("{}~{} got {}, {}, repeat ...",
 	         c.trans, offset, end, pos.get_key(), pos.get_val());
 	  ceph_assert((pos.get_key().add_offset(pos.get_val().len)) > offset);
-	  ret.emplace_back(pos.get_pin(c));
+	  ret.emplace_back(BackrefMapping::create(pos.get_cursor(c)));
 	  return BackrefBtree::iterate_repeat_ret_inner(
 	    interruptible::ready_future_marker{},
 	    seastar::stop_iteration::no);
@@ -237,8 +237,8 @@ BtreeBackrefManager::new_mapping(
 	  });
 	});
     }).si_then([c](auto &&state) {
-      return new_mapping_iertr::make_ready_future<BackrefMappingRef>(
-	state.ret->get_pin(c));
+      return new_mapping_iertr::make_ready_future<BackrefMapping>(
+	BackrefMapping::create(state.ret->get_cursor(c)));
     });
 }
 
@@ -455,7 +455,7 @@ BtreeBackrefManager::scan_mapped_space(
 }
 
 BtreeBackrefManager::base_iertr::future<> _init_cached_extent(
-  op_context_t<paddr_t> c,
+  op_context_t c,
   const CachedExtentRef &e,
   BackrefBtree &btree,
   bool &ret)
@@ -503,7 +503,7 @@ BtreeBackrefManager::remove_mapping(
   paddr_t addr)
 {
   auto c = get_context(t);
-  return with_btree_ret<BackrefBtree, remove_mapping_result_t>(
+  return with_btree<BackrefBtree>(
     cache,
     c,
     [c, addr](auto &btree) mutable {
@@ -574,7 +574,7 @@ BtreeBackrefManager::retrieve_backref_extents_in_range(
 	ent.key);
 
       auto c = get_context(t);
-      return with_btree_ret<BackrefBtree, CachedExtentRef>(
+      return with_btree<BackrefBtree>(
 	cache,
 	c,
 	[c, &ent](auto &btree) {

src/crimson/os/seastore/backref/btree_backref_manager.cc

@@ -9,31 +9,11 @@
 
 namespace crimson::os::seastore::backref {
 
-class BtreeBackrefMapping : public BackrefMapping {
-public:
-  BtreeBackrefMapping(op_context_t<paddr_t> ctx)
-    : BackrefMapping(ctx) {}
-  BtreeBackrefMapping(
-    op_context_t<paddr_t> ctx,
-    BackrefLeafNodeRef parent,
-    uint16_t pos,
-    backref_map_val_t &val,
-    backref_node_meta_t &&meta)
-    : BackrefMapping(
-	val.type,
-	ctx,
-	parent,
-	pos,
-	val.laddr,
-	val.len,
-	std::forward<backref_node_meta_t>(meta)) {}
-};
-
 constexpr size_t BACKREF_BLOCK_SIZE = 4096;
 
 using BackrefBtree = FixedKVBtree<
   paddr_t, backref_map_val_t, BackrefInternalNode,
-  BackrefLeafNode, BtreeBackrefMapping, BACKREF_BLOCK_SIZE>;
+  BackrefLeafNode, BackrefCursor, BACKREF_BLOCK_SIZE>;
 
 class BtreeBackrefManager : public BackrefManager {
 public:
@@ -101,8 +81,8 @@ class BtreeBackrefManager : public BackrefManager {
 private:
   Cache &cache;
 
-  op_context_t<paddr_t> get_context(Transaction &t) {
-    return op_context_t<paddr_t>{cache, t};
+  op_context_t get_context(Transaction &t) {
+    return op_context_t{cache, t};
   }
 };
 

src/crimson/os/seastore/backref/btree_backref_manager.h

@@ -30,7 +30,7 @@ class BackrefManager {
    * Future will not resolve until all pins have resolved
    */
   using get_mappings_iertr = base_iertr;
-  using get_mappings_ret = get_mappings_iertr::future<backref_pin_list_t>;
+  using get_mappings_ret = get_mappings_iertr::future<backref_mapping_list_t>;
   virtual get_mappings_ret get_mappings(
     Transaction &t,
     paddr_t offset,
@@ -43,7 +43,7 @@ class BackrefManager {
    */
   using get_mapping_iertr = base_iertr::extend<
     crimson::ct_error::enoent>;
-  using get_mapping_ret = get_mapping_iertr::future<BackrefMappingRef>;
+  using get_mapping_ret = get_mapping_iertr::future<BackrefMapping>;
   virtual get_mapping_ret  get_mapping(
     Transaction &t,
     paddr_t offset) = 0;
@@ -63,7 +63,7 @@ class BackrefManager {
    * Insert new paddr_t -> laddr_t mapping
    */
   using new_mapping_iertr = base_iertr;
-  using new_mapping_ret = new_mapping_iertr::future<BackrefMappingRef>;
+  using new_mapping_ret = new_mapping_iertr::future<BackrefMapping>;
   virtual new_mapping_ret new_mapping(
     Transaction &t,
     paddr_t key,

src/crimson/os/seastore/backref_manager.h

@@ -3,25 +3,57 @@
 
 #pragma once
 
-#include "crimson/os/seastore/btree/btree_range_pin.h"
+#include "crimson/os/seastore/btree/btree_types.h"
 
 namespace crimson::os::seastore {
 
-class BackrefMapping : public BtreeNodeMapping<paddr_t, laddr_t> {
-  extent_types_t type;
+class BackrefMapping {
+  BackrefCursorRef cursor;
+
+  BackrefMapping(BackrefCursorRef cursor)
+      : cursor(std::move(cursor)) {}
+
 public:
-  BackrefMapping(op_context_t<paddr_t> ctx)
-    : BtreeNodeMapping(ctx) {}
-  template <typename... T>
-  BackrefMapping(extent_types_t type, T&&... t)
-    : BtreeNodeMapping(std::forward<T>(t)...),
-      type(type) {}
+  static BackrefMapping create(BackrefCursorRef cursor) {
+    return BackrefMapping(std::move(cursor));
+  }
+
+  BackrefMapping() = default;
+
+  BackrefMapping(const BackrefMapping &) = delete;
+  BackrefMapping(BackrefMapping &&) = default;
+
+  BackrefMapping &operator=(const BackrefMapping &) = delete;
+  BackrefMapping &operator=(BackrefMapping &&) = default;
+
+  ~BackrefMapping() = default;
+
+  bool is_viewable() const {
+    assert(cursor);
+    return cursor->is_viewable();
+  }
+
+  extent_len_t get_length() const {
+    assert(cursor);
+    return cursor->get_length();
+  }
+
+  laddr_t get_val() const {
+    assert(cursor);
+    return cursor->get_laddr();
+  }
+
+  paddr_t get_key() const {
+    assert(cursor);
+    return cursor->get_paddr();
+  }
+
   extent_types_t get_type() const {
-    return type;
+    assert(cursor);
+    return cursor->get_type();
   }
 };
 
-using BackrefMappingRef = std::unique_ptr<BackrefMapping>;
-using backref_pin_list_t = std::list<BackrefMappingRef>;
+using backref_mapping_list_t = std::list<BackrefMapping>;
 
 } // namespace crimson::os::seastore