From ac077112d635aff687b2164ca669a1abce34f860 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Thu, 15 Feb 2018 12:19:28 -0500 Subject: [PATCH] librbd: use move-semantics for object IO data payloads Signed-off-by: Jason Dillaman --- src/librbd/LibrbdWriteback.cc | 20 ++++---- src/librbd/io/ImageRequest.cc | 15 ++++-- src/librbd/io/ObjectRequest.cc | 20 ++++---- src/librbd/io/ObjectRequest.h | 22 ++++----- src/librbd/operation/FlattenRequest.cc | 3 +- src/test/librbd/io/test_mock_ObjectRequest.cc | 46 +++++++++---------- 6 files changed, 69 insertions(+), 57 deletions(-) diff --git a/src/librbd/LibrbdWriteback.cc b/src/librbd/LibrbdWriteback.cc index 7bc90cadefb85..bcb71298cd159 100644 --- a/src/librbd/LibrbdWriteback.cc +++ b/src/librbd/LibrbdWriteback.cc @@ -100,6 +100,7 @@ namespace librbd { std::string oid; uint64_t object_no; uint64_t off; + uint64_t length; bufferlist bl; SnapContext snapc; uint64_t journal_tid; @@ -109,12 +110,12 @@ namespace librbd { C_WriteJournalCommit(ImageCtx *_image_ctx, const std::string &_oid, uint64_t _object_no, uint64_t _off, - const bufferlist &_bl, const SnapContext& _snapc, + bufferlist&& _bl, const SnapContext& _snapc, uint64_t _journal_tid, const ZTracer::Trace &trace, Context *_req_comp) : image_ctx(_image_ctx), oid(_oid), object_no(_object_no), off(_off), - bl(_bl), snapc(_snapc), journal_tid(_journal_tid), - trace(trace), req_comp(_req_comp) { + length(_bl.length()), bl(std::move(_bl)), snapc(_snapc), + journal_tid(_journal_tid), trace(trace), req_comp(_req_comp) { CephContext *cct = image_ctx->cct; ldout(cct, 20) << this << " C_WriteJournalCommit: " << "delaying write until journal tid " @@ -150,7 +151,7 @@ namespace librbd { Extents file_extents; Striper::extent_to_file(cct, &image_ctx->layout, object_no, off, - bl.length(), file_extents); + length, file_extents); for (Extents::iterator it = file_extents.begin(); it != file_extents.end(); ++it) { image_ctx->journal->commit_io_event_extent(journal_tid, it->first, @@ -167,7 +168,8 @@ namespace librbd { request_sent = true; auto req = new io::ObjectWriteRequest<>(image_ctx, oid, object_no, off, - bl, snapc, 0, trace, this); + std::move(bl), snapc, 0, trace, + this); req->send(); } }; @@ -281,14 +283,16 @@ namespace librbd { // all IO operations are flushed prior to closing the journal assert(journal_tid == 0 || m_ictx->journal != NULL); + bufferlist bl_copy(bl); if (journal_tid != 0) { m_ictx->journal->flush_event( journal_tid, new C_WriteJournalCommit( - m_ictx, oid.name, object_no, off, bl, snapc, journal_tid, trace, - req_comp)); + m_ictx, oid.name, object_no, off, std::move(bl_copy), snapc, + journal_tid, trace, req_comp)); } else { auto req = new io::ObjectWriteRequest<>( - m_ictx, oid.name, object_no, off, bl, snapc, 0, trace, req_comp); + m_ictx, oid.name, object_no, off, std::move(bl_copy), snapc, 0, trace, + req_comp); req->send(); } return ++m_tid; diff --git a/src/librbd/io/ImageRequest.cc b/src/librbd/io/ImageRequest.cc index fdd7b18db438a..b34a0ff247abf 100644 --- a/src/librbd/io/ImageRequest.cc +++ b/src/librbd/io/ImageRequest.cc @@ -478,7 +478,8 @@ ObjectRequestHandle *ImageWriteRequest::create_object_request( assemble_extent(object_extent, &bl); ObjectRequest *req = ObjectRequest::create_write( &image_ctx, object_extent.oid.name, object_extent.objectno, - object_extent.offset, bl, snapc, m_op_flags, this->m_trace, on_finish); + object_extent.offset, std::move(bl), snapc, m_op_flags, this->m_trace, + on_finish); return req; } @@ -781,12 +782,13 @@ ObjectRequestHandle *ImageWriteSameRequest::create_object_request( req = ObjectRequest::create_writesame( &image_ctx, object_extent.oid.name, object_extent.objectno, object_extent.offset, object_extent.length, - bl, snapc, m_op_flags, this->m_trace, on_finish); + std::move(bl), snapc, m_op_flags, this->m_trace, on_finish); return req; } req = ObjectRequest::create_write( &image_ctx, object_extent.oid.name, object_extent.objectno, - object_extent.offset, bl, snapc, m_op_flags, this->m_trace, on_finish); + object_extent.offset, std::move(bl), snapc, m_op_flags, this->m_trace, + on_finish); return req; } @@ -862,13 +864,16 @@ ObjectRequestHandle *ImageCompareAndWriteRequest::create_object_request( Context *on_finish) { I &image_ctx = this->m_image_ctx; + // NOTE: safe to move m_cmp_bl since we only support this op against + // a single object bufferlist bl; assemble_extent(object_extent, &bl); ObjectRequest *req = ObjectRequest::create_compare_and_write( &image_ctx, object_extent.oid.name, object_extent.objectno, object_extent.offset, - m_cmp_bl, bl, snapc, m_mismatch_offset, - m_op_flags, this->m_trace, on_finish); + std::move(m_cmp_bl), std::move(bl), snapc, + m_mismatch_offset, m_op_flags, this->m_trace, + on_finish); return req; } diff --git a/src/librbd/io/ObjectRequest.cc b/src/librbd/io/ObjectRequest.cc index 15babcaeff655..da35c001181ed 100644 --- a/src/librbd/io/ObjectRequest.cc +++ b/src/librbd/io/ObjectRequest.cc @@ -48,12 +48,13 @@ template ObjectRequest* ObjectRequest::create_write(I *ictx, const std::string &oid, uint64_t object_no, uint64_t object_off, - const ceph::bufferlist &data, + ceph::bufferlist&& data, const ::SnapContext &snapc, int op_flags, const ZTracer::Trace &parent_trace, Context *completion) { - return new ObjectWriteRequest(ictx, oid, object_no, object_off, data, - snapc, op_flags, parent_trace, completion); + return new ObjectWriteRequest(ictx, oid, object_no, object_off, + std::move(data), snapc, op_flags, + parent_trace, completion); } template @@ -77,13 +78,13 @@ ObjectRequest* ObjectRequest::create_writesame(I *ictx, const std::string &oid, uint64_t object_no, uint64_t object_off, uint64_t object_len, - const ceph::bufferlist &data, + ceph::bufferlist&& data, const ::SnapContext &snapc, int op_flags, const ZTracer::Trace &parent_trace, Context *completion) { return new ObjectWriteSameRequest(ictx, oid, object_no, object_off, - object_len, data, snapc, op_flags, - parent_trace, completion); + object_len, std::move(data), snapc, + op_flags, parent_trace, completion); } template @@ -91,15 +92,16 @@ ObjectRequest* ObjectRequest::create_compare_and_write(I *ictx, const std::string &oid, uint64_t object_no, uint64_t object_off, - const ceph::bufferlist &cmp_data, - const ceph::bufferlist &write_data, + ceph::bufferlist&& cmp_data, + ceph::bufferlist&& write_data, const ::SnapContext &snapc, uint64_t *mismatch_offset, int op_flags, const ZTracer::Trace &parent_trace, Context *completion) { return new ObjectCompareAndWriteRequest(ictx, oid, object_no, object_off, - cmp_data, write_data, snapc, + std::move(cmp_data), + std::move(write_data), snapc, mismatch_offset, op_flags, parent_trace, completion); } diff --git a/src/librbd/io/ObjectRequest.h b/src/librbd/io/ObjectRequest.h index a015fb199ec95..b13a195f85eaa 100644 --- a/src/librbd/io/ObjectRequest.h +++ b/src/librbd/io/ObjectRequest.h @@ -43,7 +43,7 @@ public: static ObjectRequest* create_write(ImageCtxT *ictx, const std::string &oid, uint64_t object_no, uint64_t object_off, - const ceph::bufferlist &data, + ceph::bufferlist&& data, const ::SnapContext &snapc, int op_flags, const ZTracer::Trace &parent_trace, Context *completion); @@ -60,7 +60,7 @@ public: uint64_t object_no, uint64_t object_off, uint64_t object_len, - const ceph::bufferlist &data, + ceph::bufferlist&& data, const ::SnapContext &snapc, int op_flags, const ZTracer::Trace &parent_trace, @@ -69,8 +69,8 @@ public: const std::string &oid, uint64_t object_no, uint64_t object_off, - const ceph::bufferlist &cmp_data, - const ceph::bufferlist &write_data, + ceph::bufferlist&& cmp_data, + ceph::bufferlist&& write_data, const ::SnapContext &snapc, uint64_t *mismatch_offset, int op_flags, const ZTracer::Trace &parent_trace, @@ -309,13 +309,13 @@ class ObjectWriteRequest : public AbstractObjectWriteRequest { public: ObjectWriteRequest(ImageCtxT *ictx, const std::string &oid, uint64_t object_no, uint64_t object_off, - const ceph::bufferlist &data, const ::SnapContext &snapc, + ceph::bufferlist&& data, const ::SnapContext &snapc, int op_flags, const ZTracer::Trace &parent_trace, Context *completion) : AbstractObjectWriteRequest(ictx, oid, object_no, object_off, data.length(), snapc, "write", parent_trace, completion), - m_write_data(data), m_op_flags(op_flags) { + m_write_data(std::move(data)), m_op_flags(op_flags) { } bool is_empty_write_op() const override { @@ -434,14 +434,14 @@ class ObjectWriteSameRequest : public AbstractObjectWriteRequest { public: ObjectWriteSameRequest(ImageCtxT *ictx, const std::string &oid, uint64_t object_no, uint64_t object_off, - uint64_t object_len, const ceph::bufferlist &data, + uint64_t object_len, ceph::bufferlist&& data, const ::SnapContext &snapc, int op_flags, const ZTracer::Trace &parent_trace, Context *completion) : AbstractObjectWriteRequest(ictx, oid, object_no, object_off, object_len, snapc, "writesame", parent_trace, completion), - m_write_data(data), m_op_flags(op_flags) { + m_write_data(std::move(data)), m_op_flags(op_flags) { } const char *get_op_type() const override { @@ -461,8 +461,8 @@ class ObjectCompareAndWriteRequest : public AbstractObjectWriteRequest(&image_ctx, oid, m_object_no, 0, - bl, m_snapc, 0, {}, this); + std::move(bl), m_snapc, 0, {}, + this); if (!req->has_parent()) { // stop early if the parent went away - it just means // another flatten finished first or the image was resized diff --git a/src/test/librbd/io/test_mock_ObjectRequest.cc b/src/test/librbd/io/test_mock_ObjectRequest.cc index 09c48f0dc70b8..a9f8df05eba5a 100644 --- a/src/test/librbd/io/test_mock_ObjectRequest.cc +++ b/src/test/librbd/io/test_mock_ObjectRequest.cc @@ -667,8 +667,8 @@ TEST_F(TestMockIoObjectRequest, Write) { C_SaferCond ctx; auto req = MockObjectWriteRequest::create_write( - &mock_image_ctx, ictx->get_object_name(0), 0, 0, bl, mock_image_ctx.snapc, - 0, {}, &ctx); + &mock_image_ctx, ictx->get_object_name(0), 0, 0, std::move(bl), + mock_image_ctx.snapc, 0, {}, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } @@ -702,8 +702,8 @@ TEST_F(TestMockIoObjectRequest, WriteFull) { C_SaferCond ctx; auto req = MockObjectWriteRequest::create_write( - &mock_image_ctx, ictx->get_object_name(0), 0, 0, bl, mock_image_ctx.snapc, - 0, {}, &ctx); + &mock_image_ctx, ictx->get_object_name(0), 0, 0, std::move(bl), + mock_image_ctx.snapc, 0, {}, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } @@ -736,8 +736,8 @@ TEST_F(TestMockIoObjectRequest, WriteObjectMap) { C_SaferCond ctx; auto req = MockObjectWriteRequest::create_write( - &mock_image_ctx, ictx->get_object_name(0), 0, 0, bl, mock_image_ctx.snapc, - 0, {}, &ctx); + &mock_image_ctx, ictx->get_object_name(0), 0, 0, std::move(bl), + mock_image_ctx.snapc, 0, {}, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } @@ -758,8 +758,8 @@ TEST_F(TestMockIoObjectRequest, WriteError) { C_SaferCond ctx; auto req = MockObjectWriteRequest::create_write( - &mock_image_ctx, ictx->get_object_name(0), 0, 0, bl, mock_image_ctx.snapc, - 0, {}, &ctx); + &mock_image_ctx, ictx->get_object_name(0), 0, 0, std::move(bl), + mock_image_ctx.snapc, 0, {}, &ctx); req->send(); ASSERT_EQ(-EPERM, ctx.wait()); } @@ -812,8 +812,8 @@ TEST_F(TestMockIoObjectRequest, Copyup) { C_SaferCond ctx; auto req = MockObjectWriteRequest::create_write( - &mock_image_ctx, ictx->get_object_name(0), 0, 0, bl, mock_image_ctx.snapc, - 0, {}, &ctx); + &mock_image_ctx, ictx->get_object_name(0), 0, 0, std::move(bl), + mock_image_ctx.snapc, 0, {}, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } @@ -860,8 +860,8 @@ TEST_F(TestMockIoObjectRequest, CopyupOptimization) { C_SaferCond ctx; auto req = MockObjectWriteRequest::create_write( - &mock_image_ctx, ictx->get_object_name(0), 0, 0, bl, mock_image_ctx.snapc, - 0, {}, &ctx); + &mock_image_ctx, ictx->get_object_name(0), 0, 0, std::move(bl), + mock_image_ctx.snapc, 0, {}, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } @@ -901,8 +901,8 @@ TEST_F(TestMockIoObjectRequest, CopyupError) { C_SaferCond ctx; auto req = MockObjectWriteRequest::create_write( - &mock_image_ctx, ictx->get_object_name(0), 0, 0, bl, mock_image_ctx.snapc, - 0, {}, &ctx); + &mock_image_ctx, ictx->get_object_name(0), 0, 0, std::move(bl), + mock_image_ctx.snapc, 0, {}, &ctx); req->send(); ASSERT_EQ(-EPERM, ctx.wait()); } @@ -1143,7 +1143,7 @@ TEST_F(TestMockIoObjectRequest, WriteSame) { C_SaferCond ctx; auto req = MockObjectWriteSameRequest::create_writesame( - &mock_image_ctx, ictx->get_object_name(0), 0, 0, 4096, bl, + &mock_image_ctx, ictx->get_object_name(0), 0, 0, 4096, std::move(bl), mock_image_ctx.snapc, 0, {}, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); @@ -1183,8 +1183,8 @@ TEST_F(TestMockIoObjectRequest, CompareAndWrite) { C_SaferCond ctx; uint64_t mismatch_offset; auto req = MockObjectWriteSameRequest::create_compare_and_write( - &mock_image_ctx, ictx->get_object_name(0), 0, 0, cmp_bl, bl, - mock_image_ctx.snapc, &mismatch_offset, 0, {}, &ctx); + &mock_image_ctx, ictx->get_object_name(0), 0, 0, std::move(cmp_bl), + std::move(bl), mock_image_ctx.snapc, &mismatch_offset, 0, {}, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } @@ -1223,8 +1223,8 @@ TEST_F(TestMockIoObjectRequest, CompareAndWriteFull) { C_SaferCond ctx; uint64_t mismatch_offset; auto req = MockObjectWriteSameRequest::create_compare_and_write( - &mock_image_ctx, ictx->get_object_name(0), 0, 0, cmp_bl, bl, - mock_image_ctx.snapc, &mismatch_offset, 0, {}, &ctx); + &mock_image_ctx, ictx->get_object_name(0), 0, 0, std::move(cmp_bl), + std::move(bl), mock_image_ctx.snapc, &mismatch_offset, 0, {}, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } @@ -1285,8 +1285,8 @@ TEST_F(TestMockIoObjectRequest, CompareAndWriteCopyup) { C_SaferCond ctx; uint64_t mismatch_offset; auto req = MockObjectWriteSameRequest::create_compare_and_write( - &mock_image_ctx, ictx->get_object_name(0), 0, 0, cmp_bl, bl, - mock_image_ctx.snapc, &mismatch_offset, 0, {}, &ctx); + &mock_image_ctx, ictx->get_object_name(0), 0, 0, std::move(cmp_bl), + std::move(bl), mock_image_ctx.snapc, &mismatch_offset, 0, {}, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } @@ -1324,8 +1324,8 @@ TEST_F(TestMockIoObjectRequest, CompareAndWriteMismatch) { C_SaferCond ctx; uint64_t mismatch_offset; auto req = MockObjectWriteSameRequest::create_compare_and_write( - &mock_image_ctx, ictx->get_object_name(0), 0, 0, cmp_bl, bl, - mock_image_ctx.snapc, &mismatch_offset, 0, {}, &ctx); + &mock_image_ctx, ictx->get_object_name(0), 0, 0, std::move(cmp_bl), + std::move(bl), mock_image_ctx.snapc, &mismatch_offset, 0, {}, &ctx); req->send(); ASSERT_EQ(-EILSEQ, ctx.wait()); ASSERT_EQ(1ULL, mismatch_offset); -- 2.39.5