crimson/os/seastore/BtreeLBAManager: support refresh LBAMapping

Signed-off-by: Zhang Song <[email protected]>
(cherry picked from commit dca4e56)

Author: zhscn

src/crimson/os/seastore/lba_manager.h

@@ -230,6 +230,12 @@ class LBAManager {
     laddr_t laddr,
     extent_len_t len) = 0;
 
+  using refresh_lba_mapping_iertr = base_iertr;
+  using refresh_lba_mapping_ret = refresh_lba_mapping_iertr::future<LBAMapping>;
+  virtual refresh_lba_mapping_ret refresh_lba_mapping(
+    Transaction &t,
+    LBAMapping mapping) = 0;
+
   virtual ~LBAManager() {}
 };
 using LBAManagerRef = std::unique_ptr<LBAManager>;

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

@@ -721,6 +721,100 @@ BtreeLBAManager::get_physical_extent_if_live(
     });
 }
 
+BtreeLBAManager::refresh_lba_mapping_ret
+BtreeLBAManager::refresh_lba_mapping(Transaction &t, LBAMapping mapping)
+{
+  assert(mapping.is_linked_direct());
+  if (mapping.is_viewable()) {
+    return refresh_lba_mapping_iertr::make_ready_future<
+      LBAMapping>(std::move(mapping));
+  }
+  auto c = get_context(t);
+  return with_btree_state<LBABtree, LBAMapping>(
+    cache,
+    c,
+    std::move(mapping),
+    [c, this](LBABtree &btree, LBAMapping &mapping) mutable
+  {
+    return refresh_lba_cursor(c, btree, *mapping.direct_cursor
+    ).si_then([c, this, &btree, &mapping] {
+      if (mapping.indirect_cursor) {
+	return refresh_lba_cursor(c, btree, *mapping.indirect_cursor);
+      }
+      return refresh_lba_cursor_iertr::make_ready_future();
+#ifndef NDEBUG
+    }).si_then([&mapping] {
+      assert(mapping.is_viewable());
+#endif
+    });
+  });
+}
+
+BtreeLBAManager::refresh_lba_cursor_ret
+BtreeLBAManager::refresh_lba_cursor(
+  op_context_t c,
+  LBABtree &btree,
+  LBACursor &cursor)
+{
+  LOG_PREFIX(BtreeLBAManager::refresh_lba_cursor);
+  stats.num_refresh_parent_total++;
+
+  if (!cursor.parent->is_valid()) {
+    stats.num_refresh_invalid_parent++;
+    TRACET("cursor {} parent is invalid, re-search from scratch",
+	   c.trans, cursor);
+    return btree.lower_bound(c, cursor.get_laddr()
+    ).si_then([&cursor](LBABtree::iterator iter) {
+      auto leaf = iter.get_leaf_node();
+      cursor.parent = leaf;
+      cursor.modifications = leaf->modifications;
+      cursor.pos = iter.get_leaf_pos();
+      if (!cursor.is_end()) {
+	ceph_assert(!iter.is_end());
+	ceph_assert(iter.get_key() == cursor.get_laddr());
+	cursor.val = iter.get_val();
+	assert(cursor.is_viewable());
+      }
+    });
+  }
+
+  auto [viewable, state] = cursor.parent->is_viewable_by_trans(c.trans);
+  auto leaf = cursor.parent->cast<LBALeafNode>();
+
+  TRACET("cursor: {} viewable: {} state: {}",
+	 c.trans, cursor, viewable, state);
+
+  if (!viewable) {
+    stats.num_refresh_unviewable_parent++;
+    leaf = leaf->find_pending_version(c.trans, cursor.get_laddr());
+    cursor.parent = leaf;
+  }
+
+  if (!viewable ||
+      leaf->modified_since(cursor.modifications)) {
+    if (viewable) {
+      stats.num_refresh_modified_viewable_parent++;
+    }
+
+    cursor.modifications = leaf->modifications;
+    if (cursor.is_end()) {
+      cursor.pos = leaf->get_size();
+      assert(!cursor.val);
+    } else {
+      auto i = leaf->lower_bound(cursor.get_laddr());
+      cursor.pos = i.get_offset();
+      cursor.val = i.get_val();
+
+      auto iter = LBALeafNode::iterator(leaf.get(), cursor.pos);
+      ceph_assert(iter.get_key() == cursor.key);
+      ceph_assert(iter.get_val() == cursor.val);
+      assert(cursor.is_viewable());
+    }
+  }
+
+  return refresh_lba_cursor_iertr::make_ready_future();
+}
+
 void BtreeLBAManager::register_metrics()
 {
   LOG_PREFIX(BtreeLBAManager::register_metrics);
@@ -740,6 +834,26 @@ void BtreeLBAManager::register_metrics()
         stats.num_alloc_extents_iter_nexts,
         sm::description("total number of iterator next operations during extent allocation")
       ),
