common/bl: splice() doesn't thrash _carriage anymore.

Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>

diff --git a/src/common/buffer.cc b/src/common/buffer.cc
index c9ddc73c28aec103af69108f70da067746240b57..efd49b5b67115548566d60ab51bb72063d6b6b4c 100644 (file)
--- a/src/common/buffer.cc
+++ b/src/common/buffer.cc
@@ -1317,7 +1317,7 @@ static ceph::spinlock debug_lock;
     _len++;
   }
 
-  buffer::ptr buffer::list::always_empty_bptr;
+  buffer::ptr_node buffer::list::always_empty_bptr;
 
   buffer::ptr_node& buffer::list::refill_append_space(const unsigned len)
   {
@@ -1620,8 +1620,6 @@ static ceph::spinlock debug_lock;
       ++curbuf_prev;
     }
     
-    _carriage = &always_empty_bptr;
-
     while (len > 0) {
       // partial or the last (appendable) one?
       if (const auto to_drop = off + len; to_drop < curbuf->length()) {
@@ -1641,8 +1639,16 @@ static ceph::spinlock debug_lock;
       if (claim_by) 
        claim_by->append(*curbuf, off, howmuch);
       _len -= curbuf->length();
-      _num -= 1;
-      curbuf = _buffers.erase_after_and_dispose(curbuf_prev);
+      if (curbuf == _carriage) {
+        // no need to reallocate, shrinking and relinking is enough.
+        curbuf = _buffers.erase_after(curbuf_prev);
+       _carriage->set_offset(_carriage->offset() + _carriage->length());
+       _carriage->set_length(0);
+       _buffers.push_back(*_carriage);
+      } else {
+       curbuf = _buffers.erase_after_and_dispose(curbuf_prev);
+       _num -= 1;
+      }
       len -= howmuch;
       off = 0;
     }

diff --git a/src/include/buffer.h b/src/include/buffer.h
index 2ca686828d2fb3d29fb6d5836d111c4e5e81c393..97c54d51132e36a04aa031b3d40554894c5847ba 100644 (file)
--- a/src/include/buffer.h
+++ b/src/include/buffer.h
@@ -389,6 +389,8 @@ struct error_code;
     static ptr_node* copy_hypercombined(const ptr_node& copy_this);
 
   private:
+    friend list;
+
     template <class... Args>
     ptr_node(Args&&... args) : ptr(std::forward<Args>(args)...) {
     }
@@ -632,7 +634,7 @@ struct error_code;
     // track bufferptr we can modify (especially ::append() to). Not all bptrs
     // bufferlist holds have this trait -- if somebody ::push_back(const ptr&),
     // he expects it won't change.
-    ptr* _carriage;
+    ptr_node* _carriage;
     unsigned _len, _num;
 
     template <bool is_const>
@@ -919,7 +921,7 @@ struct error_code;
     // always_empty_bptr has no underlying raw but its _len is always 0.
     // This is useful for e.g. get_append_buffer_unused_tail_length() as
     // it allows to avoid conditionals on hot paths.
-    static ptr always_empty_bptr;
+    static ptr_node always_empty_bptr;
     ptr_node& refill_append_space(const unsigned len);
 
     // for page_aligned_appender; never ever expose this publicly!

diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc
index 90cb9239efcb2a27f7a2af0842232f25ab7d5031..44652367cfa1078f00b2bda1f7f1cbec2ca0ddc1 100644 (file)
--- a/src/os/bluestore/BlueFS.cc
+++ b/src/os/bluestore/BlueFS.cc
@@ -2775,8 +2775,6 @@ ceph::bufferlist BlueFS::FileWriter::flush_buffer(
     // Otherwise a costly rebuild could happen in e.g. `KernelDevice`.
     buffer_appender.append_zero(padding_len);
     buffer_appender.flush();
-    // FIXME: `splice` is currently broken at it always thrashes
-    // the `_carriage`. This will be fixed in a separated patch.
     buffer.splice(buffer.length() - padding_len, padding_len, &bl);
     // Deep copy the tail here. This allows to avoid costlier copy on
     // bufferlist rebuild in e.g. `KernelDevice` and minimizes number