crimson/os/seastore/object_data_handler: remove redundant indirect

mappings at the end of ObjectDataHandler::clone()

Fixes: https://tracker.ceph.com/issues/72539
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 7828a7e5f8052f855bc12399ae6ad1c5ed3c5c1f..8e9cf8462da775e6cf7592b7b22be2cbdf8c8946 100644 (file)
--- a/src/crimson/os/seastore/lba_mapping.h
+++ b/src/crimson/os/seastore/lba_mapping.h
@@ -111,6 +111,9 @@ public:
   bool is_zero_reserved() const {
     return !is_indirect() && get_val().is_zero();
   }
+  bool is_real() const {
+    return !is_indirect() && !get_val().is_zero();
+  }
 
   extent_len_t get_length() const {
     assert(!is_null());

diff --git a/src/crimson/os/seastore/object_data_handler.cc b/src/crimson/os/seastore/object_data_handler.cc
index 8ed405180881be857d0523ae775bef39221a92f4..7a3692e6e78f4c47cfcdc6fade024578fca7e7a4 100644 (file)
--- a/src/crimson/os/seastore/object_data_handler.cc
+++ b/src/crimson/os/seastore/object_data_handler.cc
@@ -804,7 +804,7 @@ ObjectDataHandler::punch_inner_mappings(
     extent_len_t>(overwrite_range.aligned_begin);
   return ctx.tm.remove_mappings_in_range(
     ctx.t, overwrite_range.aligned_begin,
-    unaligned_len, std::move(first_mapping));
+    unaligned_len, std::move(first_mapping), {});
 }
 
 // The last step in the multi-mapping-hole-punching scenario: remap
@@ -1403,6 +1403,8 @@ ObjectDataHandler::clone_ret ObjectDataHandler::clone(
     }
     return ctx.tm.get_pin(ctx.t, object_data.get_reserved_data_base()
     ).si_then([this, &object_data, &d_object_data, ctx](auto mapping) {
+      auto old_base = object_data.get_reserved_data_base();
+      auto old_len = object_data.get_reserved_data_len();
       return prepare_data_reservation(
        ctx,
        d_object_data,
@@ -1435,12 +1437,15 @@ ObjectDataHandler::clone_ret ObjectDataHandler::clone(
            object_data.get_reserved_data_len());
          return ctx.tm.remove(ctx.t, std::move(*mapping));
        });
-      }).si_then([ctx, &object_data,
-                 mapping=std::move(mapping)](auto pos) mutable {
+      }).si_then([ctx, &object_data, mapping](auto pos) mutable {
        auto base = object_data.get_reserved_data_base();
        auto len = object_data.get_reserved_data_len();
        return ctx.tm.clone_range(
          ctx.t, base, len, std::move(pos), std::move(mapping), false);
+      }).si_then([ctx, mapping, old_base, old_len] {
+       return ctx.tm.remove_mappings_in_range(
+         ctx.t, old_base, old_len, std::move(mapping), {false, true}
+       ).discard_result();
       });
     }).handle_error_interruptible(
       clone_iertr::pass_further{},

diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h
index 2a35c7e34d10a6d2914b121e69405eb8ee824b40..4f3eac692110b187eaba1b8a9b8040541851db27 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.h
+++ b/src/crimson/os/seastore/transaction_manager.h
@@ -1100,6 +1100,10 @@ public:
     });
   }
 
+  struct remove_mappings_param_t {
+    bool cascade_remove_on_indirect = true;
+    bool skip_direct_mapping = false;
+  };
   /*
    * remove_mappings_in_range
    *
@@ -1111,7 +1115,8 @@ public:
     Transaction &t,
     laddr_t start,
     objaddr_t unaligned_len,
-    LBAMapping first_mapping)
+    LBAMapping first_mapping,
+    remove_mappings_param_t params)
   {
     LOG_PREFIX(TransactionManager::remove_mappings_in_range);
     SUBDEBUGT(seastore_tm, "{}~{}, first_mapping: {}",
@@ -1119,20 +1124,36 @@ public:
     auto mapping = co_await first_mapping.refresh();
     while (!mapping.is_end()) {
       assert(mapping.get_key() >= start);
-      auto mapping_end =
-       (mapping.get_key() + mapping.get_length()).checked_to_laddr();
+      auto mapping_end = (mapping.get_key() + mapping.get_length()
+       ).checked_to_laddr();
       if (mapping_end > start + unaligned_len) {
        break;
       }
-      mapping = co_await remove(t, std::move(mapping)
-      ).handle_error_interruptible(
-       punch_mappings_iertr::pass_further{},
-       crimson::ct_error::assert_all{
-         "remove_mappings_in_range hit invalid error"
-       }
-      );
+      if (params.skip_direct_mapping && mapping.is_real()) {
+       mapping = co_await mapping.next();
+       continue;
+      }
+      if (params.cascade_remove_on_indirect ||
+         mapping.is_zero_reserved()) {
+       mapping = co_await remove(t, std::move(mapping)
+       ).handle_error_interruptible(
+         punch_mappings_iertr::pass_further{},
+         crimson::ct_error::assert_all{
+           "remove_mappings_in_range hit invalid error"
+         }
+       );
+      } else {
+       mapping = co_await _remove_indirect_mapping_only(
+         t, std::move(mapping)
+       ).handle_error_interruptible(
+         punch_mappings_iertr::pass_further{},
+         crimson::ct_error::assert_all{
+           "remove_mappings_in_range hit invalid error"
+         }
+       );
+      }
     }
-    co_return std::move(mapping);
+    co_return mapping;
   }
 
   ~TransactionManager();