crimson/os/seastore/rbm: use do_with() to prevent from freeing allocated memory

Signed-off-by: Myoungwon Oh <myoungwon.oh@samsung.com>

diff --git a/src/crimson/os/seastore/journal/circular_bounded_journal.cc b/src/crimson/os/seastore/journal/circular_bounded_journal.cc
index a4e234a8a90d53869d1c897692169f4d645d7dfc..1f9762bdbb214d4e5e4edb6f9a2cebab49f59d1c 100644 (file)
--- a/src/crimson/os/seastore/journal/circular_bounded_journal.cc
+++ b/src/crimson/os/seastore/journal/circular_bounded_journal.cc
@@ -502,19 +502,21 @@ CircularBoundedJournal::write_ertr::future<>
 CircularBoundedJournal::write_header()
 {
   LOG_PREFIX(CircularBoundedJournal::write_header);
-  ceph::bufferlist bl;
-  try {
-    bl = encode_header();
-  } catch (ceph::buffer::error &e) {
-    DEBUG("unable to encode header block from underlying deivce");
-    return crimson::ct_error::input_output_error::make();
-  }
+  ceph::bufferlist bl = encode_header();
   ceph_assert(bl.length() <= get_block_size());
   DEBUG(
     "sync header of CircularBoundedJournal, length {}",
     bl.length());
   assert(device);
-  return device_write_bl(device->get_journal_start(), bl);
+  auto iter = bl.begin();
+  assert(bl.length() < get_block_size());
+  bufferptr bp = bufferptr(ceph::buffer::create_page_aligned(get_block_size()));
+  iter.copy(bl.length(), bp.c_str());
+  return device->write(device->get_journal_start(), std::move(bp)
+  ).handle_error(
+    write_ertr::pass_further{},
+    crimson::ct_error::assert_all{ "Invalid error device->write" }
+  );
 }
 seastar::future<> CircularBoundedJournal::finish_commit(transaction_type_t type) {
   if (is_trim_transaction(type)) {

diff --git a/src/crimson/os/seastore/random_block_manager/block_rb_manager.cc b/src/crimson/os/seastore/random_block_manager/block_rb_manager.cc
index e06f2d62622e96787ca3cf936f8b1aa69d8971b6..58983956ca2a3aaa4717f904befadcff8d9cb1ad 100644 (file)
--- a/src/crimson/os/seastore/random_block_manager/block_rb_manager.cc
+++ b/src/crimson/os/seastore/random_block_manager/block_rb_manager.cc
@@ -105,9 +105,11 @@ BlockRBManager::write_ertr::future<> BlockRBManager::write(
       start, end, addr, bptr.length());
     return crimson::ct_error::erange::make();
   }
+  bufferptr bp = bufferptr(ceph::buffer::create_page_aligned(bptr.length()));
+  bp.copy_in(0, bptr.length(), bptr.c_str());
   return device->write(
     addr,
-    bptr);
+    std::move(bp));
 }
 
 BlockRBManager::read_ertr::future<> BlockRBManager::read(
@@ -153,7 +155,7 @@ BlockRBManager::write_ertr::future<> BlockRBManager::write(
   }
   return device->write(
     addr,
-    bptr);
+    std::move(bptr));
 }
 
 std::ostream &operator<<(std::ostream &out, const rbm_metadata_header_t &header)

diff --git a/src/crimson/os/seastore/random_block_manager/nvme_block_device.cc b/src/crimson/os/seastore/random_block_manager/nvme_block_device.cc
index c6bca85996e24b7c5c8e09c0aa527be803b25eb6..6d5f5294d9a22d4070b07e78e235e972e8e316cb 100644 (file)
--- a/src/crimson/os/seastore/random_block_manager/nvme_block_device.cc
+++ b/src/crimson/os/seastore/random_block_manager/nvme_block_device.cc
@@ -79,58 +79,58 @@ write_ertr::future<> RBMDevice::write_rbm_header()
   auto bp = bufferptr(ceph::buffer::create_page_aligned(super.block_size));
   assert(bl.length() < super.block_size);
   iter.copy(bl.length(), bp.c_str());
