crimson/os/seastore/transaction_manager: linking retired placeholders to

lba leaf nodes must be synchronous with getting child pos

Signed-off-by: Xuehan Xu <xuxuehan@qianxin.com>

diff --git a/src/crimson/os/seastore/lba_mapping.h b/src/crimson/os/seastore/lba_mapping.h
index a2be0319126db90154830485b2e7efb37cdba678..05d33a913781d8f0145b8f2498883989f4d668c4 100644 (file)
--- a/src/crimson/os/seastore/lba_mapping.h
+++ b/src/crimson/os/seastore/lba_mapping.h
@@ -46,6 +46,17 @@ public:
   LBAMapping &operator=(LBAMapping &&) = default;
   ~LBAMapping() = default;
 
+  // whether the removal of this mapping would cause
+  // other mappings to be removed.
+  //
+  // Note that this should only be called on complete
+  // indirect mappings
+  bool would_cascade_remove() const {
+    assert(is_indirect());
+    assert(is_complete_indirect());
+    return direct_cursor->get_refcount() == 1;
+  }
+
   // whether the mapping corresponds to a pending extent
   bool is_pending() const {
     return !is_indirect() && !is_data_stable();

diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc
index 8f004894eba43c2fb1c5185a0c0c4785b8da73e8..454d01fc45078f97a1425c675d8621425ce9f8fe 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.cc
+++ b/src/crimson/os/seastore/transaction_manager.cc
@@ -292,34 +292,35 @@ TransactionManager::_remove_indirect_mapping(
           });
         });
       } else {
-        auto unlinked_child = std::move(std::get<0>(ret));
+        auto remove_direct = mapping.would_cascade_remove();
+        if (remove_direct) {
+          auto unlinked_child = std::move(std::get<0>(ret));
+          auto retired_placeholder = cache->retire_absent_extent_addr(
+            t, mapping.get_intermediate_base(),
+            mapping.get_val(),
+            mapping.get_intermediate_length()
+          )->template cast<RetiredExtentPlaceholder>();
+          unlinked_child.child_pos.link_child(retired_placeholder.get());
+        }
         return lba_manager->remove_mapping(t, std::move(mapping)
-        ).si_then([child_pos=unlinked_child.child_pos, &t,
-                  FNAME, this](auto result) mutable {
+        ).si_then([&t, FNAME, remove_direct](auto result) mutable {
           ceph_assert(result.direct_result);
           auto &primary_result = result.result;
           ceph_assert(primary_result.refcount == 0);
           auto &direct_result = *result.direct_result;
           ceph_assert(direct_result.addr.is_paddr());
           ceph_assert(!direct_result.addr.get_paddr().is_zero());
-          if (direct_result.refcount == 0) {
-            auto retired_placeholder = cache->retire_absent_extent_addr(
-              t, direct_result.key,
-              direct_result.addr.get_paddr(),
-              direct_result.length
-            )->template cast<RetiredExtentPlaceholder>();
-            child_pos.link_child(retired_placeholder.get());
-          }
-            DEBUGT("removed indirect mapping {}~0x{:x} refcount={} offset={} "
-                   "with direct mapping {}~0x{:x} refcount={} offset={}",
-                   t, primary_result.addr,
-                   primary_result.length,
-                   primary_result.refcount,
-                   primary_result.key,
-                   direct_result.addr,
-                   direct_result.length,
-                   direct_result.refcount,
-                   direct_result.key);
+          ceph_assert(remove_direct == (direct_result.refcount == 0));
+          DEBUGT("removed indirect mapping {}~0x{:x} refcount={} offset={} "
+                 "with direct mapping {}~0x{:x} refcount={} offset={}",
+                 t, primary_result.addr,
+                 primary_result.length,
+                 primary_result.refcount,
+                 primary_result.key,
+                 direct_result.addr,
+                 direct_result.length,
+                 direct_result.refcount,
+                 direct_result.key);
           return ref_iertr::make_ready_future<
             _remove_mapping_result_t>(std::move(result));
         });
@@ -356,19 +357,16 @@ TransactionManager::_remove_direct_mapping(
     });
   } else {
     auto unlinked_child = std::move(std::get<0>(ret));
+    auto retired_placeholder = cache->retire_absent_extent_addr(
+      t, mapping.get_key(), mapping.get_val(), mapping.get_length()
+    )->template cast<RetiredExtentPlaceholder>();
+    unlinked_child.child_pos.link_child(retired_placeholder.get());
     return lba_manager->remove_mapping(t, std::move(mapping)
-    ).si_then([child_pos=unlinked_child.child_pos, &t,
-              FNAME, this](auto result) mutable {
+    ).si_then([&t, FNAME](auto result) mutable {
       auto &primary_result = result.result;
       ceph_assert(primary_result.refcount == 0);
       ceph_assert(primary_result.addr.is_paddr());
       ceph_assert(!primary_result.addr.get_paddr().is_zero());
-      auto retired_placeholder = cache->retire_absent_extent_addr(
-        t, primary_result.key,
-        primary_result.addr.get_paddr(),
-        primary_result.length
-      )->template cast<RetiredExtentPlaceholder>();
-      child_pos.link_child(retired_placeholder.get());
       DEBUGT("removed {}~0x{:x} refcount={} -- offset={}",
              t, primary_result.addr, primary_result.length,
              primary_result.refcount, primary_result.key);