crimson/os/seastore/../btree_lba_manager: rework get_mapping(s)

Remove the unnecessary recursion to simplify the implementations.

Signed-off-by: Yingxin Cheng <[email protected]>

Author: cyx1231st

src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc

@@ -147,96 +147,89 @@ BtreeLBAManager::mkfs(
 BtreeLBAManager::get_mappings_ret
 BtreeLBAManager::get_mappings(
   Transaction &t,
-  laddr_t offset, extent_len_t length)
+  laddr_t laddr,
+  extent_len_t length)
 {
   LOG_PREFIX(BtreeLBAManager::get_mappings);
-  TRACET("{}~{}", t, offset, length);
+  TRACET("{}~0x{:x} ...", t, laddr, length);
   auto c = get_context(t);
   return with_btree_state<LBABtree, lba_pin_list_t>(
-    cache,
-    c,
-    [c, offset, length, FNAME, this](auto &btree, auto &ret) {
+    cache, c,
+    [FNAME, this, c, laddr, length](auto& btree, auto& ret)
+  {
+    return _get_mappings(c, btree, laddr, length
+    ).si_then([FNAME, this, c, laddr, length, &btree, &ret](auto pin_list) {
       return seastar::do_with(
-	std::list<BtreeLBAMappingRef>(),
-	[offset, length, c, FNAME, this, &ret, &btree](auto &pin_list) {
-	return LBABtree::iterate_repeat(
-	  c,
-	  btree.upper_bound_right(c, offset),
-	  [&pin_list, offset, length, c, FNAME](auto &pos) {
-	    if (pos.is_end() || pos.get_key() >= (offset + length)) {
-	      TRACET("{}~{} done with {} results",
-		     c.trans, offset, length, pin_list.size());
-	      return LBABtree::iterate_repeat_ret_inner(
-		interruptible::ready_future_marker{},
-		seastar::stop_iteration::yes);
-	    }
-	    TRACET("{}~{} got {}, {}, repeat ...",
-		   c.trans, offset, length, pos.get_key(), pos.get_val());
-	    ceph_assert((pos.get_key() + pos.get_val().len) > offset);
-	    pin_list.push_back(pos.get_pin(c));
-	    return LBABtree::iterate_repeat_ret_inner(
-	      interruptible::ready_future_marker{},
-	      seastar::stop_iteration::no);
-	  }).si_then([this, &ret, c, &pin_list] {
-	    return _get_original_mappings(c, pin_list
-	    ).si_then([&ret](auto _ret) {
-	      ret = std::move(_ret);
-	    });
-	  });
-	});
+        std::move(pin_list),
+        [FNAME, this, c, laddr, length, &btree, &ret](auto& pin_list)
+      {
+        return trans_intr::do_for_each(
+          pin_list,
+          [FNAME, this, c, laddr, length, &btree, &ret](auto& pin)
+        {
+          if (!pin->is_indirect()) {
+            TRACET("{}~0x{:x} got direct pin {}~0x{:x}",
+                   c.trans, laddr, length, pin->get_key(), pin->get_length());
+            ret.emplace_back(std::move(pin));
+            return get_mappings_iertr::now();
+          }
+          auto key = pin->get_key();
+          auto intermediate_key = pin->get_raw_val().get_laddr();
+          auto intermediate_len = pin->get_length();
+          return _get_mappings(c, btree, intermediate_key, intermediate_len
+          ).si_then([FNAME, c, laddr, length, key, intermediate_key,
+                     intermediate_len, &ret](auto pin_list) {
+            ceph_assert(pin_list.size() == 1);
+            auto& pin = pin_list.front();
+            assert(!pin->is_indirect());
+            assert(pin->get_key() <= intermediate_key);
+            assert(pin->get_key() + pin->get_length() >= intermediate_key + intermediate_len);
+            TRACET("{}~0x{:x} got indirect pin {}~0x{:x}->{}({}~0x{:x})",
+                   c.trans, laddr, length, key, intermediate_len, intermediate_key,
+                   pin->get_key(), pin->get_length());
+            pin->make_indirect(key, intermediate_len, intermediate_key);
+            ret.emplace_back(std::move(pin));
+            return get_mappings_iertr::now();
+          });
+        });
+      });
     });
+  });
 }
 
