crimson/os/seastore/cache: don't index already removed backref entries in Cache:...

This is needed by extent splitting, and can avoid inserting/removing
unnecessary backrefs

Signed-off-by: Xuehan Xu <xxhdx1985126@gmail.com>

diff --git a/src/crimson/os/seastore/backref/btree_backref_manager.cc b/src/crimson/os/seastore/backref/btree_backref_manager.cc
index 235cbc32ac709b9c969f0ab8804adbfc7ffbd82f..9c0f2ba52412515502342b33ae437dfe488ea155 100644 (file)
--- a/src/crimson/os/seastore/backref/btree_backref_manager.cc
+++ b/src/crimson/os/seastore/backref/btree_backref_manager.cc
@@ -218,17 +218,17 @@ BtreeBackrefManager::merge_cached_backrefs(
     auto &backref_buffer = cache.get_backref_buffer();
     if (backref_buffer) {
       return seastar::do_with(
-       backref_buffer->backrefs.begin(),
+       backref_buffer->backrefs_by_seq.begin(),
        JOURNAL_SEQ_NULL,
        [this, &t, &limit, &backref_buffer, max](auto &iter, auto &inserted_to) {
        return trans_intr::repeat(
          [&iter, this, &t, &limit, &backref_buffer, max, &inserted_to]()
          -> merge_cached_backrefs_iertr::future<seastar::stop_iteration> {
-         if (iter == backref_buffer->backrefs.end())
+         if (iter == backref_buffer->backrefs_by_seq.end())
            return seastar::make_ready_future<seastar::stop_iteration>(
              seastar::stop_iteration::yes);
          auto &seq = iter->first;
-         auto &backref_list = iter->second;
+         auto &backref_list = iter->second.br_list;
          LOG_PREFIX(BtreeBackrefManager::merge_cached_backrefs);
          DEBUGT("seq {}, limit {}, num_fresh_backref {}"
            , t, seq, limit, t.get_num_fresh_backref());
@@ -238,22 +238,22 @@ BtreeBackrefManager::merge_cached_backrefs(
              backref_list,
              [this, &t](auto &backref) {
              LOG_PREFIX(BtreeBackrefManager::merge_cached_backrefs);
-             if (backref->laddr != L_ADDR_NULL) {
+             if (backref.laddr != L_ADDR_NULL) {
                DEBUGT("new mapping: {}~{} -> {}",
-                 t, backref->paddr, backref->len, backref->laddr);
+                 t, backref.paddr, backref.len, backref.laddr);
                return new_mapping(
                  t,
-                 backref->paddr,
-                 backref->len,
-                 backref->laddr,
-                 backref->type).si_then([](auto &&pin) {
+                 backref.paddr,
+                 backref.len,
+                 backref.laddr,
+                 backref.type).si_then([](auto &&pin) {
                  return seastar::now();
                });
              } else {
-               DEBUGT("remove mapping: {}", t, backref->paddr);
+               DEBUGT("remove mapping: {}", t, backref.paddr);
                return remove_mapping(
                  t,
-                 backref->paddr).si_then([](auto&&) {
+                 backref.paddr).si_then([](auto&&) {
                  return seastar::now();
                }).handle_error_interruptible(
                  crimson::ct_error::input_output_error::pass_further(),
@@ -502,4 +502,8 @@ BtreeBackrefManager::retrieve_backref_extents(
   });
 }
 
+bool BtreeBackrefManager::backref_should_be_removed(paddr_t paddr) {
+  return cache.backref_should_be_removed(paddr);
+}
+
 } // namespace crimson::os::seastore::backref

diff --git a/src/crimson/os/seastore/backref/btree_backref_manager.h b/src/crimson/os/seastore/backref/btree_backref_manager.h
index 0a3395270378d80c292a1e79325aa7166d1b84f1..ddfae7ec48c5388c65e1cf640a3dd13e1a892782 100644 (file)
--- a/src/crimson/os/seastore/backref/btree_backref_manager.h
+++ b/src/crimson/os/seastore/backref/btree_backref_manager.h
@@ -126,6 +126,8 @@ public:
 
   void cache_new_backref_extent(paddr_t paddr, extent_types_t type) final;
 
+  bool backref_should_be_removed(paddr_t paddr) final;
+
 private:
   SegmentManagerGroup &sm_group;
   Cache &cache;

diff --git a/src/crimson/os/seastore/backref_manager.h b/src/crimson/os/seastore/backref_manager.h
index ab9d5108b89bd7133bb5356ad09b281c2c574b77..f3f661086d59ceb4dd1cb864009be8911e97ae21 100644 (file)
--- a/src/crimson/os/seastore/backref_manager.h
+++ b/src/crimson/os/seastore/backref_manager.h
@@ -102,6 +102,8 @@ public:
     paddr_t start,
     paddr_t end) = 0;
 
+  virtual bool backref_should_be_removed(paddr_t paddr) = 0;
+
   using retrieve_backref_extents_iertr = trans_iertr<
     crimson::errorator<
       crimson::ct_error::input_output_error>

diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc
index 9183e5cd0bb8b22e090c8332271b1a91bb552653..7c6872e65093eafb97f22cbf61f97c93ddda6cbc 100644 (file)
--- a/src/crimson/os/seastore/cache.cc
+++ b/src/crimson/os/seastore/cache.cc
@@ -1297,40 +1297,53 @@ void Cache::backref_batch_update(
   LOG_PREFIX(Cache::backref_batch_update);
   DEBUG("inserting {} entries", list.size());
   if (!backref_buffer) {
-    backref_buffer = std::make_unique<backref_buffer_t>();
+    backref_buffer = std::make_unique<backref_cache_t>();
   }
   // backref_buf_entry_t::laddr == L_ADDR_NULL means erase
   for (auto &ent : list) {
     if (ent->laddr == L_ADDR_NULL) {
-      auto [it, insert] = backref_remove_set.insert(*ent);
-      boost::ignore_unused(insert);
+      auto insert_set_iter = backref_inserted_set.find(
+       ent->paddr, backref_buf_entry_t::cmp_t());
+      if (insert_set_iter == backref_inserted_set.end()) {
+       // backref to be removed isn't in the backref buffer,
+       // it must be in the backref tree.
+       auto [it, insert] = backref_remove_set.insert(*ent);
+       boost::ignore_unused(insert);
 #ifndef NDEBUG
-      if (!insert) {
-       ERROR("backref_remove_set already contains {}", ent->paddr);
-      }
+       if (!insert) {
+         ERROR("backref_remove_set already contains {}", ent->paddr);
+       }
 #endif
-      assert(insert);
-    } else {
-#ifndef NDEBUG
-      auto r = backref_remove_set.erase(ent->paddr, backref_buf_entry_t::cmp_t());
-      if (r) {
-       ERROR("backref_remove_set contains: {}", ent->paddr);
+       assert(insert);
+      } else {
+       // the backref insertion hasn't been applied to the
+       // backref tree
+       auto seq = insert_set_iter->seq;
+       auto it = backref_buffer->backrefs_by_seq.find(seq);
+       ceph_assert(it != backref_buffer->backrefs_by_seq.end());
+       auto &backref_buf = it->second;
+       assert(insert_set_iter->backref_buf_hook.is_linked());
+       backref_buf.br_list.erase(
+         backref_buf_entry_t::list_t::s_iterator_to(*insert_set_iter));
+       backref_inserted_set.erase(insert_set_iter);
       }
-      assert(!r);
-#endif
+    } else {
       auto [it, insert] = backref_inserted_set.insert(*ent);
       boost::ignore_unused(insert);
       assert(insert);
     }
   }
 
-  auto iter = backref_buffer->backrefs.find(seq);
-  if (iter == backref_buffer->backrefs.end()) {
-    backref_buffer->backrefs.emplace(
+  auto iter = backref_buffer->backrefs_by_seq.find(seq);
+  if (iter == backref_buffer->backrefs_by_seq.end()) {
+    backref_buffer->backrefs_by_seq.emplace(
       seq, std::move(list));
   } else {
-    iter->second.insert(
-      iter->second.end(),
+    for (auto &ref : list) {
+      iter->second.br_list.push_back(*ref);
+    }
+    iter->second.backrefs.insert(
+      iter->second.backrefs.end(),
       std::make_move_iterator(list.begin()),
       std::make_move_iterator(list.end()));
   }

diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index f78934defa3c81e65b9181ae1583f60d367197c4..316953ed43dc9150b0230dd2ce605bda7d557ead 100644 (file)
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -47,12 +47,12 @@ struct backref_buf_entry_t {
       len(alloc_blk.len),
       type(alloc_blk.type)
   {}
-  const paddr_t paddr = P_ADDR_NULL;
-  const laddr_t laddr = L_ADDR_NULL;
-  const extent_len_t len = 0;
-  const extent_types_t type =
+  paddr_t paddr = P_ADDR_NULL;
+  laddr_t laddr = L_ADDR_NULL;
+  extent_len_t len = 0;
+  extent_types_t type =
     extent_types_t::ROOT;
-  const journal_seq_t seq;
+  journal_seq_t seq;
   friend bool operator< (
     const backref_buf_entry_t &l,
     const backref_buf_entry_t &r) {
@@ -68,19 +68,35 @@ struct backref_buf_entry_t {
     const backref_buf_entry_t &r) {
     return l.paddr == r.paddr;
   }
-  using hook_t =
+  using set_hook_t =
     boost::intrusive::set_member_hook<
       boost::intrusive::link_mode<
        boost::intrusive::auto_unlink>>;
-  hook_t backref_set_hook;
+  set_hook_t backref_set_hook;
+
+  using list_hook_t =
+    boost::intrusive::list_member_hook<
+      boost::intrusive::link_mode<
+       boost::intrusive::auto_unlink>>;
+  list_hook_t backref_buf_hook;
+
   using backref_set_member_options = boost::intrusive::member_hook<
     backref_buf_entry_t,
-    hook_t,
+    set_hook_t,
     &backref_buf_entry_t::backref_set_hook>;
   using set_t = boost::intrusive::set<
     backref_buf_entry_t,
     backref_set_member_options,
     boost::intrusive::constant_time_size<false>>;
+
+  using backref_list_member_options = boost::intrusive::member_hook<
+    backref_buf_entry_t,
+    list_hook_t,
+    &backref_buf_entry_t::backref_buf_hook>;
+  using list_t = boost::intrusive::list<
+    backref_buf_entry_t,
+    backref_list_member_options,
+    boost::intrusive::constant_time_size<false>>;
   struct cmp_t {
     using is_transparent = paddr_t;
     bool operator()(
@@ -100,10 +116,20 @@ struct backref_buf_entry_t {
 using backref_buf_entry_ref =
   std::unique_ptr<backref_buf_entry_t>;
 
-struct backref_buffer_t {
-  std::map<journal_seq_t, std::vector<backref_buf_entry_ref>> backrefs;
+struct backref_buf_t {
+  backref_buf_t(std::vector<backref_buf_entry_ref> &&refs) : backrefs(std::move(refs)) {
+    for (auto &ref : backrefs) {
+      br_list.push_back(*ref);
+    }
+  }
+  std::vector<backref_buf_entry_ref> backrefs;
+  backref_buf_entry_t::list_t br_list;
 };
-using backref_buffer_ref = std::unique_ptr<backref_buffer_t>;
+
+struct backref_cache_t {
+  std::map<journal_seq_t, backref_buf_t> backrefs_by_seq;
+};
+using backref_cache_ref = std::unique_ptr<backref_cache_t>;
 
 /**
  * Cache
@@ -529,7 +555,7 @@ private:
     }
   }
 
-  backref_buffer_ref backref_buffer;
+  backref_cache_ref backref_buffer;
   // backrefs that needs to be inserted into the backref tree
   backref_buf_entry_t::set_t backref_inserted_set;
   backref_buf_entry_t::set_t backref_remove_set; // backrefs needs to be removed
@@ -589,6 +615,11 @@ private:
     return *it;
   }
 
+  bool backref_should_be_removed(paddr_t addr) {
+    return backref_remove_set.find(
+      addr, backref_buf_entry_t::cmp_t()) != backref_remove_set.end();
+  }
+
   const backref_buf_entry_t::set_t& get_backrefs() {
     return backref_inserted_set;
   }
@@ -597,7 +628,7 @@ private:
     return backref_remove_set;
   }
 
-  backref_buffer_ref& get_backref_buffer() {
+  backref_cache_ref& get_backref_buffer() {
     return backref_buffer;
   }
 
@@ -639,11 +670,11 @@ public:
   void trim_backref_bufs(const journal_seq_t &trim_to) {
     LOG_PREFIX(Cache::trim_backref_bufs);
     SUBDEBUG(seastore_cache, "trimming to {}", trim_to);
-    if (backref_buffer && !backref_buffer->backrefs.empty()) {
-      assert(backref_buffer->backrefs.rbegin()->first >= trim_to);
-      auto iter = backref_buffer->backrefs.upper_bound(trim_to);
-      backref_buffer->backrefs.erase(
-       backref_buffer->backrefs.begin(), iter);
+    if (backref_buffer && !backref_buffer->backrefs_by_seq.empty()) {
+      assert(backref_buffer->backrefs_by_seq.rbegin()->first >= trim_to);
+      auto iter = backref_buffer->backrefs_by_seq.upper_bound(trim_to);
+      backref_buffer->backrefs_by_seq.erase(
+       backref_buffer->backrefs_by_seq.begin(), iter);
     }
   }
 
@@ -889,8 +920,8 @@ public:
   std::optional<journal_seq_t> get_oldest_backref_dirty_from() const {
     LOG_PREFIX(Cache::get_oldest_backref_dirty_from);
     journal_seq_t backref_oldest = JOURNAL_SEQ_NULL;
-    if (backref_buffer && !backref_buffer->backrefs.empty()) {
-      backref_oldest = backref_buffer->backrefs.begin()->first;
+    if (backref_buffer && !backref_buffer->backrefs_by_seq.empty()) {
+      backref_oldest = backref_buffer->backrefs_by_seq.begin()->first;
     }
     if (backref_oldest == JOURNAL_SEQ_NULL) {
       SUBDEBUG(seastore_cache, "backref_oldest: null");

diff --git a/src/crimson/os/seastore/segment_cleaner.cc b/src/crimson/os/seastore/segment_cleaner.cc
index ef582006e7a1a157b2dc5006036a15b2376d8c4a..c553f58d298af35a56985319650e938912fa64ad 100644 (file)
--- a/src/crimson/os/seastore/segment_cleaner.cc
+++ b/src/crimson/os/seastore/segment_cleaner.cc
@@ -1271,11 +1271,10 @@ void SegmentCleaner::mark_space_used(
 
 void SegmentCleaner::mark_space_free(
   paddr_t addr,
-  extent_len_t len,
-  bool force)
+  extent_len_t len)
 {
   LOG_PREFIX(SegmentCleaner::mark_space_free);
-  if (!init_complete && !force) {
+  if (!init_complete) {
     return;
   }
   if (addr.get_addr_type() != addr_types_t::SEGMENT) {

diff --git a/src/crimson/os/seastore/segment_cleaner.h b/src/crimson/os/seastore/segment_cleaner.h
index 9467bc75060940b24a319bd06ed1b852e7006de8..c28e7b3686eafe4150c581b3d56149fba7750a8e 100644 (file)
--- a/src/crimson/os/seastore/segment_cleaner.h
+++ b/src/crimson/os/seastore/segment_cleaner.h
@@ -796,8 +796,7 @@ public:
 
   void mark_space_free(
     paddr_t addr,
-    extent_len_t len,
-    bool force = false);
+    extent_len_t len);
 
   SpaceTrackerIRef get_empty_space_tracker() const {
     return space_tracker->make_empty();

diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc
index 10185dbb34a4035940833d4a6db2a0fc20c21064..269e1a074149ffec940ce184cf6b0593195ae062 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.cc
+++ b/src/crimson/os/seastore/transaction_manager.cc
@@ -134,7 +134,8 @@ TransactionManager::mount_ertr::future<> TransactionManager::mount()
                    t,
                    addr,
                    len);
-                 if (addr.is_real()) {
+                 if (addr.is_real() &&
+                     !backref_manager->backref_should_be_removed(addr)) {
                    segment_cleaner->mark_space_used(
                      addr,
                      len ,
@@ -163,14 +164,6 @@ TransactionManager::mount_ertr::future<> TransactionManager::mount()
                      seastar::lowres_system_clock::time_point(),
                      true);
                  }
-                 auto &del_backrefs = backref_manager->get_cached_backref_removals();
-                 DEBUG("marking {} backrefs free", del_backrefs.size());
-                 for (auto &del_backref : del_backrefs) {
-                   segment_cleaner->mark_space_free(
-                     del_backref.paddr,
-                     del_backref.len,
-                     true);
-                 }
                  return seastar::now();
                });
            });

diff --git a/src/test/crimson/seastore/test_transaction_manager.cc b/src/test/crimson/seastore/test_transaction_manager.cc
index f8ef2b399c4ebe87e4c0907f100bc4e4e8f26f69..d650fff95ddd9987374d6f8366401208d2ba8869 100644 (file)
--- a/src/test/crimson/seastore/test_transaction_manager.cc
+++ b/src/test/crimson/seastore/test_transaction_manager.cc
@@ -402,8 +402,9 @@ struct transaction_manager_test_t :
       [this, &tracker](auto &t) {
        return backref_manager->scan_mapped_space(
          t,
-         [&tracker](auto offset, auto len, depth_t) {
-           if (offset.get_addr_type() == addr_types_t::SEGMENT) {
+         [&tracker, this](auto offset, auto len, depth_t) {
+           if (offset.get_addr_type() == addr_types_t::SEGMENT &&
+               !backref_manager->backref_should_be_removed(offset)) {
              logger().debug("check_usage: tracker alloc {}~{}",
                offset, len);
              tracker->allocate(
@@ -423,17 +424,6 @@ struct transaction_manager_test_t :
                  backref.len);
              }
            }
-           auto &del_backrefs = backref_manager->get_cached_backref_removals();
-           for (auto &del_backref : del_backrefs) {
-             if (del_backref.paddr.get_addr_type() == addr_types_t::SEGMENT) {
-               logger().debug("check_usage: by backref, tracker release {}~{}",
-                 del_backref.paddr, del_backref.len);
-               tracker->release(
-                 del_backref.paddr.as_seg_paddr().get_segment_id(),
-                 del_backref.paddr.as_seg_paddr().get_segment_off(),
-                 del_backref.len);
-             }
-           }
            return seastar::now();
          });
       }).unsafe_get0();