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 f50b56b1ffe9f53c377ecf1a4348f01c8fe137f3..70c86d5ea3e89ca62d5c0dfadc74209aea5450d8 100644 (file)
--- a/src/crimson/os/seastore/backref/btree_backref_manager.cc
+++ b/src/crimson/os/seastore/backref/btree_backref_manager.cc
@@ -296,7 +296,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(
@@ -500,16 +501,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);
@@ -519,10 +521,14 @@ BtreeBackrefManager::remove_mapping(
            remove_mapping_result_t>(remove_mapping_result_t());
        }
 
+       auto val = iter.get_val();
+       if (!is_retired_placeholder_type(type)) {
+         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 bf4940d59e722e6fc853100959c9fd1dc9d64826..f97b78ef968696142a5452225059e0578c0f24eb 100644 (file)
--- a/src/crimson/os/seastore/btree/btree_types.cc
+++ b/src/crimson/os/seastore/btree/btree_types.cc
@@ -93,6 +93,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 59f70576b42c7d66bae9f0c30429d07af9d603d8..01f1473e2529e2a63c4a09f267dbe80f02d5ff20 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)};
   }
 };
 
@@ -260,6 +260,11 @@ struct BtreeCursor
     assert(!is_end());
     return val->len;
   }
+
+  extent_types_t get_extent_type() const {
+    assert(val->type != extent_types_t::NONE);
+    return val->type;
+  }
 };
 
 struct LBACursor : BtreeCursor<laddr_t, lba::lba_map_val_t> {
@@ -333,7 +338,7 @@ std::ostream &operator<<(
   if (cursor.is_end()) {
     return out << "END)";
   }
-  return out << "," << cursor.key
+  return out << cursor.key
             << "~" << *cursor.val
             << ")";
 }

diff --git a/src/crimson/os/seastore/btree/fixed_kv_btree.h b/src/crimson/os/seastore/btree/fixed_kv_btree.h
index 15849be4cd08d06f2891ace0c98d1432e78f0ef9..256f58397ad268a37dfc7188eb454e8f4032f812 100644 (file)
--- a/src/crimson/os/seastore/btree/fixed_kv_btree.h
+++ b/src/crimson/os/seastore/btree/fixed_kv_btree.h
@@ -887,6 +887,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](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 725f7afab556f6cd779d4196148b4b70ef11380e..4d7c22eaf5813cd7e477691fdcd246f7728a8146 100644 (file)
--- a/src/crimson/os/seastore/lba/btree_lba_manager.cc
+++ b/src/crimson/os/seastore/lba/btree_lba_manager.cc
@@ -324,7 +324,8 @@ BtreeLBAManager::reserve_region(
   Transaction &t,
   LBAMapping pos,
   laddr_t addr,
-  extent_len_t len)
+  extent_len_t len,
+  extent_types_t type)
 {
   LOG_PREFIX(BtreeLBAManager::reserve_region);
   DEBUGT("{} {}~{}", t, pos, addr, len);
@@ -333,15 +334,10 @@ BtreeLBAManager::reserve_region(
   return with_btree<LBABtree>(
     cache,
     c,
-    [pos=std::move(pos), c, addr, len](auto &btree) mutable {
+    [pos=std::move(pos), c, addr, len, type](auto &btree) mutable {
     auto &cursor = pos.get_effective_cursor();
     auto iter = btree.make_partial_iter(c, cursor);
-    lba_map_val_t val{
-      len,
-      pladdr_t{P_ADDR_ZERO},
-      EXTENT_DEFAULT_REF_COUNT,
-      0,
-      extent_types_t::NONE};
+    lba_map_val_t val{len, pladdr_t{P_ADDR_ZERO}, EXTENT_DEFAULT_REF_COUNT, 0, type};
     return btree.insert(c, iter, addr, val
     ).si_then([c](auto p) {
       auto &[iter, inserted] = p;
@@ -498,11 +494,8 @@ BtreeLBAManager::clone_mapping(
            btree.make_partial_iter(c, cursor),
            state.laddr,
             lba_map_val_t{
-              state.len,
-              pladdr_t{inter_key.get_local_clone_id()},
-              EXTENT_DEFAULT_REF_COUNT,
-              0,
-              extent_types_t::NONE});
+             state.len, pladdr_t{inter_key.get_local_clone_id()},
+             EXTENT_DEFAULT_REF_COUNT, 0, state.mapping.get_extent_type()});
        }).si_then([c, &state](auto p) {
          auto &[iter, inserted] = p;
          auto &leaf_node = *iter.get_leaf_node();
@@ -1382,6 +1375,7 @@ BtreeLBAManager::remap_mappings(
          auto old_key = mapping.get_key();
          auto new_key = (old_key + remap.offset).checked_to_laddr();
          val.len = remap.len;
+         val.type = mapping.get_extent_type();
          if (pladdr.is_laddr()) {
            val.pladdr = pladdr;
          } else {

diff --git a/src/crimson/os/seastore/lba/btree_lba_manager.h b/src/crimson/os/seastore/lba/btree_lba_manager.h
index 6af3e2467c1a9b6529456d6a3d1868b71c81b7b2..e502604309649ecf99c6b1f4209f0535fc1a2859 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,
     LBAMapping 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)};
     return seastar::do_with(
       std::move(alloc_infos),
       [&t, hint, this](auto &alloc_infos) {
@@ -383,7 +385,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,
        {
@@ -391,7 +395,7 @@ private:
          pladdr_t(P_ADDR_ZERO),
          EXTENT_DEFAULT_REF_COUNT,
          0,
-          extent_types_t::NONE
+         type
        }};
     }
     static alloc_mapping_info_t create_indirect(
@@ -406,7 +410,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(
@@ -417,13 +422,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 e1381f62baeee7320d8edbd9a33e14da324e5a36..e27a97240bd696d6cf18d87b60fc1d66d53960a6 100644 (file)
--- a/src/crimson/os/seastore/lba/lba_btree_node.h
+++ b/src/crimson/os/seastore/lba/lba_btree_node.h
@@ -89,14 +89,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<

diff --git a/src/crimson/os/seastore/lba_manager.h b/src/crimson/os/seastore/lba_manager.h
index 7a1a28a82c19da7e518db155e015d2283c941a48..450004debf9a08994e4187c12078ac8aa2292632 100644 (file)
--- a/src/crimson/os/seastore/lba_manager.h
+++ b/src/crimson/os/seastore/lba_manager.h
@@ -134,7 +134,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
@@ -144,7 +145,8 @@ public:
     Transaction &t,
     LBAMapping pos,
     laddr_t hint,
-    extent_len_t len) = 0;
+    extent_len_t len,
+    extent_types_t type) = 0;
 
   struct mapping_update_result_t {
     laddr_t key;

diff --git a/src/crimson/os/seastore/lba_mapping.h b/src/crimson/os/seastore/lba_mapping.h
index 92706ab90373369e86ca018779d7a68d13ebef7d..6f862350e56d775442a8a888c59284d3611aa319 100644 (file)
--- a/src/crimson/os/seastore/lba_mapping.h
+++ b/src/crimson/os/seastore/lba_mapping.h
@@ -177,6 +177,21 @@ public:
       extent_len_t>(get_intermediate_key());
   }
 
+  extent_types_t get_extent_type() const {
+    if (direct_cursor && indirect_cursor) {
+      assert(direct_cursor->get_extent_type()
+            == indirect_cursor->get_extent_type());
+    }
+    if (direct_cursor) {
+      return direct_cursor->get_extent_type();
+    } else if (indirect_cursor) {
+      return indirect_cursor->get_extent_type();
+    } else {
+      ceph_abort("invalid LBAMapping");
+      return extent_types_t::NONE;
+    }
+  }
+
   get_child_ret_t<lba::LBALeafNode, LogicalChildNode>
   get_logical_extent(Transaction &t) const;
 

diff --git a/src/crimson/os/seastore/object_data_handler.cc b/src/crimson/os/seastore/object_data_handler.cc
index c228265fd688e9087da95ce38f4b8e149d975698..87a83b3caa4c7410bc9ff7b9c0f17b61c6226b29 100644 (file)
--- a/src/crimson/os/seastore/object_data_handler.cc
+++ b/src/crimson/os/seastore/object_data_handler.cc
@@ -109,7 +109,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(
@@ -287,7 +288,9 @@ ObjectDataHandler::write_ret do_zero(
        (data.tailbl ? ctx.tm.get_block_size() : 0)
       ).checked_to_laddr();
     auto len = end.get_byte_distance<extent_len_t>(laddr);
-    return ctx.tm.reserve_region(ctx.t, std::move(zero_pos), laddr, len);
+    return ctx.tm.reserve_region(
+      ctx.t, std::move(zero_pos), laddr, len,
+      extent_types_t::OBJECT_DATA_BLOCK);
   }).si_then([](auto zero_pos) {
     return std::make_optional<LBAMapping>(std::move(zero_pos));
   }).handle_error_interruptible(

diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h
index 0d07db324b2bd9c049f4c96499b72961a9d3ebb1..f2e8b8752a4bb2225f0fd7cc53f0643b21a548ca 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.h
+++ b/src/crimson/os/seastore/transaction_manager.h
@@ -538,13 +538,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);
     return lba_manager->reserve_region(
       t,
       hint,
-      len
+      len,
+      type
     ).si_then([FNAME, &t](auto pin) {
       SUBDEBUGT(seastore_tm, "reserved {}", t, pin);
       return pin;
@@ -555,16 +557,18 @@ 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);
     return pos.refresh(
-    ).si_then([FNAME, this, &t, hint, len](auto pos) {
+    ).si_then([FNAME, this, &t, hint, len, type](auto pos) {
       return lba_manager->reserve_region(
        t,
        std::move(pos),
        hint,
-       len
+       len,
+       type
       ).si_then([FNAME, &t](auto pin) {
        SUBDEBUGT(seastore_tm, "reserved {}", t, pin);
        return pin;
@@ -659,7 +663,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"}
@@ -979,7 +984,8 @@ public:
                t,
                std::move(pos),
                laddr,
-               remap.len
+               remap.len,
+               extent_types_t::OBJECT_DATA_BLOCK
              ).si_then([&mappings](auto new_mapping) {
                mappings.emplace_back(new_mapping);
                return new_mapping.next();

diff --git a/src/test/crimson/seastore/test_transaction_manager.cc b/src/test/crimson/seastore/test_transaction_manager.cc
index 25b053607bf308af1e9f09541d2b7c6bd772b933..6740066f77b2c1274da9b5f6a03ce11f295e1e47 100644 (file)
--- a/src/test/crimson/seastore/test_transaction_manager.cc
+++ b/src/test/crimson/seastore/test_transaction_manager.cc
@@ -1863,10 +1863,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));