crimson/os/seastore/lba_manager: simplify LBAManager::update_mapping(s)() signature

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>

diff --git a/src/crimson/os/seastore/lba_manager.cc b/src/crimson/os/seastore/lba_manager.cc
index 7d32855ee451d556d22da83b1c881ae38ee88add..50850a7de56d1a00011ce5d1d275c3026639d9eb 100644 (file)
--- a/src/crimson/os/seastore/lba_manager.cc
+++ b/src/crimson/os/seastore/lba_manager.cc
@@ -6,27 +6,6 @@
 
 namespace crimson::os::seastore {
 
-LBAManager::update_mappings_ret
-LBAManager::update_mappings(
-  Transaction& t,
-  const std::list<LogicalChildNodeRef>& extents)
-{
-  return trans_intr::do_for_each(extents,
-                                [this, &t](auto &extent) {
-    return update_mapping(
-      t,
-      extent->get_laddr(),
-      extent->get_length(),
-      extent->get_prior_paddr_and_reset(),
-      extent->get_length(),
-      extent->get_paddr(),
-      extent->get_last_committed_crc(),
-      nullptr  // all the extents should have already been
-               // added to the fixed_kv_btree
-    ).discard_result();
-  });
-}
-
 LBAManagerRef lba_manager::create_lba_manager(Cache &cache) {
   return LBAManagerRef(new btree::BtreeLBAManager(cache));
 }

diff --git a/src/crimson/os/seastore/lba_manager.h b/src/crimson/os/seastore/lba_manager.h
index a4da86f111b5f25ca600acc126470a6dacb3eb1f..331e8b56e1522b9a79c29976928ffc453d20d0d6 100644 (file)
--- a/src/crimson/os/seastore/lba_manager.h
+++ b/src/crimson/os/seastore/lba_manager.h
@@ -192,7 +192,7 @@ public:
   /**
    * update_mapping
    *
-   * update lba mapping for a delayed allocated extent
+   * update lba mapping for rewrite
    */
   using update_mapping_iertr = base_iertr;
   using update_mapping_ret = base_iertr::future<extent_ref_count_t>;
@@ -201,10 +201,7 @@ public:
     laddr_t laddr,
     extent_len_t prev_len,
     paddr_t prev_addr,
-    extent_len_t len,
-    paddr_t paddr,
-    uint32_t checksum,
-    LogicalChildNode *nextent) = 0;
+    LogicalChildNode& nextent) = 0;
 
   /**
    * update_mappings
@@ -213,9 +210,9 @@ public:
    */
   using update_mappings_iertr = update_mapping_iertr;
   using update_mappings_ret = update_mappings_iertr::future<>;