+      sm::make_counter(
+        "refresh_parent_total",
+        stats.num_refresh_parent_total,
+        sm::description("total number of refreshed cursors")
+      ),
+      sm::make_counter(
+        "refresh_invalid_parent",
+        stats.num_refresh_invalid_parent,
+        sm::description("total number of refreshed cursors with invalid parents")
+      ),
+      sm::make_counter(
+        "refresh_unviewable_parent",
+        stats.num_refresh_unviewable_parent,
+        sm::description("total number of refreshed cursors with unviewable parents")
+      ),
+      sm::make_counter(
+        "refresh_modified_viewable_parent",
+        stats.num_refresh_modified_viewable_parent,
+        sm::description("total number of refreshed cursors with viewable but modified parents")
+      ),
     }
   );
 }

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

@@ -263,12 +263,21 @@ class BtreeLBAManager : public LBAManager {
     paddr_t addr,
     laddr_t laddr,
     extent_len_t len) final;
+
+  refresh_lba_mapping_ret refresh_lba_mapping(
+    Transaction &t,
+    LBAMapping mapping) final;
+
 private:
   Cache &cache;
 
   struct {
     uint64_t num_alloc_extents = 0;
     uint64_t num_alloc_extents_iter_nexts = 0;
+    uint64_t num_refresh_parent_total = 0;
+    uint64_t num_refresh_invalid_parent = 0;
+    uint64_t num_refresh_unviewable_parent = 0;
+    uint64_t num_refresh_modified_viewable_parent = 0;
   } stats;
 
   struct alloc_mapping_info_t {
@@ -510,6 +519,13 @@ class BtreeLBAManager : public LBAManager {
     Transaction &t,
     laddr_t addr,
     extent_len_t len);
+
+  using refresh_lba_cursor_iertr = base_iertr;
+  using refresh_lba_cursor_ret = refresh_lba_cursor_iertr::future<>;
+  refresh_lba_cursor_ret refresh_lba_cursor(
+    op_context_t c,
+    LBABtree &btree,
+    LBACursor &cursor);
 };
 using BtreeLBAManagerRef = std::unique_ptr<BtreeLBAManager>;
 

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

@@ -44,39 +44,6 @@ void LBALeafNode::resolve_relative_addrs(paddr_t base)
   }
 }
 
-void LBALeafNode::maybe_fix_mapping_pos(BtreeLBAMapping &mapping)
-{
-  assert(mapping.get_parent() == this);
-  auto key = mapping.is_indirect()
-    ? mapping.get_intermediate_base()
-    : mapping.get_key();
-  if (key != iter_idx(mapping.get_pos()).get_key()) {
-    auto iter = lower_bound(key);
-    {
-      // a mapping that no longer exist or has its value
-      // modified is considered an outdated one, and
-      // shouldn't be used anymore
-      ceph_assert(iter != end());
-      assert(iter.get_val() == mapping.get_map_val());
-    }
-    mapping._new_pos(iter.get_offset());
-  }
-}
-
-BtreeLBAMappingRef LBALeafNode::get_mapping(
-  op_context_t c, laddr_t laddr)
-{
-  auto iter = lower_bound(laddr);
-  ceph_assert(iter != end() && iter->get_key() == laddr);
-  auto val = iter.get_val();
-  return std::make_unique<BtreeLBAMapping>(
-    c,
-    this,
-    iter.get_offset(),
-    val,
-    lba_node_meta_t{laddr, (laddr + val.len).checked_to_laddr(), 0});
-}
-
 void LBALeafNode::update(
   internal_const_iterator_t iter,
   lba_map_val_t val)

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

