Merge pull request ceph#56627 from zhscn/wip-rewrite-snap-refcount

crimson/os/seastore: alloc mapping with refcount when rewriting logical extents

Reviewed-by: Yingxin Cheng <[email protected]>
Reviewed-by: Chunmei Liu <[email protected]>

Author: cyx1231st

src/crimson/common/interruptible_future.h

@@ -801,6 +801,9 @@ class [[nodiscard]] interruptible_future_detail<
     return safe_then_interruptible(std::forward<Args>(args)...);
   }
 
+  auto discard_result() noexcept {
+    return si_then([](auto &&) {});
+  }
 
   template<bool interruptible = true, typename ValueInterruptCondT, typename ErrorVisitorT,
 	   typename U = T, std::enable_if_t<!std::is_void_v<U> && interruptible, int> = 0>

src/crimson/os/seastore/async_cleaner.cc

@@ -1412,7 +1412,7 @@ SegmentCleaner::scan_extents_ret SegmentCleaner::scan_no_tail_segment(
       cursor,
       segment_header.segment_nonce,
       segments.get_segment_size(),
-      handler).discard_result();
+      handler);
   }).safe_then([this, segment_id, segment_header] {
     init_mark_segment_closed(
       segment_id,

src/crimson/os/seastore/journal/circular_bounded_journal.cc

@@ -188,7 +188,7 @@ Journal::replay_ret CircularBoundedJournal::replay_segment(
         cursor,
 	cjs.get_cbj_header().magic,
         std::numeric_limits<size_t>::max(),
-        dhandler).safe_then([](auto){}
+        dhandler
       ).handle_error(
         replay_ertr::pass_further{},
         crimson::ct_error::assert_all{

src/crimson/os/seastore/journal/segmented_journal.cc

@@ -219,7 +219,7 @@ SegmentedJournal::scan_last_segment(
       cursor,
       nonce,
       std::numeric_limits<std::size_t>::max(),
-      handler).discard_result();
+      handler);
   });
 }
 
@@ -312,7 +312,7 @@ SegmentedJournal::replay_segment(
 	cursor,
 	header.segment_nonce,
 	std::numeric_limits<size_t>::max(),
-	dhandler).safe_then([](auto){}
+	dhandler
       ).handle_error(
 	replay_ertr::pass_further{},
 	crimson::ct_error::assert_all{

src/crimson/os/seastore/lba_manager.cc

@@ -22,7 +22,7 @@ LBAManager::update_mappings(
       extent->get_paddr(),
       nullptr	// all the extents should have already been
 		// added to the fixed_kv_btree
-    );
+    ).discard_result();
   });
 }
 

src/crimson/os/seastore/lba_manager.h