-  update_mappings_ret update_mappings(
+  virtual update_mappings_ret update_mappings(
     Transaction& t,
-    const std::list<LogicalChildNodeRef>& extents);
+    const std::list<LogicalChildNodeRef>& extents) = 0;
 
   /**
    * get_physical_extent_if_live

diff --git a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc
index 5f4816e6cc46829c0cfd4bca2c46a74091d8f6bb..74d85390a333e362cc21e636ba2738907f6cea02 100644 (file)
--- a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc
+++ b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc
@@ -595,19 +595,21 @@ BtreeLBAManager::update_mapping(
   laddr_t laddr,
   extent_len_t prev_len,
   paddr_t prev_addr,
-  extent_len_t len,
-  paddr_t addr,
-  uint32_t checksum,
-  LogicalChildNode *nextent)
+  LogicalChildNode& nextent)
 {
   LOG_PREFIX(BtreeLBAManager::update_mapping);
-  TRACET("laddr={}, paddr {} => {}", t, laddr, prev_addr, addr);
+  auto addr = nextent.get_paddr();
+  auto len = nextent.get_length();
+  auto checksum = nextent.get_last_committed_crc();
+  TRACET("laddr={}, paddr {}~0x{:x} => {}~0x{:x}, crc=0x{:x}",
+         t, laddr, prev_addr, prev_len, addr, len, checksum);
+  assert(laddr == nextent.get_laddr());
+  assert(!addr.is_null());
   return _update_mapping(
     t,
     laddr,
-    [prev_addr, addr, prev_len, len, checksum](
-      const lba_map_val_t &in) {
-      assert(!addr.is_null());
+    [prev_addr, addr, prev_len, len, checksum]
+    (const lba_map_val_t &in) {
       lba_map_val_t ret = in;
       ceph_assert(in.pladdr.is_paddr());
       ceph_assert(in.pladdr.get_paddr() == prev_addr);
@@ -617,11 +619,11 @@ BtreeLBAManager::update_mapping(
       ret.checksum = checksum;
       return ret;
     },
-    nextent
-  ).si_then([&t, laddr, prev_addr, addr, FNAME](auto res) {
+    &nextent
+  ).si_then([&t, laddr, prev_addr, prev_len, addr, len, checksum, FNAME](auto res) {
       auto &result = res.map_value;
-      DEBUGT("laddr={}, paddr {} => {} done -- {}",
-             t, laddr, prev_addr, addr, result);
+      DEBUGT("laddr={}, paddr {}~0x{:x} => {}~0x{:x}, crc=0x{:x} done -- {}",
+             t, laddr, prev_addr, prev_len, addr, len, checksum, result);
       return update_mapping_iertr::make_ready_future<
        extent_ref_count_t>(result.refcount);
     },
@@ -633,6 +635,51 @@ BtreeLBAManager::update_mapping(
   );
 }
 
+BtreeLBAManager::update_mappings_ret
+BtreeLBAManager::update_mappings(
+  Transaction& t,
+  const std::list<LogicalChildNodeRef>& extents)
+{
+  return trans_intr::do_for_each(extents, [this, &t](auto &extent) {
+    LOG_PREFIX(BtreeLBAManager::update_mappings);
+    auto laddr = extent->get_laddr();
+    auto prev_addr = extent->get_prior_paddr_and_reset();
+    auto len = extent->get_length();
+    auto addr = extent->get_paddr();
+    auto checksum = extent->get_last_committed_crc();
+    TRACET("laddr={}, paddr {}~0x{:x} => {}, crc=0x{:x}",
+           t, laddr, prev_addr, len, addr, checksum);
+    assert(!addr.is_null());
+    return _update_mapping(
+      t,
+      laddr,
+      [prev_addr, addr, len, checksum](
+        const lba_map_val_t &in) {
+        lba_map_val_t ret = in;
+        ceph_assert(in.pladdr.is_paddr());
+        ceph_assert(in.pladdr.get_paddr() == prev_addr);
+        ceph_assert(in.len == len);
+        ret.pladdr = addr;
+        ret.checksum = checksum;
+        return ret;
+      },
+      nullptr   // all the extents should have already been
+                // added to the fixed_kv_btree
+    ).si_then([&t, laddr, prev_addr, len, addr, checksum, FNAME](auto res) {
+        auto &result = res.map_value;
+        DEBUGT("laddr={}, paddr {}~0x{:x} => {}, crc=0x{:x} done -- {}",
+               t, laddr, prev_addr, len, addr, checksum, result);
+        return update_mapping_iertr::make_ready_future();
+      },
+      update_mapping_iertr::pass_further{},
+      /* ENOENT in particular should be impossible */
+      crimson::ct_error::assert_all{
+        "Invalid error in BtreeLBAManager::update_mappings"
+      }
+    );
+  });
+}
+
 BtreeLBAManager::get_physical_extent_if_live_ret
 BtreeLBAManager::get_physical_extent_if_live(
   Transaction &t,

diff --git a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.h b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.h
index 39cd0d995dbcefcbd21df926f017d3729b104e96..d9e1247113b0a899a94a0ef8899bbf70c0706e8d 100644 (file)
--- a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.h
+++ b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.h
@@ -580,10 +580,11 @@ public:
     laddr_t laddr,
     extent_len_t prev_len,
     paddr_t prev_addr,
-    extent_len_t len,
-    paddr_t paddr,
-    uint32_t checksum,
-    LogicalChildNode*) final;
+    LogicalChildNode&) final;
+
+  update_mappings_ret update_mappings(
+    Transaction& t,
+    const std::list<LogicalChildNodeRef>& extents);
 
   get_physical_extent_if_live_ret get_physical_extent_if_live(
     Transaction &t,
@@ -594,7 +595,6 @@ public:
 private:
   Cache &cache;
 
-
   struct {
     uint64_t num_alloc_extents = 0;
     uint64_t num_alloc_extents_iter_nexts = 0;

diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc
index c979ec1b58cc5a299e621403a0d4f1e0aa43e8d2..dd77577315e9df632159bd1c3c49c99068e34138 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.cc
+++ b/src/crimson/os/seastore/transaction_manager.cc
@@ -526,10 +526,7 @@ TransactionManager::rewrite_logical_extent(
       extent->get_laddr(),
       extent->get_length(),
       extent->get_paddr(),
-      nextent->get_length(),
-      nextent->get_paddr(),
-      nextent->get_last_committed_crc(),
-      nextent.get()
+      *nextent
     ).discard_result();
   } else {
     assert(get_extent_category(extent->get_type()) == data_category_t::DATA);
@@ -570,15 +567,13 @@ TransactionManager::rewrite_logical_extent(
            * avoid this complication. */
           auto fut = base_iertr::now();
           if (first_extent) {
+            assert(off == 0);
             fut = lba_manager->update_mapping(
               t,
-              (extent->get_laddr() + off).checked_to_laddr(),
+              extent->get_laddr(),
               extent->get_length(),
               extent->get_paddr(),
-              nextent->get_length(),
-              nextent->get_paddr(),
-              nextent->get_last_committed_crc(),
-              nextent.get()
+              *nextent
             ).si_then([&refcount](auto c) {
               refcount = c;
             });