@@ -278,9 +278,6 @@ struct LBALeafNode
   }
 
   std::ostream &print_detail(std::ostream &out) const final;
-
-  void maybe_fix_mapping_pos(BtreeLBAMapping &mapping);
-  std::unique_ptr<BtreeLBAMapping> get_mapping(op_context_t c, laddr_t laddr);
 };
 using LBALeafNodeRef = TCachedExtentRef<LBALeafNode>;
 

src/crimson/os/seastore/transaction_manager.h

@@ -277,10 +277,9 @@ class TransactionManager : public ExtentCallbackInterface {
     SUBDEBUGT(seastore_tm, "{} {} 0x{:x}~0x{:x} direct_off=0x{:x} ...",
               t, T::TYPE, pin, partial_off, partial_len, direct_partial_off);
 
-    auto fut = base_iertr::make_ready_future<LBAMapping>();
-    // TODO: refresh pin
-    return fut.si_then([&t, this, direct_partial_off, partial_len,
-			maybe_init=std::move(maybe_init)](auto npin) mutable {
+    return lba_manager->refresh_lba_mapping(t, std::move(pin)
+    ).si_then([&t, this, direct_partial_off, partial_len,
+	       maybe_init=std::move(maybe_init)](auto npin) mutable {
       // checking the lba child must be atomic with creating
       // and linking the absent child
       auto ret = get_extent_if_linked<T>(t, std::move(npin));
@@ -525,12 +524,12 @@ class TransactionManager : public ExtentCallbackInterface {
                 t, original_laddr, original_len, original_paddr, remaps.size(), pin);
       // The according extent might be stable or pending.
       auto fut = base_iertr::now();
-      if (!pin->is_indirect()) {
-        ceph_assert(!pin->is_clone());
-
-	// TODO: refresh pin
-
-	fut = fut.si_then([this, &t, &pin] {
+      if (!pin.is_indirect()) {
+        ceph_assert(!pin.is_clone());
+	fut = fut.si_then([this, &t, &pin]() mutable {
+	  return lba_manager->refresh_lba_mapping(t, std::move(pin));
+	}).si_then([this, &t, &pin](auto newpin) {
+	  pin = std::move(newpin);
 	  if (full_extent_integrity_check) {
 	    return read_pin<T>(t, pin.duplicate()
             ).si_then([](auto maybe_indirect_extent) {

src/test/crimson/seastore/test_transaction_manager.cc

@@ -726,6 +726,12 @@ struct transaction_manager_test_t :
       }).unsafe_get();
   }
 
+  LBAMapping refresh_lba_mapping(test_transaction_t &t, LBAMapping mapping) {
+    return with_trans_intr(*t.t, [this, mapping=std::move(mapping)](auto &t) mutable {
+      return lba_manager->refresh_lba_mapping(t, std::move(mapping));
+    }).unsafe_get();
+  }
+
   bool try_submit_transaction(test_transaction_t t) {
     using ertr = with_trans_ertr<TransactionManager::submit_transaction_iertr>;
     using ret = ertr::future<bool>;
@@ -2135,8 +2141,7 @@ TEST_P(tm_single_device_test_t, invalid_lba_mapping_detect)
       assert(pin.is_viewable());
       std::ignore = alloc_extent(t, get_laddr_hint((LEAF_NODE_CAPACITY + 1) * 4096), 4096, 'a');
       assert(!pin.is_viewable());
-      // TODO: refresh pin
-      // pin->maybe_fix_pos();
+      pin = refresh_lba_mapping(t, pin.duplicate());
       auto extent2 = with_trans_intr(*(t.t), [&pin](auto& trans) {
         auto v = pin.get_logical_extent(trans);
         assert(v.has_child());