crimson/os/seastore/lba: set extent type for ZERO lba mappings

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

diff --git a/src/crimson/os/seastore/backref/btree_backref_manager.cc b/src/crimson/os/seastore/backref/btree_backref_manager.cc
index 9645fe1773ed217f403959757f4f5e660b256cdc..0e1d17e8e5713de503a2204844e3308415a69651 100644 (file)
--- a/src/crimson/os/seastore/backref/btree_backref_manager.cc
+++ b/src/crimson/os/seastore/backref/btree_backref_manager.cc
@@ -297,7 +297,8 @@ BtreeBackrefManager::merge_cached_backrefs(
               DEBUGT("remove mapping: {}", t, backref_entry.paddr);
               return remove_mapping(
                 t,
-                backref_entry.paddr
+                backref_entry.paddr,
+                backref_entry.type
               ).si_then([](auto&&) {
                 return seastar::now();
               }).handle_error_interruptible(
@@ -501,16 +502,17 @@ BtreeBackrefManager::rewrite_extent(
 BtreeBackrefManager::remove_mapping_ret
 BtreeBackrefManager::remove_mapping(
   Transaction &t,
-  paddr_t addr)
+  paddr_t addr,
+  extent_types_t type)
 {
   auto c = get_context(t);
   return with_btree<BackrefBtree>(
     cache,
     c,
-    [c, addr](auto &btree) mutable {
+    [c, addr, type](auto &btree) mutable {
       return btree.lower_bound(
        c, addr
-      ).si_then([&btree, c, addr](auto iter)
+      ).si_then([&btree, c, addr, type](auto iter)
                -> remove_mapping_ret {
        if (iter.is_end() || iter.get_key() != addr) {
          LOG_PREFIX(BtreeBackrefManager::remove_mapping);
@@ -520,10 +522,12 @@ BtreeBackrefManager::remove_mapping(
            remove_mapping_result_t>(remove_mapping_result_t());
        }
 
+       auto val = iter.get_val();
+        ceph_assert(type == val.type);
        auto ret = remove_mapping_result_t{
          iter.get_key(),
-         iter.get_val().len,
-         iter.get_val().laddr};
+         val.len,
+         val.laddr};
        return btree.remove(
          c,
          iter

diff --git a/src/crimson/os/seastore/backref/btree_backref_manager.h b/src/crimson/os/seastore/backref/btree_backref_manager.h
index 59392f2c746192f7c9b143ce337d9c40518dd08d..b8b69af77d96d52a04c82a70aa2f75aa6121c4f0 100644 (file)
--- a/src/crimson/os/seastore/backref/btree_backref_manager.h
+++ b/src/crimson/os/seastore/backref/btree_backref_manager.h
@@ -48,7 +48,8 @@ public:
 
   remove_mapping_ret remove_mapping(
     Transaction &t,
-    paddr_t offset) final;
+    paddr_t offset,
+    extent_types_t type) final;
 
   scan_mapped_space_ret scan_mapped_space(
     Transaction &t,

diff --git a/src/crimson/os/seastore/backref_manager.h b/src/crimson/os/seastore/backref_manager.h
index 9ffec10f78c89194f8a487495fc4239db1ce7b72..6b6f1676189629ee8207b8ebaee15d0dda764f55 100644 (file)
--- a/src/crimson/os/seastore/backref_manager.h
+++ b/src/crimson/os/seastore/backref_manager.h
@@ -123,7 +123,8 @@ public:
   using remove_mapping_ret = remove_mapping_iertr::future<remove_mapping_result_t>;
   virtual remove_mapping_ret remove_mapping(
     Transaction &t,
-    paddr_t offset) = 0;
+    paddr_t offset,
+    extent_types_t type) = 0;
 
   /**
    * scan all extents in both tree and cache,

diff --git a/src/crimson/os/seastore/btree/btree_types.cc b/src/crimson/os/seastore/btree/btree_types.cc
index 410af029b370fcf662b0d7b3a271e678a8b27184..bc5d0cafab75a7448858457f27886f50c013c627 100644 (file)
--- a/src/crimson/os/seastore/btree/btree_types.cc
+++ b/src/crimson/os/seastore/btree/btree_types.cc
@@ -18,6 +18,7 @@ std::ostream& operator<<(std::ostream& out, const lba_map_val_t& v)
              << ", type=" << (extent_types_t)v.type
              << ", checksum=0x" << v.checksum
              << ", refcount=" << std::dec << v.refcount
+             << ", type=" << v.type
              << ")";
 }
 

diff --git a/src/crimson/os/seastore/btree/btree_types.h b/src/crimson/os/seastore/btree/btree_types.h
index 32ee7701fece176a07e7fb98eb4c37226d45e8d1..ade1e759899b06626a129531f0a31438a060947d 100644 (file)
--- a/src/crimson/os/seastore/btree/btree_types.h
+++ b/src/crimson/os/seastore/btree/btree_types.h
@@ -138,7 +138,7 @@ struct __attribute__((packed)) lba_map_val_le_t {
   pladdr_le_t pladdr;
   extent_ref_count_le_t refcount{0};
   checksum_le_t checksum{0};
-  extent_types_le_t type = 0;
+  extent_types_le_t type{EXTENT_TYPES_MAX};
 
   lba_map_val_le_t() = default;
   lba_map_val_le_t(const lba_map_val_le_t &) = default;
@@ -147,7 +147,7 @@ struct __attribute__((packed)) lba_map_val_le_t {
       pladdr(pladdr_le_t(val.pladdr)),
       refcount(val.refcount),
       checksum(val.checksum),
-      type((extent_types_le_t)val.type) {}
+      type(static_cast<extent_types_le_t>(val.type)) {}
 
   operator lba_map_val_t() const {
     return lba_map_val_t{
@@ -155,7 +155,7 @@ struct __attribute__((packed)) lba_map_val_le_t {
       pladdr,
       refcount,
       checksum,
-      (extent_types_t)type};
+      static_cast<extent_types_t>(type)};
   }
 };
 

diff --git a/src/crimson/os/seastore/btree/fixed_kv_btree.h b/src/crimson/os/seastore/btree/fixed_kv_btree.h
index 3aa841f3b09035a517a20697af094e425d1f32af..8d70ad3e866f5a0b6239dee45b219180c2d70fbb 100644 (file)
--- a/src/crimson/os/seastore/btree/fixed_kv_btree.h
+++ b/src/crimson/os/seastore/btree/fixed_kv_btree.h
@@ -890,6 +890,10 @@ public:
       c.trans,
       laddr,
       iter.is_end() ? min_max_t<node_key_t>::max : iter.get_key());
+    if constexpr (std::is_same_v<node_key_t, laddr_t>) {
+      // avoid unexpect default extent type for lba btree
+      assert(val.type != extent_types_t::ROOT);
+    }
     return seastar::do_with(
       iter,
       [this, c, laddr, val, child](auto &ret) {

diff --git a/src/crimson/os/seastore/lba/btree_lba_manager.cc b/src/crimson/os/seastore/lba/btree_lba_manager.cc
index 5226c7b95542c44bbdb5b47ce9f422146e2c54e5..d2c35cfeb4016c72038234e42e1b4eb3f3f36513 100644 (file)
--- a/src/crimson/os/seastore/lba/btree_lba_manager.cc
+++ b/src/crimson/os/seastore/lba/btree_lba_manager.cc
@@ -242,7 +242,8 @@ BtreeLBAManager::reserve_region(
   Transaction &t,
   LBACursorRef cursor,
   laddr_t addr,
-  extent_len_t len)
+  extent_len_t len,
+  extent_types_t type)
 {
   LOG_PREFIX(BtreeLBAManager::reserve_region);
   DEBUGT("{} {}~{}", t, *cursor, addr, len);
@@ -255,7 +256,7 @@ BtreeLBAManager::reserve_region(
     pladdr_t{P_ADDR_ZERO},
     EXTENT_DEFAULT_REF_COUNT,
     0,
-    extent_types_t::NONE};
+    type};
   auto p = co_await btree.insert(
     c, iter, addr, val,
     get_reserved_ptr<LBALeafNode, laddr_t>()
@@ -1054,6 +1055,7 @@ BtreeLBAManager::remap_mappings(
   assert(orig_indirect ||
         (orig_val.pladdr.is_paddr() &&
          orig_val.pladdr.get_paddr().is_absolute()));
+  auto type = cursor->get_extent_type();
   cursor = co_await update_mapping_refcount(
     c.trans, cursor, -1);
   iter = btree.make_partial_iter(c, *cursor);
@@ -1075,6 +1077,7 @@ BtreeLBAManager::remap_mappings(
     val.refcount = EXTENT_DEFAULT_REF_COUNT;
     // Checksum will be updated when the committing the transaction
     val.checksum = CRC_NULL;
+    val.type = type;
     // committing the transaction
     auto p = co_await btree.insert(
       c, iter, new_key, std::move(val),

diff --git a/src/crimson/os/seastore/lba/btree_lba_manager.h b/src/crimson/os/seastore/lba/btree_lba_manager.h
index 4dc9fd8378cb58ea29e6c69d8c9ce69349863c1b..04628f7ee26f919b78bfe0594d664a329f9c1517 100644 (file)
--- a/src/crimson/os/seastore/lba/btree_lba_manager.h
+++ b/src/crimson/os/seastore/lba/btree_lba_manager.h
@@ -79,15 +79,17 @@ public:
     Transaction &t,
     LBACursorRef pos,
     laddr_t laddr,
-    extent_len_t len) final;
+    extent_len_t len,
+    extent_types_t type) final;
 
   alloc_extent_ret reserve_region(
     Transaction &t,
     laddr_hint_t hint,
-    extent_len_t len) final
+    extent_len_t len,
+    extent_types_t type) final
   {
     std::vector<alloc_mapping_info_t> alloc_infos = {
-      alloc_mapping_info_t::create_zero(len)};
+      alloc_mapping_info_t::create_zero(len, type)};
     auto cursors = co_await alloc_contiguous_mappings(
       t, hint, alloc_infos);
     assert(cursors.size() == 1);
@@ -271,7 +273,9 @@ private:
       return value.pladdr.is_laddr();
     }
 
-    static alloc_mapping_info_t create_zero(extent_len_t len) {
+    static alloc_mapping_info_t create_zero(
+      extent_len_t len,
+      extent_types_t type) {
       return {
        L_ADDR_NULL,
        {
@@ -279,7 +283,7 @@ private:
          pladdr_t(P_ADDR_ZERO),
          EXTENT_DEFAULT_REF_COUNT,
          0,
-          extent_types_t::NONE
+         type
        }};
     }
     static alloc_mapping_info_t create_indirect(
@@ -294,7 +298,8 @@ private:
          EXTENT_DEFAULT_REF_COUNT,
          0,    // crc will only be used and checked with LBA direct mappings
                // also see pin_to_extent(_by_type)
-          extent_types_t::NONE
+         // only OBJECT_DATA_BLOCK support indirect mapping for now
+         extent_types_t::OBJECT_DATA_BLOCK
        }};
     }
     static alloc_mapping_info_t create_direct(
@@ -305,13 +310,10 @@ private:
       checksum_t checksum,
       LogicalChildNode& extent) {
       return {
-        laddr,
-        {len,
-         pladdr_t(paddr),
-         refcount,
-         checksum,
-         extent.get_type()},
-        &extent};
+       laddr,
+       {len, pladdr_t(paddr), refcount, checksum, extent.get_type()},
+       &extent
+      };
     }
   };
 

diff --git a/src/crimson/os/seastore/lba/lba_btree_node.h b/src/crimson/os/seastore/lba/lba_btree_node.h
index e242061d3f45c39e99b68c3726710152479f59f2..9aaf30bda84f99f1edefa34c01198a2452711645 100644 (file)
--- a/src/crimson/os/seastore/lba/lba_btree_node.h
+++ b/src/crimson/os/seastore/lba/lba_btree_node.h
@@ -90,14 +90,14 @@ using LBAInternalNodeRef = LBAInternalNode::Ref;
  *   checksum   : ceph_le32[1]                4B
  *   size       : ceph_le32[1]                4B
  *   meta       : lba_node_meta_le_t[1]       36B
- *   keys       : laddr_le_t[CAPACITY]        (109*16)B
- *   values     : lba_map_val_le_t[CAPACITY]  (109*21)B
+ *   keys       : laddr_le_t[CAPACITY]        (106*16)B
+ *   values     : lba_map_val_le_t[CAPACITY]  (106*21)B
  *                                            = 4077B
  *
  * TODO: update FixedKVNodeLayout to handle the above calculation
  * TODO: the above alignment probably isn't portable without further work
  */
-constexpr size_t LEAF_NODE_CAPACITY = 109;
+constexpr size_t LEAF_NODE_CAPACITY = 106;
 
 struct LBALeafNode
   : FixedKVLeafNode<
@@ -445,6 +445,7 @@ struct LBACursor : BtreeCursor<laddr_t, lba::lba_map_val_t, LBALeafNode> {
   extent_types_t get_extent_type() const {
     assert(is_viewable());
     assert(!is_end());
+    assert(iter.get_val().type != extent_types_t::NONE);
     return iter.get_val().type;
   }
 

diff --git a/src/crimson/os/seastore/lba_manager.h b/src/crimson/os/seastore/lba_manager.h
index 1b066b575ed5133ed3bb9cb0a5477e20a5408c29..a0e4dcec518a2c463492662d1cdf4d009b7d938a 100644 (file)
--- a/src/crimson/os/seastore/lba_manager.h
+++ b/src/crimson/os/seastore/lba_manager.h
@@ -117,7 +117,8 @@ public:
   virtual alloc_extent_ret reserve_region(
     Transaction &t,
     laddr_hint_t hint,
-    extent_len_t len) = 0;
+    extent_len_t len,
+    extent_types_t type) = 0;
 
   /*
    * Inserts a zero mapping at the position "pos" with
@@ -127,7 +128,8 @@ public:
     Transaction &t,
     LBACursorRef cursor,
     laddr_t hint,
-    extent_len_t len) = 0;
+    extent_len_t len,
+    extent_types_t type) = 0;
 
   using ref_iertr = base_iertr::extend<
     crimson::ct_error::enoent>;

diff --git a/src/crimson/os/seastore/object_data_handler.cc b/src/crimson/os/seastore/object_data_handler.cc
index 7cca29f93be1fafade8f145f098224e976f2208d..1167c8f6a1d9b61d54a4ad35b3a08b2806f30e09 100644 (file)
--- a/src/crimson/os/seastore/object_data_handler.cc
+++ b/src/crimson/os/seastore/object_data_handler.cc
@@ -124,7 +124,8 @@ ObjectDataHandler::prepare_data_reservation(
     return ctx.tm.reserve_region(
       ctx.t,
       ctx.onode.get_data_hint(),
-      max_object_size
+      max_object_size,
+      extent_types_t::OBJECT_DATA_BLOCK
     ).si_then([max_object_size=max_object_size, &object_data](auto pin) {
       ceph_assert(pin.get_length() == max_object_size);
       object_data.update_reserved(
@@ -303,7 +304,9 @@ ObjectDataHandler::write_ret do_zero(
     ).checked_to_laddr();
   auto len = end.get_byte_distance<extent_len_t>(laddr);
   if (len != 0) {
-    zero_pos = co_await ctx.tm.reserve_region(ctx.t, std::move(zero_pos), laddr, len
+    zero_pos = co_await ctx.tm.reserve_region(
+      ctx.t, std::move(zero_pos), laddr, len,
+      extent_types_t::OBJECT_DATA_BLOCK
     ).handle_error_interruptible(
       crimson::ct_error::enospc::assert_failure{"unexpected enospc"},
       TransactionManager::get_pin_iertr::pass_further{}

diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h
index 5a834ea4b0dcb3d4d900bc5c7ccbdfbd6657bd76..945f9063e1fa83c420f7a3b7648e057397dcbf19 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.h
+++ b/src/crimson/os/seastore/transaction_manager.h
@@ -641,13 +641,15 @@ public:
   reserve_extent_ret reserve_region(
     Transaction &t,
     laddr_hint_t hint,
-    extent_len_t len) {
+    extent_len_t len,
+    extent_types_t type) {
     LOG_PREFIX(TransactionManager::reserve_region);
-    SUBDEBUGT(seastore_tm, "hint {}~0x{:x} ...", t, hint, len);
+    SUBDEBUGT(seastore_tm, "hint {}~0x{:x} {} ...", t, hint, len, type);
     auto pin = co_await lba_manager->reserve_region(
       t,
       hint,
-      len
+      len,
+      type
     );
     SUBDEBUGT(seastore_tm, "reserved {}", t, *pin);
     co_return LBAMapping::create_direct(std::move(pin));
@@ -657,15 +659,17 @@ public:
     Transaction &t,
     LBAMapping pos,
     laddr_t hint,
-    extent_len_t len) {
+    extent_len_t len,
+    extent_types_t type) {
     LOG_PREFIX(TransactionManager::reserve_region);
-    SUBDEBUGT(seastore_tm, "hint {}~0x{:x} ...", t, hint, len);
+    SUBDEBUGT(seastore_tm, "hint {}~0x{:x} {} ...", t, hint, len, type);
     pos = co_await pos.refresh();
     auto pin = co_await lba_manager->reserve_region(
       t,
       pos.get_effective_cursor_ref(),
       hint,
-      len
+      len,
+      type
     );
     co_return LBAMapping::create_direct(std::move(pin));
   }
@@ -760,7 +764,8 @@ public:
          t,
          std::move(pos),
          (dst_base + cloned_to).checked_to_laddr(),
-         clone_len
+         clone_len,
+         mapping.get_extent_type()
        ).handle_error_interruptible(
          clone_iertr::pass_further{},
          crimson::ct_error::assert_all{"unexpected error"}
@@ -1060,6 +1065,9 @@ public:
     if (!mapping.is_indirect() && mapping.is_zero_reserved()) {
       SUBDEBUGT(seastore_tm, "zero reserved, mapping {}, {} remaps",
                t, mapping, remaps);
+      //TODO: drop this assert
+      assert(mapping.get_extent_type() == extent_types_t::OBJECT_DATA_BLOCK);
+      auto type = mapping.get_extent_type();
       std::vector<LBAMapping> ret;
       auto orig_laddr = mapping.get_key();
       auto pos = co_await remove(
@@ -1074,7 +1082,8 @@ public:
          t,
          std::move(pos),
          laddr,
-         remap.len
+         remap.len,
+          type
        ).handle_error_interruptible(
          remap_mappings_iertr::pass_further{},
          crimson::ct_error::assert_all{"unexpected error"}

diff --git a/src/test/crimson/seastore/test_transaction_manager.cc b/src/test/crimson/seastore/test_transaction_manager.cc
index 040beb967bf51ad23ff27bef38319989728e26bc..8461d151f8910ac48e00a7cf3d73e4e3465acf3c 100644 (file)
--- a/src/test/crimson/seastore/test_transaction_manager.cc
+++ b/src/test/crimson/seastore/test_transaction_manager.cc
@@ -1892,10 +1892,10 @@ TEST_P(tm_random_block_device_test_t, scatter_allocation)
     laddr_t ADDR = get_laddr_hint(0xFF * 4096);
     epm->prefill_fragmented_devices();
     auto t = create_transaction();
-    for (int i = 0; i < 1975; i++) {
+    for (int i = 0; i < 1974; i++) {
       auto extents = alloc_extents(t, (ADDR + i * 16384).checked_to_laddr(), 16384, 'a');
     }
-    alloc_extents_deemed_fail(t, (ADDR + 1975 * 16384).checked_to_laddr(), 16384, 'a');
+    alloc_extents_deemed_fail(t, (ADDR + 1974 * 16384).checked_to_laddr(), 16384, 'a');
     check_mappings(t);
     check();
     submit_transaction(std::move(t));