-BtreeLBAManager::_get_original_mappings_ret
-BtreeLBAManager::_get_original_mappings(
+BtreeLBAManager::_get_mappings_ret
+BtreeLBAManager::_get_mappings(
   op_context_t<laddr_t> c,
-  std::list<BtreeLBAMappingRef> &pin_list)
+  LBABtree& btree,
+  laddr_t laddr,
+  extent_len_t length)
 {
+  LOG_PREFIX(BtreeLBAManager::_get_mappings);
+  TRACET("{}~0x{:x} ...", c.trans, laddr, length);
   return seastar::do_with(
-    lba_pin_list_t(),
-    [this, c, &pin_list](auto &ret) {
-    return trans_intr::do_for_each(
-      pin_list,
-      [this, c, &ret](auto &pin) {
-	LOG_PREFIX(BtreeLBAManager::get_mappings);
-	if (pin->get_raw_val().is_paddr()) {
-	  ret.emplace_back(std::move(pin));
-	  return get_mappings_iertr::now();
-	}
-	TRACET(
-	  "getting original mapping for indirect mapping {}~{}",
-	  c.trans, pin->get_key(), pin->get_length());
-	return this->get_mappings(
-	  c.trans, pin->get_raw_val().get_laddr(), pin->get_length()
-	).si_then([&pin, &ret, c](auto new_pin_list) {
-	  LOG_PREFIX(BtreeLBAManager::get_mappings);
-	  assert(new_pin_list.size() == 1);
-	  auto &new_pin = new_pin_list.front();
-	  auto intermediate_key = pin->get_raw_val().get_laddr();
-	  assert(!new_pin->is_indirect());
-	  assert(new_pin->get_key() <= intermediate_key);
-	  assert(new_pin->get_key() + new_pin->get_length() >=
-	  intermediate_key + pin->get_length());
-
-	  TRACET("Got mapping {}~{} for indirect mapping {}~{}, "
-	    "intermediate_key {}",
-	    c.trans,
-	    new_pin->get_key(), new_pin->get_length(),
-	    pin->get_key(), pin->get_length(),
-	    pin->get_raw_val().get_laddr());
-	  auto &btree_new_pin = static_cast<BtreeLBAMapping&>(*new_pin);
-	  btree_new_pin.make_indirect(
-	    pin->get_key(),
-	    pin->get_length(),
-	    pin->get_raw_val().get_laddr());
-	  ret.emplace_back(std::move(new_pin));
-	  return seastar::now();
-	}).handle_error_interruptible(
-	  crimson::ct_error::input_output_error::pass_further{},
-	  crimson::ct_error::assert_all("unexpected enoent")
-	);
+    std::list<BtreeLBAMappingRef>(),
+    [FNAME, c, laddr, length, &btree](auto& ret)
+  {
+    return LBABtree::iterate_repeat(
+      c,
+      btree.upper_bound_right(c, laddr),
+      [FNAME, c, laddr, length, &ret](auto& pos)
+    {
+      if (pos.is_end() || pos.get_key() >= (laddr + length)) {
+        TRACET("{}~0x{:x} done with {} results",
+               c.trans, laddr, length, ret.size());
+        return LBABtree::iterate_repeat_ret_inner(
+          interruptible::ready_future_marker{},
+          seastar::stop_iteration::yes);
       }
-    ).si_then([&ret] {
+      TRACET("{}~0x{:x} got {}, {}, repeat ...",
+             c.trans, laddr, length, pos.get_key(), pos.get_val());
+      ceph_assert((pos.get_key() + pos.get_val().len) > laddr);
+      ret.push_back(pos.get_pin(c));
+      return LBABtree::iterate_repeat_ret_inner(
+        interruptible::ready_future_marker{},
+        seastar::stop_iteration::no);
+    }).si_then([&ret] {
       return std::move(ret);
     });
   });
@@ -245,62 +238,58 @@ BtreeLBAManager::_get_original_mappings(
 BtreeLBAManager::get_mapping_ret
 BtreeLBAManager::get_mapping(
   Transaction &t,
-  laddr_t offset)
+  laddr_t laddr)
 {
   LOG_PREFIX(BtreeLBAManager::get_mapping);
-  TRACET("{}", t, offset);
-  return _get_mapping(t, offset
-  ).si_then([](auto pin) {
-    return get_mapping_iertr::make_ready_future<LBAMappingRef>(std::move(pin));
+  TRACET("{} ...", t, laddr);
+  auto c = get_context(t);
+  return with_btree_ret<LBABtree, LBAMappingRef>(
+    cache, c,
+    [FNAME, this, c, laddr](auto& btree)
+  {
+    return _get_mapping(c, btree, laddr
+    ).si_then([FNAME, this, c, laddr, &btree](auto pin) {
+      if (!pin->is_indirect()) {
+        TRACET("{} got direct pin len 0x{:x}",
+               c.trans, laddr, pin->get_length());
+        return get_mapping_iertr::make_ready_future<LBAMappingRef>(std::move(pin));
+      }
+      assert(laddr == pin->get_key());
+      auto len = pin->get_length();
+      laddr_t direct_laddr = pin->get_raw_val().get_laddr();
+      return _get_mapping(c, btree, direct_laddr
+      ).si_then([FNAME, c, laddr, len](auto pin) {
+        ceph_assert(len == pin->get_length());
+        TRACET("{} got indirect pin->{}~0x{:x}",
+               c.trans, laddr, pin->get_key(), len);
+        pin->make_indirect(laddr, len);
+        return get_mapping_iertr::make_ready_future<LBAMappingRef>(std::move(pin));
+      });
+    });
   });
 }
 
 BtreeLBAManager::_get_mapping_ret
 BtreeLBAManager::_get_mapping(
-  Transaction &t,
-  laddr_t offset)
+  op_context_t<laddr_t> c,
+  LBABtree& btree,
+  laddr_t laddr)
 {
   LOG_PREFIX(BtreeLBAManager::_get_mapping);
-  TRACET("{}", t, offset);
-  auto c = get_context(t);
-  return with_btree_ret<LBABtree, BtreeLBAMappingRef>(
-    cache,
-    c,
-    [FNAME, c, offset, this](auto &btree) {
-      return btree.lower_bound(
-	c, offset
-      ).si_then([FNAME, offset, c](auto iter) -> _get_mapping_ret {
-	if (iter.is_end() || iter.get_key() != offset) {
-	  ERRORT("laddr={} doesn't exist", c.trans, offset);
-	  return crimson::ct_error::enoent::make();
-	} else {
-	  TRACET("{} got {}, {}",
-	         c.trans, offset, iter.get_key(), iter.get_val());
-	  auto e = iter.get_pin(c);
-	  return _get_mapping_ret(
-	    interruptible::ready_future_marker{},
-	    std::move(e));
-	}
-      }).si_then([this, c](auto pin) -> _get_mapping_ret {
-	if (pin->get_raw_val().is_laddr()) {
-	  return seastar::do_with(
-	    std::move(pin),
-	    [this, c](auto &pin) {
-	    return _get_mapping(
-	      c.trans, pin->get_raw_val().get_laddr()
-	    ).si_then([&pin](auto new_pin) {
-	      ceph_assert(pin->get_length() == new_pin->get_length());
-	      new_pin->make_indirect(
-		pin->get_key(),
-		pin->get_length());
-	      return new_pin;
-	    });
-	  });
-	} else {
-	  return get_mapping_iertr::make_ready_future<BtreeLBAMappingRef>(std::move(pin));
-	}
-      });
-    });
+  TRACET("{} ...", c.trans, laddr);
+  return btree.lower_bound(
+    c, laddr
+  ).si_then([FNAME, c, laddr](auto iter) -> _get_mapping_ret {
+    if (iter.is_end() || iter.get_key() != laddr) {
+      ERRORT("{} doesn't exist", c.trans, laddr);
+      return crimson::ct_error::enoent::make();
+    }
+    TRACET("{} got value {}", c.trans, laddr, iter.get_val());
+    auto e = iter.get_pin(c);
+    return _get_mapping_ret(
+      interruptible::ready_future_marker{},
+      std::move(e));
+  });
 }
 
 BtreeLBAManager::alloc_extents_ret

src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.h

@@ -702,13 +702,16 @@ class BtreeLBAManager : public LBAManager {
 
   using _get_mapping_ret = get_mapping_iertr::future<BtreeLBAMappingRef>;
   _get_mapping_ret _get_mapping(
-    Transaction &t,
+    op_context_t<laddr_t> c,
+    LBABtree& btree,
     laddr_t offset);
 
-  using _get_original_mappings_ret = get_mappings_ret;
-  _get_original_mappings_ret _get_original_mappings(
+  using _get_mappings_ret = get_mappings_iertr::future<std::list<BtreeLBAMappingRef>>;
+  _get_mappings_ret _get_mappings(
     op_context_t<laddr_t> c,
-    std::list<BtreeLBAMappingRef> &pin_list);
+    LBABtree& btree,
+    laddr_t offset,
+    extent_len_t length);
 
   using _decref_intermediate_ret = ref_iertr::future<
     std::optional<ref_update_result_t>>;