@@ -87,7 +87,8 @@ class LBAManager {
     laddr_t hint,
     extent_len_t len,
     paddr_t addr,
-    LogicalCachedExtent &nextent) = 0;
+    LogicalCachedExtent &nextent,
+    extent_ref_count_t refcount = EXTENT_DEFAULT_REF_COUNT) = 0;
 
   virtual alloc_extent_ret clone_mapping(
     Transaction &t,
@@ -103,7 +104,7 @@ class LBAManager {
     extent_len_t len) = 0;
 
   struct ref_update_result_t {
-    unsigned refcount = 0;
+    extent_ref_count_t refcount = 0;
     pladdr_t addr;
     extent_len_t length = 0;
   };
@@ -187,7 +188,7 @@ class LBAManager {
    * update lba mapping for a delayed allocated extent
    */
   using update_mapping_iertr = base_iertr;
-  using update_mapping_ret = base_iertr::future<>;
+  using update_mapping_ret = base_iertr::future<extent_ref_count_t>;
   virtual update_mapping_ret update_mapping(
     Transaction& t,
     laddr_t laddr,
@@ -203,7 +204,7 @@ class LBAManager {
    * update lba mappings for delayed allocated extents
    */
   using update_mappings_iertr = update_mapping_iertr;
-  using update_mappings_ret = update_mapping_ret;
+  using update_mappings_ret = update_mappings_iertr::future<>;
   update_mappings_ret update_mappings(
     Transaction& t,
     const std::list<LogicalCachedExtentRef>& extents);

src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc

@@ -307,7 +307,8 @@ BtreeLBAManager::_alloc_extent(
   extent_len_t len,
   pladdr_t addr,
   paddr_t actual_addr,
-  LogicalCachedExtent* nextent)
+  LogicalCachedExtent* nextent,
+  extent_ref_count_t refcount)
 {
   struct state_t {
     laddr_t last_end;
@@ -319,7 +320,7 @@ BtreeLBAManager::_alloc_extent(
   };
 
   LOG_PREFIX(BtreeLBAManager::_alloc_extent);
-  TRACET("{}~{}, hint={}", t, addr, len, hint);
+  TRACET("{}~{}, hint={}, refcount={}", t, addr, len, hint, refcount);
 
   ceph_assert(actual_addr != P_ADDR_NULL ? addr.is_laddr() : addr.is_paddr());
   auto c = get_context(t);
@@ -330,7 +331,7 @@ BtreeLBAManager::_alloc_extent(
     c,
     hint,
     [this, FNAME, c, hint, len, addr, lookup_attempts,
-    &t, nextent](auto &btree, auto &state) {
+    &t, nextent, refcount](auto &btree, auto &state) {
       return LBABtree::iterate_repeat(
 	c,
 	btree.upper_bound_right(c, hint),
@@ -366,12 +367,12 @@ BtreeLBAManager::_alloc_extent(
 	      interruptible::ready_future_marker{},
 	      seastar::stop_iteration::no);
 	  }
-	}).si_then([FNAME, c, addr, len, hint, &btree, &state, nextent] {
+	}).si_then([FNAME, c, addr, len, hint, &btree, &state, nextent, refcount] {
 	  return btree.insert(
 	    c,
 	    *state.insert_iter,
 	    state.last_end,
-	    lba_map_val_t{len, pladdr_t(addr), 1, 0},
+	    lba_map_val_t{len, pladdr_t(addr), refcount, 0},
 	    nextent
 	  ).si_then([&state, FNAME, c, addr, len, hint, nextent](auto &&p) {
 	    auto [iter, inserted] = std::move(p);
@@ -553,10 +554,12 @@ BtreeLBAManager::update_mapping(
       return ret;
     },
     nextent
-  ).si_then([&t, laddr, prev_addr, addr, FNAME](auto p) {
-      auto &result = p.first;
+  ).si_then([&t, laddr, prev_addr, addr, FNAME](auto res) {
+      auto &result = res.map_value;
       DEBUGT("laddr={}, paddr {} => {} done -- {}",
              t, laddr, prev_addr, addr, result);
+      return update_mapping_iertr::make_ready_future<
+	extent_ref_count_t>(result.refcount);
     },
     update_mapping_iertr::pass_further{},
     /* ENOENT in particular should be impossible */
@@ -616,7 +619,7 @@ void BtreeLBAManager::register_metrics()
   );
 }
 
-BtreeLBAManager::ref_iertr::future<std::optional<std::pair<paddr_t, extent_len_t>>>
+BtreeLBAManager::_decref_intermediate_ret
 BtreeLBAManager::_decref_intermediate(
   Transaction &t,
   laddr_t addr,
@@ -650,15 +653,20 @@ BtreeLBAManager::_decref_intermediate(
 	if (!val.refcount) {
 	  return btree.remove(c, iter
 	  ).si_then([val] {
-	    return std::make_optional<
-	      std::pair<paddr_t, extent_len_t>>(
-		val.pladdr.get_paddr(), val.len);
+	    auto res = ref_update_result_t{
+	      val.refcount,
+	      val.pladdr.get_paddr(),
+	      val.len
+	    };
+	    return ref_iertr::make_ready_future<
+	      std::optional<ref_update_result_t>>(
+	        std::make_optional<ref_update_result_t>(res));
 	  });
 	} else {
 	  return btree.update(c, iter, val, nullptr
 	  ).si_then([](auto) {
-	    return seastar::make_ready_future<
-	      std::optional<std::pair<paddr_t, extent_len_t>>>(std::nullopt);
+	    return ref_iertr::make_ready_future<
+	      std::optional<ref_update_result_t>>(std::nullopt);
 	  });
 	}
       });
@@ -685,36 +693,36 @@ BtreeLBAManager::update_refcount(
       return out;
     },
     nullptr
-  ).si_then([&t, addr, delta, FNAME, this, cascade_remove](auto p) {
-    auto &result = p.first;
-    auto &mapping = p.second;
-    DEBUGT("laddr={}, delta={} done -- {}", t, addr, delta, result);
+  ).si_then([&t, addr, delta, FNAME, this, cascade_remove](auto res) {
+    auto &map_value = res.map_value;
+    auto &mapping = res.mapping;
+    DEBUGT("laddr={}, delta={} done -- {}", t, addr, delta, map_value);
     auto fut = ref_iertr::make_ready_future<
-      std::optional<std::pair<paddr_t, extent_len_t>>>();
-    if (!result.refcount && result.pladdr.is_laddr() && cascade_remove) {
+      std::optional<ref_update_result_t>>();
+    if (!map_value.refcount && map_value.pladdr.is_laddr() && cascade_remove) {
       fut = _decref_intermediate(
 	t,
-	result.pladdr.get_laddr(),
-	result.len
+	map_value.pladdr.get_laddr(),
+	map_value.len
       );
     }
-    return fut.si_then([result, mapping=std::move(mapping)]
-		       (auto removed) mutable {
-      if (result.pladdr.is_laddr()
-	  && removed) {
-	return std::make_pair(
-	    ref_update_result_t{
-	      result.refcount,
-	      removed->first,
-	      removed->second},
-	    std::move(mapping));
+    return fut.si_then([map_value, mapping=std::move(mapping)]
+		       (auto decref_intermediate_res) mutable {
+      if (map_value.pladdr.is_laddr()
+	  && decref_intermediate_res) {
+	return update_refcount_ret_bare_t{
+	  *decref_intermediate_res,
+	  std::move(mapping)
+	};
       } else {
-	return std::make_pair(
-	    ref_update_result_t{
-	      result.refcount,
-	      result.pladdr,
-	      result.len},
-	    std::move(mapping));
+	return update_refcount_ret_bare_t{
+	  ref_update_result_t{
+	    map_value.refcount,
+	    map_value.pladdr,
+	    map_value.len
+	  },
+	  std::move(mapping)
+	};
       }
     });
   });
@@ -728,7 +736,7 @@ BtreeLBAManager::_update_mapping(
   LogicalCachedExtent* nextent)
 {
   auto c = get_context(t);
-  return with_btree_ret<LBABtree, _update_mapping_ret_bare>(
+  return with_btree_ret<LBABtree, update_mapping_ret_bare_t>(
     cache,
     c,
     [f=std::move(f), c, addr, nextent](auto &btree) mutable {
@@ -748,8 +756,10 @@ BtreeLBAManager::_update_mapping(
 	    c,
 	    iter
 	  ).si_then([ret] {
-	    return std::make_pair(
-		std::move(ret), BtreeLBAMappingRef(nullptr));
+	    return update_mapping_ret_bare_t{
+	      std::move(ret),
+	      BtreeLBAMappingRef(nullptr)
+	    };
 	  });
 	} else {
 	  return btree.update(
@@ -758,8 +768,10 @@ BtreeLBAManager::_update_mapping(
 	    ret,
 	    nextent
 	  ).si_then([c, ret](auto iter) {
-	    return std::make_pair(
-		std::move(ret), iter.get_pin(c));
+	    return update_mapping_ret_bare_t{
+	      std::move(ret),
+	      iter.get_pin(c)
+	    };
 	  });
 	}
       });