From: myoungwon oh Date: Mon, 13 Feb 2023 02:37:21 +0000 (+0900) Subject: crimson/os/seastore/rbm: use do_with() to prevent from freeing allocated memory X-Git-Tag: v18.1.0~271^2~7 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=70d0145c358e2a585e71e26d0f741f9cb6f10a83;p=ceph.git crimson/os/seastore/rbm: use do_with() to prevent from freeing allocated memory Signed-off-by: Myoungwon Oh --- diff --git a/src/crimson/os/seastore/journal/circular_bounded_journal.cc b/src/crimson/os/seastore/journal/circular_bounded_journal.cc index a4e234a8a90d5..1f9762bdbb214 100644 --- 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 e06f2d62622e9..58983956ca2a3 100644 --- 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 c6bca85996e24..6d5f5294d9a22 100644 --- 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 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 { - 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(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 { + 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( - 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(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( + 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 { - 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 { + 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 fada2e7733159..0fb0245a710b1 100644 --- 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 d58af939b7a51..0752265b39ae7 100644 --- 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 74ea2aab8cfd1..21aebcd8e8db5 100644 --- 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;