crimson/os/seastore: replace fresh_block_list with inline/ool_block_list

With this change, Transaction::write_set is the union of mutually exclusive
lists
- delayed_alloc_list, inline_block_list, and mutated_block_list prior to
  calling ExtentPlacementManager::delayed_alloc_or_ool_write

and

- inline_block_list, ool_block_list, and mutated_block_list after
  calling ExtentPlacementManager::delayed_alloc_or_ool_write

Signed-off-by: Samuel Just <sjust@redhat.com>

diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc
index a7c5d4fcc6af2598c79e8368d3fbefa0d3817687..505c6e49566d0e17896e5d4229096c9b4ed693df 100644 (file)
--- a/src/crimson/os/seastore/cache.cc
+++ b/src/crimson/os/seastore/cache.cc
@@ -653,10 +653,10 @@ void Cache::invalidate(Transaction& t, CachedExtent& conflicting_extent)
       efforts.retire.bytes += i->get_length();
     }
 
-    efforts.fresh.extents += t.fresh_block_list.size();
-    for (auto &i: t.fresh_block_list) {
+    efforts.fresh.extents += t.get_num_fresh_blocks();
+    t.for_each_fresh_block([&](auto &i) {
       efforts.fresh.bytes += i->get_length();
-    }
+    });
 
     for (auto &i: t.mutated_block_list) {
       if (!i->is_valid()) {
@@ -680,7 +680,7 @@ void Cache::invalidate(Transaction& t, CachedExtent& conflicting_extent)
   } else {
     // read transaction won't have non-read efforts
     assert(t.retired_set.empty());
-    assert(t.fresh_block_list.empty());
+    assert(t.get_num_fresh_blocks() == 0);
     assert(t.mutated_block_list.empty());
     assert(t.onode_tree_stats.is_clear());
     assert(t.lba_tree_stats.is_clear());
@@ -703,7 +703,7 @@ void Cache::on_transaction_destruct(Transaction& t)
     }
     // read transaction won't have non-read efforts
     assert(t.retired_set.empty());
-    assert(t.fresh_block_list.empty());
+    assert(t.get_num_fresh_blocks() == 0);
     assert(t.mutated_block_list.empty());
     assert(t.onode_tree_stats.is_clear());
     assert(t.lba_tree_stats.is_clear());
@@ -883,8 +883,8 @@ record_t Cache::prepare_record(Transaction &t)
     retire_extent(i);
   }
 
