crimson/os/seastore/BtreeLBAManager: cleanup alloc_mapping_info_t

zhscn · Matan-B · commit 64ebfc8ab44b · 2025-06-08T10:02:03.000+03:00
Signed-off-by: Zhang Song <zhangsong02@qianxin.com> (cherry picked from commit 20f2ed8)
diff --git a/src/crimson/os/seastore/lba_manager.h b/src/crimson/os/seastore/lba_manager.h
@@ -77,7 +77,7 @@ class LBAManager {
     Transaction &t,
     laddr_t hint,
     LogicalChildNode &nextent,
-    extent_ref_count_t refcount = EXTENT_DEFAULT_REF_COUNT) = 0;
+    extent_ref_count_t refcount) = 0;
 
   using alloc_extents_ret = alloc_extent_iertr::future<
     std::vector<LBAMappingRef>>;
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
@@ -279,8 +279,7 @@ BtreeLBAManager::alloc_extents_ret
 BtreeLBAManager::_alloc_extents(
   Transaction &t,
   laddr_t hint,
-  std::vector<alloc_mapping_info_t> &alloc_infos,
-  extent_ref_count_t refcount)
+  std::vector<alloc_mapping_info_t> &alloc_infos)
 {
   ceph_assert(hint != L_ADDR_NULL);
   extent_len_t total_len = 0;
@@ -291,16 +290,16 @@ BtreeLBAManager::_alloc_extents(
     assert((info.key == L_ADDR_NULL) == (laddr_null));
     if (!laddr_null) {
       assert(info.key >= last_end);
-      last_end = (info.key + info.len).checked_to_laddr();
+      last_end = (info.key + info.value.len).checked_to_laddr();
     }
   }
 #endif
   if (alloc_infos.front().key == L_ADDR_NULL) {
     for (auto &info : alloc_infos) {
-      total_len += info.len;
+      total_len += info.value.len;
     }
   } else {
-    auto end = alloc_infos.back().key + alloc_infos.back().len;
+    auto end = alloc_infos.back().key + alloc_infos.back().value.len;
     total_len = end.get_byte_distance<extent_len_t>(hint);
   }
 
@@ -314,21 +313,21 @@ BtreeLBAManager::_alloc_extents(
   };
 
   LOG_PREFIX(BtreeLBAManager::_alloc_extents);
-  TRACET("{}~{}, hint={}, num of extents: {}, refcount={}",
-    t, alloc_infos.front().val, total_len, hint, alloc_infos.size(), refcount);
+  TRACET("{}~{}, hint={}, num of extents: {}",
+    t, alloc_infos.front().value.pladdr, total_len, hint, alloc_infos.size());
 
   auto c = get_context(t);
   stats.num_alloc_extents += alloc_infos.size();
   auto lookup_attempts = stats.num_alloc_extents_iter_nexts;
   return seastar::do_with(
     std::vector<LBAMappingRef>(),
     [this, FNAME, &alloc_infos, hint, &t, total_len, c,
-    lookup_attempts, refcount](auto &rets) {
+    lookup_attempts](auto &rets) {
     return crimson::os::seastore::with_btree_state<LBABtree, state_t>(
       cache,
       c,
       hint,
-      [this, c, hint, total_len, addr=alloc_infos.front().val, &rets, refcount,
+      [this, c, hint, total_len, addr=alloc_infos.front().value.pladdr.get_paddr(), &rets,
       lookup_attempts, &t, &alloc_infos, FNAME](auto &btree, auto &state) {
       return LBABtree::iterate_repeat(
 	c,
@@ -367,7 +366,7 @@ BtreeLBAManager::_alloc_extents(
 	    seastar::stop_iteration::no);
 	}
       }).si_then([c, addr, hint, &btree, &state, &alloc_infos,
-		  total_len, &rets, refcount, FNAME] {
+		  total_len, &rets, FNAME] {
 	return trans_intr::do_for_each(
 	  alloc_infos,
 	  [c, addr, hint, &btree, &state, FNAME,
@@ -379,11 +378,7 @@ BtreeLBAManager::_alloc_extents(
 	    c,
 	    *state.insert_iter,
 	    state.last_end,
-	    lba_map_val_t{
-	      alloc_info.len,
-	      pladdr_t(alloc_info.val),
-	      refcount,
-	      alloc_info.checksum}
+	    alloc_info.value
 	  ).si_then([&state, c, addr, total_len, hint, FNAME,
 		    &alloc_info, &rets](auto &&p) {
 	    auto [iter, inserted] = std::move(p);
@@ -395,9 +390,9 @@ BtreeLBAManager::_alloc_extents(
 	    TRACET("{}~{}, hint={}, inserted at {}",
 		   c.trans, addr, total_len, hint, state.last_end);
 	    if (is_valid_child_ptr(alloc_info.extent)) {
-	      ceph_assert(alloc_info.val.is_paddr());
-	      assert(alloc_info.val == iter.get_val().pladdr);
-	      assert(alloc_info.len == iter.get_val().len);
+	      ceph_assert(alloc_info.value.pladdr.is_paddr());
+	      assert(alloc_info.value.pladdr == iter.get_val().pladdr);
+	      assert(alloc_info.value.len == iter.get_val().len);
 	      assert(alloc_info.extent->is_logical());
 	      if (alloc_info.extent->has_laddr()) {
 	        // see TM::remap_pin()
@@ -414,7 +409,7 @@ BtreeLBAManager::_alloc_extents(
 	    return iter.next(c).si_then([&state, &alloc_info](auto it) {
 	      state.insert_iter = it;
 	      if (alloc_info.key == L_ADDR_NULL) {
-		state.last_end = (state.last_end + alloc_info.len).checked_to_laddr();
+		state.last_end = (state.last_end + alloc_info.value.len).checked_to_laddr();
 	      }
 	    });
 	  });
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
@@ -69,44 +69,6 @@ class BtreeLBAManager : public LBAManager {
     Transaction &t,
     laddr_t offset) final;
 
-  struct alloc_mapping_info_t {
-    laddr_t key = L_ADDR_NULL; // once assigned, the allocation to
-			       // key must be exact and successful
-    extent_len_t len = 0;
-    pladdr_t val;
-    uint32_t checksum = 0;
-    LogicalChildNode* extent = nullptr;
-
-    static alloc_mapping_info_t create_zero(extent_len_t len) {
-      return {
-	L_ADDR_NULL,
-	len,
-	P_ADDR_ZERO,
-	0,
-	static_cast<LogicalChildNode*>(get_reserved_ptr<LBALeafNode, laddr_t>())};
-    }
-    static alloc_mapping_info_t create_indirect(
-      laddr_t laddr,
-      extent_len_t len,
-      laddr_t intermediate_key) {
-      return {
-	laddr,
-	len,
-	intermediate_key,
-	0,	// crc will only be used and checked with LBA direct mappings
-		// also see pin_to_extent(_by_type)
-	static_cast<LogicalChildNode*>(get_reserved_ptr<LBALeafNode, laddr_t>())};
-    }
-    static alloc_mapping_info_t create_direct(
-      laddr_t laddr,
-      extent_len_t len,
-      paddr_t paddr,
-      uint32_t checksum,
-      LogicalChildNode& extent) {
-      return {laddr, len, paddr, checksum, &extent};
-    }
-  };
-
   alloc_extent_ret reserve_region(
     Transaction &t,
     laddr_t hint,
@@ -120,8 +82,7 @@ class BtreeLBAManager : public LBAManager {
       return _alloc_extents(
 	t,
 	hint,
-	alloc_infos,
-	EXTENT_DEFAULT_REF_COUNT
+	alloc_infos
       ).si_then([](auto mappings) {
 	assert(mappings.size() == 1);
 	auto mapping = std::move(mappings.front());
@@ -169,7 +130,7 @@ class BtreeLBAManager : public LBAManager {
     Transaction &t,
     laddr_t hint,
     LogicalChildNode &ext,
-    extent_ref_count_t refcount = EXTENT_DEFAULT_REF_COUNT) final
+    extent_ref_count_t refcount) final
   {
     // The real checksum will be updated upon transaction commit
     assert(ext.get_last_committed_crc() == 0);
@@ -179,6 +140,7 @@ class BtreeLBAManager : public LBAManager {
 	L_ADDR_NULL,
 	ext.get_length(),
 	ext.get_paddr(),
+	refcount,
 	ext.get_last_committed_crc(),
 	ext)};
     return seastar::do_with(
@@ -187,8 +149,7 @@ class BtreeLBAManager : public LBAManager {
       return _alloc_extents(
 	t,
 	hint,
-	alloc_infos,
-	refcount
+	alloc_infos
       ).si_then([](auto mappings) {
 	assert(mappings.size() == 1);
 	auto mapping = std::move(mappings.front());
@@ -211,13 +172,14 @@ class BtreeLBAManager : public LBAManager {
 	  extent->has_laddr() ? extent->get_laddr() : L_ADDR_NULL,
 	  extent->get_length(),
 	  extent->get_paddr(),
+	  refcount,
 	  extent->get_last_committed_crc(),
 	  *extent));
     }
     return seastar::do_with(
       std::move(alloc_infos),
-      [this, &t, hint, refcount](auto &alloc_infos) {
-      return _alloc_extents(t, hint, alloc_infos, refcount);
+      [this, &t, hint](auto &alloc_infos) {
+      return _alloc_extents(t, hint, alloc_infos);
     });
   }
 
@@ -396,6 +358,49 @@ class BtreeLBAManager : public LBAManager {
     uint64_t num_alloc_extents_iter_nexts = 0;
   } stats;
 
+  struct alloc_mapping_info_t {
+    laddr_t key = L_ADDR_NULL; // once assigned, the allocation to
+			       // key must be exact and successful
+    lba_map_val_t value;
+    LogicalChildNode* extent = nullptr;
+
+    static alloc_mapping_info_t create_zero(extent_len_t len) {
+      return {
+	L_ADDR_NULL,
+	{
+	  len,
+	  pladdr_t(P_ADDR_ZERO),
+	  EXTENT_DEFAULT_REF_COUNT,
+	  0
+	},
+	static_cast<LogicalChildNode*>(get_reserved_ptr<LBALeafNode, laddr_t>())};
+    }
+    static alloc_mapping_info_t create_indirect(
+      laddr_t laddr,
+      extent_len_t len,
+      laddr_t intermediate_key) {
+      return {
+	laddr,
+	{
+	  len,
+	  pladdr_t(intermediate_key),
+	  EXTENT_DEFAULT_REF_COUNT,
+	  0	// crc will only be used and checked with LBA direct mappings
+		// also see pin_to_extent(_by_type)
+	},
+	static_cast<LogicalChildNode*>(get_reserved_ptr<LBALeafNode, laddr_t>())};
+    }
+    static alloc_mapping_info_t create_direct(
+      laddr_t laddr,
+      extent_len_t len,
+      paddr_t paddr,
+      extent_ref_count_t refcount,
+      checksum_t checksum,
+      LogicalChildNode& extent) {
+      return {laddr, {len, pladdr_t(paddr), refcount, checksum}, &extent};
+    }
+  };
+
   op_context_t get_context(Transaction &t) {
     return op_context_t{cache, t};
   }
@@ -445,8 +450,7 @@ class BtreeLBAManager : public LBAManager {
   alloc_extents_ret _alloc_extents(
     Transaction &t,
     laddr_t hint,
-    std::vector<alloc_mapping_info_t> &alloc_infos,
-    extent_ref_count_t refcount);
+    std::vector<alloc_mapping_info_t> &alloc_infos);
 
   ref_ret _incref_extent(
     Transaction &t,
@@ -466,7 +470,8 @@ class BtreeLBAManager : public LBAManager {
   {
 #ifndef NDEBUG
     for (auto &alloc_info : alloc_infos) {
-      assert(alloc_info.val.get_laddr() != L_ADDR_NULL);
+      assert(alloc_info.value.pladdr.get_laddr() != L_ADDR_NULL);
+      assert(alloc_info.value.refcount == EXTENT_DEFAULT_REF_COUNT);
     }
 #endif
     return seastar::do_with(
@@ -475,8 +480,7 @@ class BtreeLBAManager : public LBAManager {
       return _alloc_extents(
 	t,
 	laddr,
-	alloc_infos,
-	EXTENT_DEFAULT_REF_COUNT
+	alloc_infos
       ).si_then([&alloc_infos](auto mappings) {
 	assert(alloc_infos.size() == mappings.size());
 	std::vector<BtreeLBAMappingRef> rets;
@@ -487,8 +491,8 @@ class BtreeLBAManager : public LBAManager {
 	  [[maybe_unused]] auto &alloc_info = *ait;
 	  assert(mapping->get_key() == alloc_info.key);
 	  assert(mapping->get_raw_val().get_laddr() ==
-	    alloc_info.val.get_laddr());
-	  assert(mapping->get_length() == alloc_info.len);
+	    alloc_info.value.pladdr.get_laddr());
+	  assert(mapping->get_length() == alloc_info.value.len);
 	  rets.emplace_back(mapping);
 	}
 	return rets;
diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h
@@ -402,7 +402,8 @@ class TransactionManager : public ExtentCallbackInterface {
     return lba_manager->alloc_extent(
       t,
       laddr_hint,
-      *ext
+      *ext,
+      EXTENT_DEFAULT_REF_COUNT
     ).si_then([ext=std::move(ext), &t, FNAME](auto &&) mutable {
       SUBDEBUGT(seastore_tm, "allocated {}", t, *ext);
       return alloc_extent_iertr::make_ready_future<TCachedExtentRef<T>>(
