crimson/os/seastore: delay setting bptr for all types of extent in case of read

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>

diff --git a/src/crimson/common/fixed_kv_node_layout.h b/src/crimson/common/fixed_kv_node_layout.h
index 2a91ac3954006b16aa7c2368cb2c955d52da3464..db62a2df32dd6231528b90d23053edf2eab7a58f 100644 (file)
--- a/src/crimson/common/fixed_kv_node_layout.h
+++ b/src/crimson/common/fixed_kv_node_layout.h
@@ -360,11 +360,16 @@ public:
   }
 
 
-  FixedKVNodeLayout(char *buf) :
-    buf(buf) {}
+  FixedKVNodeLayout() : buf(nullptr) {}
 
   virtual ~FixedKVNodeLayout() = default;
 
+  void set_layout_buf(char *_buf) {
+    assert(buf == nullptr);
+    assert(_buf != nullptr);
+    buf = _buf;
+  }
+
   const_iterator begin() const {
     return const_iterator(
       this,

diff --git a/src/crimson/os/seastore/btree/fixed_kv_node.h b/src/crimson/os/seastore/btree/fixed_kv_node.h
index 1f1b37c3d81eac315f262b9663b482e468b27efa..63e2ca38c427dc62b1318e6e2d79093b95656f44 100644 (file)
--- a/src/crimson/os/seastore/btree/fixed_kv_node.h
+++ b/src/crimson/os/seastore/btree/fixed_kv_node.h
@@ -165,6 +165,11 @@ struct FixedKVNode : ChildableCachedExtent {
     : ChildableCachedExtent(std::move(ptr)),
       children(capacity, nullptr),
       capacity(capacity) {}
+  // Must be identical with FixedKVNode(capacity, ptr) after on_fully_loaded()
+  explicit FixedKVNode(uint16_t capacity, extent_len_t length)
+    : ChildableCachedExtent(length),
+      children(capacity, nullptr),
+      capacity(capacity) {}
   FixedKVNode(const FixedKVNode &rhs)
     : ChildableCachedExtent(rhs),
       range(rhs.range),
@@ -708,12 +713,17 @@ struct FixedKVInternalNode
     node_size,
     node_type_t>;
 
-  FixedKVInternalNode(ceph::bufferptr &&ptr)
-    : FixedKVNode<NODE_KEY>(CAPACITY, std::move(ptr)),
-      node_layout_t(this->get_bptr().c_str()) {}
+  explicit FixedKVInternalNode(ceph::bufferptr &&ptr)
+    : FixedKVNode<NODE_KEY>(CAPACITY, std::move(ptr)) {
+    this->set_layout_buf(this->get_bptr().c_str());
+  }
+  // Must be identical with FixedKVInternalNode(ptr) after on_fully_loaded()
+  explicit FixedKVInternalNode(extent_len_t length)
+    : FixedKVNode<NODE_KEY>(CAPACITY, length) {}
   FixedKVInternalNode(const FixedKVInternalNode &rhs)
-    : FixedKVNode<NODE_KEY>(rhs),
-      node_layout_t(this->get_bptr().c_str()) {}
+    : FixedKVNode<NODE_KEY>(rhs) {
+    this->set_layout_buf(this->get_bptr().c_str());
+  }
 
   bool have_children() const final {
     return true;
@@ -985,6 +995,10 @@ struct FixedKVInternalNode
       pivot);
   }
 
+  void on_fully_loaded() final {
+    this->set_layout_buf(this->get_bptr().c_str());
+  }
+
   /**
    * Internal relative addresses on read or in memory prior to commit
    * are either record or block relative depending on whether this
@@ -1121,13 +1135,18 @@ struct FixedKVLeafNode
     node_type_t,
     has_children>;
   using base_t = FixedKVNode<NODE_KEY>;
-  FixedKVLeafNode(ceph::bufferptr &&ptr)
-    : FixedKVNode<NODE_KEY>(has_children ? CAPACITY : 0, std::move(ptr)),
-      node_layout_t(this->get_bptr().c_str()) {}
+  explicit FixedKVLeafNode(ceph::bufferptr &&ptr)
+    : FixedKVNode<NODE_KEY>(has_children ? CAPACITY : 0, std::move(ptr)) {
+    this->set_layout_buf(this->get_bptr().c_str());
+  }
+  // Must be identical with FixedKVLeafNode(ptr) after on_fully_loaded()
+  explicit FixedKVLeafNode(extent_len_t length)
+    : FixedKVNode<NODE_KEY>(has_children ? CAPACITY : 0, length) {}
   FixedKVLeafNode(const FixedKVLeafNode &rhs)
     : FixedKVNode<NODE_KEY>(rhs),
-      node_layout_t(this->get_bptr().c_str()),
-      modifications(rhs.modifications) {}
+      modifications(rhs.modifications) {
+    this->set_layout_buf(this->get_bptr().c_str());
+  }
 
   static constexpr bool do_has_children = has_children;
   // for the stable extent, modifications is always 0;
@@ -1234,6 +1253,10 @@ struct FixedKVLeafNode
     }
   }
 
+  void on_fully_loaded() final {
+    this->set_layout_buf(this->get_bptr().c_str());
+  }
+
   void prepare_commit() final {
     if constexpr (has_children) {
       if (this->is_initial_pending()) {

diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h
index 07db3bd488e11b6b88fe1aee9e1745214833e612..0df66e37d1169cbef83a019ec7bb491666f7041e 100644 (file)
--- a/src/crimson/os/seastore/cached_extent.h
+++ b/src/crimson/os/seastore/cached_extent.h
@@ -348,6 +348,17 @@ public:
    */
   virtual void on_initial_write() {}
 
+  /**
+   * on_fully_loaded
+   *
+   * Called when ptr is ready. Normally this should be used to initiate
+   * the extent to be identical to CachedExtent(ptr).
+   *
+   * Note this doesn't mean the content is fully read, use on_clean_read for
+   * this purpose.
+   */
+  virtual void on_fully_loaded() {}
+
   /**
    * on_clean_read
    *
@@ -880,7 +891,7 @@ protected:
   CachedExtent(CachedExtent &&other) = delete;
 
   /// construct a fully loaded CachedExtent
-  CachedExtent(ceph::bufferptr &&_ptr)
+  explicit CachedExtent(ceph::bufferptr &&_ptr)
     : length(_ptr.length()),
       loaded_length(_ptr.length()) {
     ptr = std::move(_ptr);
@@ -892,7 +903,8 @@ protected:
   }
 
   /// construct a partially loaded CachedExtent
-  CachedExtent(extent_len_t _length)
+  /// must be identical with CachedExtent(ptr) after on_fully_loaded()
+  explicit CachedExtent(extent_len_t _length)
     : length(_length),
       loaded_length(0),
       buffer_space(std::in_place) {
@@ -1048,6 +1060,7 @@ protected:
       loaded_length = _length;
       buffer_space.reset();
       assert(is_fully_loaded());
+      on_fully_loaded();
       load_ranges_t ret;
       ret.push_back(offset, *ptr);
       return ret;
@@ -1061,6 +1074,7 @@ protected:
       ptr = buffer_space->to_full_ptr(length);
       buffer_space.reset();
       assert(is_fully_loaded());
+      on_fully_loaded();
       // adjust ret since the ptr has been rebuild
       for (load_range_t& range : ret.ranges) {
         auto range_length = range.ptr.length();

diff --git a/src/crimson/os/seastore/collection_manager/collection_flat_node.h b/src/crimson/os/seastore/collection_manager/collection_flat_node.h
index aa1e71356130825c1f4b80fea00127638e906d9d..1f4de652bba36d80b30625764a940df31528d012 100644 (file)
--- a/src/crimson/os/seastore/collection_manager/collection_flat_node.h
+++ b/src/crimson/os/seastore/collection_manager/collection_flat_node.h
@@ -96,6 +96,8 @@ struct CollectionNode
 
   explicit CollectionNode(ceph::bufferptr &&ptr)
     : LogicalCachedExtent(std::move(ptr)) {}
+  explicit CollectionNode(extent_len_t length)
+    : LogicalCachedExtent(length) {}
   explicit CollectionNode(const CollectionNode &other)
     : LogicalCachedExtent(other),
       decoded(other.decoded) {}

diff --git a/src/crimson/os/seastore/omap_manager/btree/omap_btree_node.h b/src/crimson/os/seastore/omap_manager/btree/omap_btree_node.h
index 795daeddb11c5f87eec47879e050697bd074ab7f..7c2392731c0e8a02d0db16645c50265d20feb3a5 100644 (file)
--- a/src/crimson/os/seastore/omap_manager/btree/omap_btree_node.h
+++ b/src/crimson/os/seastore/omap_manager/btree/omap_btree_node.h
@@ -48,7 +48,8 @@ struct OMapNode : LogicalCachedExtent {
       need_merge(n_merge) {}
   };
 
-  OMapNode(ceph::bufferptr &&ptr) : LogicalCachedExtent(std::move(ptr)) {}
+  explicit OMapNode(ceph::bufferptr &&ptr) : LogicalCachedExtent(std::move(ptr)) {}
+  explicit OMapNode(extent_len_t length) : LogicalCachedExtent(length) {}
   OMapNode(const OMapNode &other)
   : LogicalCachedExtent(other) {}
 

diff --git a/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.h b/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.h
index a2b51bbb0e135f04c48ea34336cce84ac121c2d1..2267942f0355a20ead9528c5446af50a27bcf238 100644 (file)
--- a/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.h
+++ b/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.h
@@ -31,10 +31,18 @@ struct OMapInnerNode
     StringKVInnerNodeLayout {
   using OMapInnerNodeRef = TCachedExtentRef<OMapInnerNode>;
   using internal_iterator_t = const_iterator;
-  template <typename... T>
-  OMapInnerNode(T&&... t) :
-    OMapNode(std::forward<T>(t)...),
-    StringKVInnerNodeLayout(get_bptr().c_str()) {}
+
+  explicit OMapInnerNode(ceph::bufferptr &&ptr)
+    : OMapNode(std::move(ptr)) {
+    this->set_layout_buf(this->get_bptr().c_str());
+  }
+  // Must be identical with OMapInnerNode(ptr) after on_fully_loaded()
+  explicit OMapInnerNode(extent_len_t length)
+    : OMapNode(length) {}
+  OMapInnerNode(const OMapInnerNode &rhs)
+    : OMapNode(rhs) {
+    this->set_layout_buf(this->get_bptr().c_str());
+  }
 
   omap_node_meta_t get_node_meta() const final { return get_meta(); }
   bool extent_will_overflow(size_t ksize, std::optional<size_t> vsize) const {
@@ -46,6 +54,10 @@ struct OMapInnerNode
   bool extent_is_below_min() const { return below_min(); }
   uint32_t get_node_size() { return get_size(); }
 
+  void on_fully_loaded() final {
+    this->set_layout_buf(this->get_bptr().c_str());
+  }
+
   CachedExtentRef duplicate_for_write(Transaction&) final {
     assert(delta_buffer.empty());
     return CachedExtentRef(new OMapInnerNode(*this));
@@ -148,10 +160,18 @@ struct OMapLeafNode
 
   using OMapLeafNodeRef = TCachedExtentRef<OMapLeafNode>;
   using internal_iterator_t = const_iterator;
-  template <typename... T>
-  OMapLeafNode(T&&... t) :
-    OMapNode(std::forward<T>(t)...),
-    StringKVLeafNodeLayout(get_bptr().c_str()) {}
+
+  explicit OMapLeafNode(ceph::bufferptr &&ptr)
+    : OMapNode(std::move(ptr)) {
+    this->set_layout_buf(this->get_bptr().c_str());
+  }
+  // Must be identical with OMapLeafNode(ptr) after on_fully_loaded()
+  explicit OMapLeafNode(extent_len_t length)
+    : OMapNode(length) {}
+  OMapLeafNode(const OMapLeafNode &rhs)
+    : OMapNode(rhs) {
+    this->set_layout_buf(this->get_bptr().c_str());
+  }
 
   omap_node_meta_t get_node_meta() const final { return get_meta(); }
   bool extent_will_overflow(
@@ -164,6 +184,10 @@ struct OMapLeafNode
   bool extent_is_below_min() const { return below_min(); }
   uint32_t get_node_size() { return get_size(); }
 
+  void on_fully_loaded() final {
+    this->set_layout_buf(this->get_bptr().c_str());
+  }
+
   CachedExtentRef duplicate_for_write(Transaction&) final {
     assert(delta_buffer.empty());
     return CachedExtentRef(new OMapLeafNode(*this));

diff --git a/src/crimson/os/seastore/omap_manager/btree/string_kv_node_layout.h b/src/crimson/os/seastore/omap_manager/btree/string_kv_node_layout.h
index 72b13fedfb12b6231299a8eecffddf8cf098e55e..3825ebef145534fd62eae7052f49ef679af1b4f9 100644 (file)
--- a/src/crimson/os/seastore/omap_manager/btree/string_kv_node_layout.h
+++ b/src/crimson/os/seastore/omap_manager/btree/string_kv_node_layout.h
@@ -504,8 +504,13 @@ public:
     inner_remove(iter);
   }
 
-  StringKVInnerNodeLayout(char *buf) :
-    buf(buf) {}
+  StringKVInnerNodeLayout() : buf(nullptr) {}
+
+  void set_layout_buf(char *_buf) {
+    assert(buf == nullptr);
+    assert(_buf != nullptr);
+    buf = _buf;
+  }
 
   uint32_t get_size() const {
     ceph_le32 &size = *layout.template Pointer<0>(buf);
@@ -1120,8 +1125,13 @@ public:
     leaf_remove(iter);
   }
 
-  StringKVLeafNodeLayout(char *buf) :
-    buf(buf) {}
+  StringKVLeafNodeLayout() : buf(nullptr) {}
+
+  void set_layout_buf(char *_buf) {
+    assert(buf == nullptr);
+    assert(_buf != nullptr);
+    buf = _buf;
+  }
 
   const_iterator iter_begin() const {
     return const_iterator(

diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager/seastore.h b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager/seastore.h
index b5d674ffcaaaaca7f36a76d92c18072e01e05b15..04b959f767de9fbf32f08036271a998e2538aca9 100644 (file)
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager/seastore.h
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager/seastore.h
@@ -41,8 +41,10 @@ class SeastoreSuper final: public Super {
 
 class SeastoreNodeExtent final: public NodeExtent {
  public:
-  SeastoreNodeExtent(ceph::bufferptr &&ptr)
+  explicit SeastoreNodeExtent(ceph::bufferptr &&ptr)
     : NodeExtent(std::move(ptr)) {}
+  explicit SeastoreNodeExtent(extent_len_t length)
+    : NodeExtent(length) {}
   SeastoreNodeExtent(const SeastoreNodeExtent& other)
     : NodeExtent(other) {}
   ~SeastoreNodeExtent() override = default;

diff --git a/src/test/crimson/seastore/test_block.h b/src/test/crimson/seastore/test_block.h
index fde6ad99c414248b48c5a860bef087c3abb89706..e1fe8e06f8a465e0d91ee7edcc5141ac2a8745d9 100644 (file)
--- a/src/test/crimson/seastore/test_block.h
+++ b/src/test/crimson/seastore/test_block.h
@@ -51,12 +51,12 @@ struct TestBlock : crimson::os::seastore::LogicalCachedExtent {
 
   interval_set<extent_len_t> modified_region;
 
-  TestBlock(ceph::bufferptr &&ptr)
+  explicit TestBlock(ceph::bufferptr &&ptr)
     : LogicalCachedExtent(std::move(ptr)) {}
+  explicit TestBlock(extent_len_t length)
+    : LogicalCachedExtent(length) {}
   TestBlock(const TestBlock &other)
     : LogicalCachedExtent(other), modified_region(other.modified_region) {}
-  TestBlock(extent_len_t length)
-    : LogicalCachedExtent(length) {}
 
   CachedExtentRef duplicate_for_write(Transaction&) final {
     return CachedExtentRef(new TestBlock(*this));
@@ -113,8 +113,10 @@ struct TestBlockPhysical : crimson::os::seastore::CachedExtent{
 
   void on_rewrite(Transaction&, CachedExtent&, extent_len_t) final {}
 
-  TestBlockPhysical(ceph::bufferptr &&ptr)
+  explicit TestBlockPhysical(ceph::bufferptr &&ptr)
     : CachedExtent(std::move(ptr)) {}
+  explicit TestBlockPhysical(extent_len_t length)
+    : CachedExtent(length) {}
   TestBlockPhysical(const TestBlockPhysical &other)
     : CachedExtent(other) {}
 

diff --git a/src/test/crimson/test_fixed_kv_node_layout.cc b/src/test/crimson/test_fixed_kv_node_layout.cc
index e6377ec14e3cf48421693ba763f55d349d835c42..9b6d19661ac1ed2fd7f7b67610d3c58a620981bf 100644 (file)
--- a/src/test/crimson/test_fixed_kv_node_layout.cc
+++ b/src/test/crimson/test_fixed_kv_node_layout.cc
@@ -88,12 +88,14 @@ struct TestNode : FixedKVNodeLayout<
   uint32_t, ceph_le32,
   test_val_t, test_val_le_t> {
   char buf[4096];
-  TestNode() : FixedKVNodeLayout(buf) {
+  TestNode() : FixedKVNodeLayout() {
+    set_layout_buf(buf);
     memset(buf, 0, sizeof(buf));
     set_meta({0, std::numeric_limits<uint32_t>::max()});
   }
   TestNode(const TestNode &rhs)
-    : FixedKVNodeLayout(buf) {
+    : FixedKVNodeLayout() {
+    set_layout_buf(buf);
     ::memcpy(buf, rhs.buf, sizeof(buf));
   }