-  record.extents.reserve(t.fresh_block_list.size());
-  for (auto &i: t.fresh_block_list) {
+  record.extents.reserve(t.inline_block_list.size());
+  for (auto &i: t.inline_block_list) {
     DEBUGT("fresh block {}", t, *i);
     if (!i->is_inline()) {
       continue;
@@ -920,7 +920,8 @@ void Cache::complete_commit(
   LOG_PREFIX(Cache::complete_commit);
   DEBUGT("enter", t);
 
-  for (auto &i: t.fresh_block_list) {
+  
+  t.for_each_fresh_block([&](auto &i) {
     if (i->is_inline()) {
       i->set_paddr(final_block_start.add_relative(i->get_paddr()));
     }
@@ -929,18 +930,17 @@ void Cache::complete_commit(
 
     if (!i->is_valid()) {
       DEBUGT("invalid {}", t, *i);
-      continue;
-    }
-
-    i->state = CachedExtent::extent_state_t::CLEAN;
-    DEBUGT("fresh {}", t, *i);
-    add_extent(i);
-    if (cleaner) {
-      cleaner->mark_space_used(
-       i->get_paddr(),
-       i->get_length());
+    } else {
+      i->state = CachedExtent::extent_state_t::CLEAN;
+      DEBUGT("fresh {}", t, *i);
+      add_extent(i);
+      if (cleaner) {
+       cleaner->mark_space_used(
+         i->get_paddr(),
+         i->get_length());
+      }
     }
-  }
+  });
 
   // Add new copy of mutated blocks, set_io_wait to block until written
   for (auto &i: t.mutated_block_list) {

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
index 5e0d608199d278048f05a463093032e2f3e0745d..a1d3d60d748dc1ffd16a09d6e0d8be603b0e518f 100644 (file)
--- a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc
+++ b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc
@@ -229,11 +229,12 @@ void BtreeLBAManager::complete_transaction(
 
   // ...but add_pin from parent->leaf
   std::vector<CachedExtentRef> to_link;
-  to_link.reserve(t.get_fresh_block_list().size());
-  for (auto &e: t.get_fresh_block_list()) {
+  to_link.reserve(t.get_num_fresh_blocks());
+  t.for_each_fresh_block([&](auto &e) {
     if (e->is_valid() && (is_lba_node(*e) || e->is_logical()))
       to_link.push_back(e);
-  }
+  });
+
   std::sort(
     to_link.begin(), to_link.end(),
     [](auto &l, auto &r) -> bool { return get_depth(*l) > get_depth(*r); });

diff --git a/src/crimson/os/seastore/transaction.h b/src/crimson/os/seastore/transaction.h
index 99920916e641d83edd91abf3c2ecbae15c497be3..9a1cf83fafc63963bf4e810af81e7c9a9b5d5f57 100644 (file)
--- a/src/crimson/os/seastore/transaction.h
+++ b/src/crimson/os/seastore/transaction.h
@@ -92,7 +92,7 @@ public:
     } else {
       ref->set_paddr(make_record_relative_paddr(offset));
       offset += ref->get_length();
-      fresh_block_list.push_back(ref);
+      inline_block_list.push_back(ref);
     }
     write_set.insert(*ref);
   }
@@ -101,7 +101,7 @@ public:
     write_set.erase(*ref);
     ref->set_paddr(make_record_relative_paddr(offset));
     offset += ref->get_length();
-    fresh_block_list.push_back(ref);
+    inline_block_list.push_back(ref);
     write_set.insert(*ref);
   }
 
@@ -110,7 +110,7 @@ public:
     ref->set_paddr(final_addr);
     assert(!ref->get_paddr().is_null());
     assert(!ref->is_inline());
-    fresh_block_list.push_back(ref);
+    ool_block_list.push_back(ref);
     write_set.insert(*ref);
   }
 
@@ -152,10 +152,6 @@ public:
     return to_release;
   }
 
-  const auto &get_fresh_block_list() {
-    return fresh_block_list;
-  }
-
   auto& get_delayed_alloc_list() {
     return delayed_alloc_list;
   }
@@ -168,6 +164,16 @@ public:
     return retired_set;
   }
 
+  template <typename F>
+  auto for_each_fresh_block(F &&f) {
+    std::for_each(ool_block_list.begin(), ool_block_list.end(), f);
+    std::for_each(inline_block_list.begin(), inline_block_list.end(), f);
+  }
+
+  auto get_num_fresh_blocks() const {
+    return inline_block_list.size() + ool_block_list.size();
+  }
+
   enum class src_t : uint8_t {
     MUTATE = 0,
     READ, // including weak and non-weak read transactions
@@ -228,9 +234,10 @@ public:
     delayed_temp_offset = 0;
     read_set.clear();
     invalidate_clear_write_set();
-    fresh_block_list.clear();
     mutated_block_list.clear();
     delayed_alloc_list.clear();
+    inline_block_list.clear();
+    ool_block_list.clear();
     retired_set.clear();
     onode_tree_stats = {};
     lba_tree_stats = {};
@@ -278,16 +285,43 @@ private:
   segment_off_t offset = 0; ///< relative offset of next block
   segment_off_t delayed_temp_offset = 0;
 
+  /**
+   * read_set
+   *
+   * Holds a reference (with a refcount) to every extent read via *this.
+   * Submitting a transaction mutating any contained extent/addr will
+   * invalidate *this.
+   */
   read_set_t<Transaction> read_set; ///< set of extents read by paddr
-  ExtentIndex write_set;            ///< set of extents written by paddr
 
-  std::list<CachedExtentRef> fresh_block_list;   ///< list of fresh blocks
-  std::list<CachedExtentRef> mutated_block_list; ///< list of mutated blocks
-  ///< list of ool extents whose addresses are not
-  //   determine until transaction submission
+  /**
+   * write_set
+   *
+   * Contains a reference (without a refcount) to every extent mutated
+   * as part of *this.  No contained extent may be referenced outside
+   * of *this.  Every contained extent will be in one of inline_block_list,
+   * ool_block_list, mutated_block_list, or delayed_alloc_list.
+   */
+  ExtentIndex write_set;
+
+  /// list of fresh blocks, holds refcounts, subset of write_set
+  std::list<CachedExtentRef> inline_block_list;
+
+  /// list of fresh blocks, holds refcounts, subset of write_set
+  std::list<CachedExtentRef> ool_block_list;
+
+  /// extents with delayed allocation, may become inline or ool
   std::list<LogicalCachedExtentRef> delayed_alloc_list;
 
-  pextent_set_t retired_set; ///< list of extents mutated by this transaction
+  /// list of mutated blocks, holds refcounts, subset of write_set
+  std::list<CachedExtentRef> mutated_block_list;
+
+  /**
+   * retire_set
+   *
+   * Set of extents retired by *this.
+   */
+  pextent_set_t retired_set;
 
   tree_stats_t onode_tree_stats;
   tree_stats_t lba_tree_stats;