-
-  return write(RBM_START_ADDRESS, bp);
+  return write(RBM_START_ADDRESS, std::move(bp));
 }
 
 read_ertr::future<rbm_metadata_header_t> RBMDevice::read_rbm_header(
   rbm_abs_addr addr)
 {
   LOG_PREFIX(RBMDevice::read_rbm_header);
-  bufferptr bptr =
-    bufferptr(ceph::buffer::create_page_aligned(RBM_SUPERBLOCK_SIZE));
-  bptr.zero();
-  return read(
-    addr,
-    bptr
-  ).safe_then([length=bptr.length(), this, bptr, FNAME]()
-    -> read_ertr::future<rbm_metadata_header_t> {
-    bufferlist bl;
-    bl.append(bptr);
-    auto p = bl.cbegin();
-    rbm_metadata_header_t super_block;
-    try {
-      decode(super_block, p);
-    }
-    catch (ceph::buffer::error& e) {
-      DEBUG("read_rbm_header: unable to decode rbm super block {}",
-           e.what());
-      return crimson::ct_error::enoent::make();
-    }
-    checksum_t crc = super_block.crc;
-    bufferlist meta_b_header;
-    super_block.crc = 0;
-    encode(super_block, meta_b_header);
-    assert(ceph::encoded_sizeof<rbm_metadata_header_t>(super_block) <
-       super_block.block_size);
-
-    // Do CRC verification only if data protection is not supported.
-    if (is_data_protection_enabled() == false) {
-      if (meta_b_header.crc32c(-1) != crc) {
-       DEBUG("bad crc on super block, expected {} != actual {} ",
-             meta_b_header.crc32c(-1), crc);
-       return crimson::ct_error::input_output_error::make();
+  return seastar::do_with(
+    bufferptr(ceph::buffer::create_page_aligned(super.block_size)),
+    [this, addr, FNAME](auto &bptr) {
+    return read(
+      addr,
+      bptr
+    ).safe_then([length=bptr.length(), this, bptr, FNAME]()
+      -> read_ertr::future<rbm_metadata_header_t> {
+      bufferlist bl;
+      bl.append(bptr);
+      auto p = bl.cbegin();
+      rbm_metadata_header_t super_block;
+      try {
+       decode(super_block, p);
       }
-    } else {
-      ceph_assert_always(crc == (checksum_t)-1);
-    }
-    super_block.crc = crc;
-    super = super_block;
-    DEBUG("got {} ", super);
-    return read_ertr::future<rbm_metadata_header_t>(
-      read_ertr::ready_future_marker{},
-      super_block
-    );
+      catch (ceph::buffer::error& e) {
+       DEBUG("read_rbm_header: unable to decode rbm super block {}",
+             e.what());
+       return crimson::ct_error::enoent::make();
+      }
+      checksum_t crc = super_block.crc;
+      bufferlist meta_b_header;
+      super_block.crc = 0;
+      encode(super_block, meta_b_header);
+      assert(ceph::encoded_sizeof<rbm_metadata_header_t>(super_block) <
+         super_block.block_size);
+
+      // Do CRC verification only if data protection is not supported.
+      if (is_data_protection_enabled() == false) {
+       if (meta_b_header.crc32c(-1) != crc) {
+         DEBUG("bad crc on super block, expected {} != actual {} ",
+               meta_b_header.crc32c(-1), crc);
+         return crimson::ct_error::input_output_error::make();
+       }
+      } else {
+       ceph_assert_always(crc == (checksum_t)-1);
+      }
+      super_block.crc = crc;
+      super = super_block;
+      DEBUG("got {} ", super);
+      return read_ertr::future<rbm_metadata_header_t>(
+       read_ertr::ready_future_marker{},
+       super_block
+      );
+    });
   });
 }
 
@@ -215,7 +215,7 @@ NVMeBlockDevice::mount_ret NVMeBlockDevice::mount()
 
 write_ertr::future<> NVMeBlockDevice::write(
   uint64_t offset,
-  bufferptr &bptr,
+  bufferptr &&bptr,
   uint16_t stream) {
   logger().debug(
       "block: write offset {} len {}",
@@ -228,17 +228,21 @@ write_ertr::future<> NVMeBlockDevice::write(
   if (stream >= stream_id_count) {
     supported_stream = WRITE_LIFE_NOT_SET;
   }
-  return io_device[supported_stream].dma_write(
-    offset, bptr.c_str(), length).handle_exception(
-    [](auto e) -> write_ertr::future<size_t> {
-    logger().error("write: dma_write got error{}", e);
-    return crimson::ct_error::input_output_error::make();
-  }).then([length](auto result) -> write_ertr::future<> {
-    if (result != length) {
-      logger().error("write: dma_write got error with not proper length");
+  return seastar::do_with(
+    std::move(bptr),
+    [this, offset, length, supported_stream] (auto& bptr) {
+    return io_device[supported_stream].dma_write(
+      offset, bptr.c_str(), length).handle_exception(
+      [](auto e) -> write_ertr::future<size_t> {
+      logger().error("write: dma_write got error{}", e);
       return crimson::ct_error::input_output_error::make();
-    }
-    return write_ertr::now();
+    }).then([length](auto result) -> write_ertr::future<> {
+      if (result != length) {
+       logger().error("write: dma_write got error with not proper length");
+       return crimson::ct_error::input_output_error::make();
+      }
+      return write_ertr::now();
+    });
   });
 }
 
@@ -419,7 +423,7 @@ open_ertr::future<> EphemeralRBMDevice::open(
 
 write_ertr::future<> EphemeralRBMDevice::write(
   uint64_t offset,
-  bufferptr &bptr,
+  bufferptr &&bptr,
   uint16_t stream) {
   ceph_assert(buf);
   logger().debug(

diff --git a/src/crimson/os/seastore/random_block_manager/nvme_block_device.h b/src/crimson/os/seastore/random_block_manager/nvme_block_device.h
index fada2e77331595194122928e1c92a818333c6e28..0fb0245a710b1870866f20b76789d8cf4a099d11 100644 (file)
--- a/src/crimson/os/seastore/random_block_manager/nvme_block_device.h
+++ b/src/crimson/os/seastore/random_block_manager/nvme_block_device.h
@@ -193,7 +193,7 @@ public:
 
   write_ertr::future<> write(
     uint64_t offset,
-    bufferptr &bptr,
+    bufferptr &&bptr,
     uint16_t stream = 0) override;
 
   using RBMDevice::read;

diff --git a/src/crimson/os/seastore/random_block_manager/rbm_device.h b/src/crimson/os/seastore/random_block_manager/rbm_device.h
index d58af939b7a5126af794f98931e0d1f501b6285e..0752265b39ae702c03150c9c43d6d7c72ad8172e 100644 (file)
--- a/src/crimson/os/seastore/random_block_manager/rbm_device.h
+++ b/src/crimson/os/seastore/random_block_manager/rbm_device.h
@@ -132,7 +132,7 @@ public:
    */
   virtual write_ertr::future<> write(
     uint64_t offset,
-    bufferptr &bptr,
+    bufferptr &&bptr,
     uint16_t stream = 0) = 0;
 
   virtual discard_ertr::future<> discard(
@@ -225,7 +225,7 @@ public:
 
   write_ertr::future<> write(
     uint64_t offset,
-    bufferptr &bptr,
+    bufferptr &&bptr,
     uint16_t stream = 0) override;
 
   using RBMDevice::read;

diff --git a/src/test/crimson/seastore/nvmedevice/test_nvmedevice.cc b/src/test/crimson/seastore/nvmedevice/test_nvmedevice.cc
index 74ea2aab8cfd1a1036545e930a98ae187936220a..21aebcd8e8db5b26eba8cc659dc6791021b8b522 100644 (file)
--- a/src/test/crimson/seastore/nvmedevice/test_nvmedevice.cc
+++ b/src/test/crimson/seastore/nvmedevice/test_nvmedevice.cc
@@ -69,7 +69,7 @@ TEST_F(nvdev_test_t, write_and_verify_test)
       bl_length = bl.length();
       auto write_buf = ceph::bufferptr(buffer::create_page_aligned(BLK_SIZE));
       bl.begin().copy(bl_length, write_buf.c_str());
-      device->write(0, write_buf).unsafe_get();
+      device->write(0, std::move(write_buf)).unsafe_get();
     }
 
     nvdev_test_block_t read_data;