Merge pull request ceph#59723 from myoungwon/wip-remove-copy-overhead-ool-write

crimson/os/seastore: remove unnecessary memory copy during ool write

Reviewed-by: Yingxin Cheng <[email protected]>

Author: cyx1231st

src/crimson/os/seastore/random_block_manager.h

@@ -122,7 +122,7 @@ class RandomBlockManager {
     crimson::ct_error::enospc,
     crimson::ct_error::erange
     >;
-  virtual write_ertr::future<> write(paddr_t addr, bufferptr &buf) = 0;
+  virtual write_ertr::future<> write(paddr_t addr, bufferptr buf) = 0;
 
   using open_ertr = crimson::errorator<
     crimson::ct_error::input_output_error,

src/crimson/os/seastore/random_block_manager/block_rb_manager.cc

@@ -115,39 +115,41 @@ BlockRBManager::open_ertr::future<> BlockRBManager::open()
   return open_ertr::now();
 }
 
-BlockRBManager::write_ertr::future<> BlockRBManager::write(
-  paddr_t paddr,
-  bufferptr &bptr)
-{
-  LOG_PREFIX(BlockRBManager::write);
-  ceph_assert(device);
-  rbm_abs_addr addr = convert_paddr_to_abs_addr(paddr);
+bool BlockRBManager::check_valid_range(rbm_abs_addr addr, bufferptr &bptr) {
+  LOG_PREFIX(BlockRBManager::check_valid_range);
   rbm_abs_addr start = device->get_shard_start();
   rbm_abs_addr end = device->get_shard_end();
   if (addr < start || addr + bptr.length() > end) {
     ERROR("out of range: start {}, end {}, addr {}, length {}",
       start, end, addr, bptr.length());
+    return false;
+  }
+  return true;
+}
+
+BlockRBManager::write_ertr::future<> BlockRBManager::write(
+  paddr_t paddr,
+  bufferptr bptr)
+{
+  ceph_assert(device);
+  ceph_assert(bptr.is_page_aligned());
+  rbm_abs_addr addr = convert_paddr_to_abs_addr(paddr);
+  if (!check_valid_range(addr, bptr)) {
     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,
-    std::move(bp));
+    bptr);
 }
 
 BlockRBManager::read_ertr::future<> BlockRBManager::read(
   paddr_t paddr,
   bufferptr &bptr)
 {
-  LOG_PREFIX(BlockRBManager::read);
   ceph_assert(device);
+  ceph_assert(bptr.is_page_aligned());
   rbm_abs_addr addr = convert_paddr_to_abs_addr(paddr);
-  rbm_abs_addr start = device->get_shard_start();
-  rbm_abs_addr end = device->get_shard_end();
-  if (addr < start || addr + bptr.length() > end) {
-    ERROR("out of range: start {}, end {}, addr {}, length {}",
-      start, end, addr, bptr.length());
+  if (!check_valid_range(addr, bptr)) {
     return crimson::ct_error::erange::make();
   }
   return device->read(

src/crimson/os/seastore/random_block_manager/block_rb_manager.h

@@ -43,7 +43,7 @@ class BlockRBManager final : public RandomBlockManager {
    */
 
   read_ertr::future<> read(paddr_t addr, bufferptr &buffer) final;
-  write_ertr::future<> write(paddr_t addr, bufferptr &buf) final;
+  write_ertr::future<> write(paddr_t addr, bufferptr buf) final;
   open_ertr::future<> open() final;
   close_ertr::future<> close() final;
 
@@ -127,6 +127,8 @@ class BlockRBManager final : public RandomBlockManager {
     return device->get_journal_size();
   }
 
+  bool check_valid_range(rbm_abs_addr paddr, bufferptr &bptr);
+
 #ifdef UNIT_TESTS_BUILT
   void prefill_fragmented_device() final;
 #endif

src/crimson/os/seastore/random_block_manager/nvme_block_device.cc

@@ -112,7 +112,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 {}",
@@ -127,13 +127,13 @@ write_ertr::future<> NVMeBlockDevice::write(
   }
   if (is_end_to_end_data_protection()) {
     return seastar::do_with(
-      std::move(bptr),
+      bptr,
       [this, offset] (auto &bptr) {
       return nvme_write(offset, bptr.length(), bptr.c_str());
     });
   }
   return seastar::do_with(
-    std::move(bptr),
+    bptr,
     [this, offset, length, supported_stream] (auto& bptr) {
     return io_device[supported_stream].dma_write(
       offset, bptr.c_str(), length).handle_exception(

src/crimson/os/seastore/random_block_manager/nvme_block_device.h

@@ -221,7 +221,7 @@ class NVMeBlockDevice : public RBMDevice {
 
   write_ertr::future<> write(
     uint64_t offset,
-    bufferptr &&bptr,
+    bufferptr bptr,
     uint16_t stream = 0) override;
 
   using RBMDevice::read;

src/crimson/os/seastore/random_block_manager/rbm_device.cc

@@ -93,7 +93,7 @@ write_ertr::future<> RBMDevice::write_rbm_superblock()
   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, std::move(bp));
+  return write(RBM_START_ADDRESS, bp);
 }
 
 read_ertr::future<rbm_superblock_t> RBMDevice::read_rbm_superblock(
@@ -212,7 +212,7 @@ open_ertr::future<> EphemeralRBMDevice::open(
 
 write_ertr::future<> EphemeralRBMDevice::write(
   uint64_t offset,
-  bufferptr &&bptr,
+  bufferptr bptr,
   uint16_t stream) {
   LOG_PREFIX(EphemeralRBMDevice::write);
   ceph_assert(buf);

src/crimson/os/seastore/random_block_manager/rbm_device.h

@@ -128,7 +128,7 @@ class RBMDevice : public Device {
    */
   virtual write_ertr::future<> write(
     uint64_t offset,
-    bufferptr &&bptr,
+    bufferptr bptr,
     uint16_t stream = 0) = 0;
 
   virtual discard_ertr::future<> discard(
@@ -223,7 +223,7 @@ class EphemeralRBMDevice : public RBMDevice {
 
   write_ertr::future<> write(
     uint64_t offset,
-    bufferptr &&bptr,
+    bufferptr bptr,
     uint16_t stream = 0) override;
 
   using RBMDevice::read;

src/test/crimson/seastore/test_randomblock_manager.cc

@@ -107,7 +107,9 @@ struct rbm_test_t :
       std::numeric_limits<char>::max()
     );
     char contents = distribution(generator);
-    return buffer::ptr(buffer::create(blocks * block_size, contents));
+    auto bp = bufferptr(ceph::buffer::create_page_aligned(blocks * block_size));
+    memset(bp.c_str(), contents, bp.length());
+    return bp;
   }
 
   void close() {