From: Radoslaw Zarzynski <rzarzyns@redhat.com>
Date: Wed, 19 Aug 2020 14:44:43 +0000 (+0200)
Subject: common/bl, os/bluestore: drop flush() from page_aligned_appender.
X-Git-Tag: v16.1.0~780^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=f40e522109f5f15a8d3730dd5b38b337477c924b;p=ceph-ci.git

common/bl, os/bluestore: drop flush() from page_aligned_appender.

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

diff --git a/src/include/buffer.h b/src/include/buffer.h
index 93ef1b57f87..5b6a4521496 100644
--- a/src/include/buffer.h
+++ b/src/include/buffer.h
@@ -872,14 +872,6 @@ struct error_code;
       friend class list;
 
     public:
-      ~page_aligned_appender() {
-	flush();
-      }
-
-      void flush() {
-	// nop
-      }
-
       void append(const bufferlist& l) {
 	bl.append(l);
 	bl.obtain_contiguous_space(0);
diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc
index eea2e1ac582..9e16ffdb8ea 100644
--- a/src/os/bluestore/BlueFS.cc
+++ b/src/os/bluestore/BlueFS.cc
@@ -2772,7 +2772,6 @@ ceph::bufferlist BlueFS::FileWriter::flush_buffer(
     // preserve in-memory contiguity and not mess with the alignment.
     // Otherwise a costly rebuild could happen in e.g. `KernelDevice`.
     buffer_appender.append_zero(padding_len);
-    buffer_appender.flush();
     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
@@ -2781,7 +2780,6 @@ ceph::bufferlist BlueFS::FileWriter::flush_buffer(
     // padding on a dedicated, 4 KB long memory chunk. This shouldn't
     // trigger the rebuild while still being less expensive.
     buffer_appender.substr_of(bl, bl.length() - padding_len - tail, tail);
-    buffer_appender.flush();
     buffer.splice(buffer.length() - tail, tail, &tail_block);
   } else {
     tail_block.clear();
@@ -2797,8 +2795,6 @@ int BlueFS::_flush_range(FileWriter *h, uint64_t offset, uint64_t length)
   ceph_assert(!h->file->deleted);
   ceph_assert(h->file->num_readers.load() == 0);
 
-  h->buffer_appender.flush();
-
   bool buffered;
   if (h->file->fnode.ino == 1)
     buffered = false;
@@ -2991,7 +2987,6 @@ int BlueFS::_flush(FileWriter *h, bool force, std::unique_lock<ceph::mutex>& l)
 
 int BlueFS::_flush(FileWriter *h, bool force, bool *flushed)
 {
-  h->buffer_appender.flush();
   uint64_t length = h->get_buffer_length();
   uint64_t offset = h->pos;
   if (flushed) {
@@ -3032,8 +3027,6 @@ int BlueFS::_truncate(FileWriter *h, uint64_t offset)
   // we never truncate internal log files
   ceph_assert(h->file->fnode.ino > 1);
 
-  h->buffer_appender.flush();
-
   // truncate off unflushed data?
   if (h->pos < offset &&
       h->pos + h->get_buffer_length() > offset) {
diff --git a/src/os/bluestore/BlueFS.h b/src/os/bluestore/BlueFS.h
index 3d95d69fdea..3ed3f359233 100644
--- a/src/os/bluestore/BlueFS.h
+++ b/src/os/bluestore/BlueFS.h
@@ -202,7 +202,6 @@ public:
     }
 
     uint64_t get_effective_write_pos() {
-      buffer_appender.flush();
       return pos + buffer.length();
     }
   };
@@ -557,7 +556,6 @@ public:
     ceph_assert(r == 0);
   }
   void try_flush(FileWriter *h) {
-    h->buffer_appender.flush();
     if (h->get_buffer_length() >= cct->_conf->bluefs_min_flush_size) {
       flush(h, true);
     }
diff --git a/src/test/bufferlist.cc b/src/test/bufferlist.cc
index c7dfc80c6e3..37bacee1c61 100644
--- a/src/test/bufferlist.cc
+++ b/src/test/bufferlist.cc
@@ -1609,7 +1609,6 @@ TEST(BufferList, page_aligned_appender) {
   {
     auto a = bl.get_page_aligned_appender(5);
     a.append("asdf", 4);
-    a.flush();
     cout << bl << std::endl;
     ASSERT_EQ(1u, bl.get_num_buffers());
     ASSERT_TRUE(bl.contents_equal("asdf", 4));
@@ -1617,7 +1616,6 @@ TEST(BufferList, page_aligned_appender) {
     for (unsigned n = 0; n < 3 * CEPH_PAGE_SIZE; ++n) {
       a.append("x", 1);
     }
-    a.flush();
     cout << bl << std::endl;
     ASSERT_EQ(1u, bl.get_num_buffers());
     // verify the beginning
@@ -1629,12 +1627,10 @@ TEST(BufferList, page_aligned_appender) {
     for (unsigned n = 0; n < 3 * CEPH_PAGE_SIZE; ++n) {
       a.append("y", 1);
     }
-    a.flush();
     cout << bl << std::endl;
     ASSERT_EQ(2u, bl.get_num_buffers());
 
     a.append_zero(42);
-    a.flush();
     // ensure append_zero didn't introduce a fragmentation
     ASSERT_EQ(2u, bl.get_num_buffers());
     // verify the end is actually zeroed
@@ -1660,7 +1656,6 @@ TEST(BufferList, page_aligned_appender) {
       // moreover, the next C string-taking `append()` had to
       // create anoter `ptr_node` instance but...
       a.append("xyz", 3);
-      a.flush();
       ASSERT_EQ(4u, bl.get_num_buffers());
 
       // ... it should point to the same `buffer::raw` instance
@@ -1675,8 +1670,6 @@ TEST(BufferList, page_aligned_appender) {
     }
   }
 
-  // `flush()` should be called by automatically when destructing
-  // the appender.
   {
     cout << bl << std::endl;
     ASSERT_EQ(6u, bl.get_num_buffers());