From: Radoslaw Zarzynski <rzarzyns@redhat.com>
Date: Wed, 19 Aug 2020 13:53:46 +0000 (+0200)
Subject: common/bl: splice() doesn't thrash _carriage anymore.
X-Git-Tag: v16.1.0~780^2~3
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=a94a4d0227a0e86ab4b81db2eb550afb9caff84c;p=ceph.git

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 c9ddc73c28a..efd49b5b671 100644
--- 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 2ca686828d2..97c54d51132 100644
--- 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 90cb9239efc..44652367cfa 100644
--- 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