From: Jason Dillaman Date: Thu, 15 Feb 2018 21:37:48 +0000 (-0500) Subject: librbd: decoupled object read requests from result assembler X-Git-Tag: v13.0.2~184^2~1 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=c2d47e7df104c2bb8715e4a4dd38e6af3179e247;p=ceph.git librbd: decoupled object read requests from result assembler Signed-off-by: Jason Dillaman --- diff --git a/src/librbd/LibrbdWriteback.cc b/src/librbd/LibrbdWriteback.cc index bcb71298cd159..d0650e2b55301 100644 --- a/src/librbd/LibrbdWriteback.cc +++ b/src/librbd/LibrbdWriteback.cc @@ -223,12 +223,12 @@ namespace librbd { aio_comp->read_result = io::ReadResult{pbl}; aio_comp->set_request_count(1); - auto req_comp = new io::ReadResult::C_SparseReadRequest<>( - aio_comp, {{0, len}}, false); + auto req_comp = new io::ReadResult::C_ObjectReadRequest( + aio_comp, off, len, {{0, len}}, false); auto req = io::ObjectReadRequest<>::create(m_ictx, oid.name, object_no, off, len, snapid, op_flags, true, - trace, req_comp); - req_comp->request = req; + trace, &req_comp->bl, + &req_comp->extent_map, req_comp); req->send(); } diff --git a/src/librbd/io/ImageRequest.cc b/src/librbd/io/ImageRequest.cc index b34a0ff247abf..85872004bb983 100644 --- a/src/librbd/io/ImageRequest.cc +++ b/src/librbd/io/ImageRequest.cc @@ -259,12 +259,13 @@ void ImageReadRequest::send_request() { << extent.length << " from " << extent.buffer_extents << dendl; - auto req_comp = new io::ReadResult::C_SparseReadRequest( - aio_comp, std::move(extent.buffer_extents), true); + auto req_comp = new io::ReadResult::C_ObjectReadRequest( + aio_comp, extent.offset, extent.length, + std::move(extent.buffer_extents), true); ObjectReadRequest *req = ObjectReadRequest::create( &image_ctx, extent.oid.name, extent.objectno, extent.offset, - extent.length, snap_id, m_op_flags, false, this->m_trace, req_comp); - req_comp->request = req; + extent.length, snap_id, m_op_flags, false, this->m_trace, + &req_comp->bl, &req_comp->extent_map, req_comp); req->send(); } } diff --git a/src/librbd/io/ObjectRequest.cc b/src/librbd/io/ObjectRequest.cc index da35c001181ed..eec6eb06cb201 100644 --- a/src/librbd/io/ObjectRequest.cc +++ b/src/librbd/io/ObjectRequest.cc @@ -186,10 +186,13 @@ ObjectReadRequest::ObjectReadRequest(I *ictx, const std::string &oid, uint64_t len, librados::snap_t snap_id, int op_flags, bool cache_initiated, const ZTracer::Trace &parent_trace, + bufferlist* read_data, + ExtentMap* extent_map, Context *completion) : ObjectRequest(ictx, oid, objectno, offset, len, snap_id, "read", parent_trace, completion), - m_op_flags(op_flags), m_cache_initiated(cache_initiated) { + m_op_flags(op_flags), m_cache_initiated(cache_initiated), + m_read_data(read_data), m_extent_map(extent_map) { } template @@ -214,7 +217,7 @@ void ObjectReadRequest::read_cache() { *image_ctx, util::create_context_callback< ObjectReadRequest, &ObjectReadRequest::handle_read_cache>(this)); image_ctx->aio_read_from_cache( - this->m_oid, this->m_object_no, &m_read_data, this->m_object_len, + this->m_oid, this->m_object_no, m_read_data, this->m_object_len, this->m_object_off, cache_ctx, m_op_flags, (this->m_trace.valid() ? &this->m_trace : nullptr)); } @@ -255,10 +258,10 @@ void ObjectReadRequest::read_object() { librados::ObjectReadOperation op; if (this->m_object_len >= image_ctx->sparse_read_threshold_bytes) { - op.sparse_read(this->m_object_off, this->m_object_len, &m_ext_map, - &m_read_data, nullptr); + op.sparse_read(this->m_object_off, this->m_object_len, m_extent_map, + m_read_data, nullptr); } else { - op.read(this->m_object_off, this->m_object_len, &m_read_data, nullptr); + op.read(this->m_object_off, this->m_object_len, m_read_data, nullptr); } op.set_op_flags2(m_op_flags); @@ -329,7 +332,7 @@ void ObjectReadRequest::read_parent() { ObjectReadRequest, &ObjectReadRequest::handle_read_parent>( this, util::get_image_ctx(image_ctx->parent), AIO_TYPE_READ); ImageRequest::aio_read(image_ctx->parent, parent_completion, - std::move(parent_extents), ReadResult{&m_read_data}, + std::move(parent_extents), ReadResult{m_read_data}, 0, this->m_trace); } diff --git a/src/librbd/io/ObjectRequest.h b/src/librbd/io/ObjectRequest.h index b13a195f85eaa..be319cb802af3 100644 --- a/src/librbd/io/ObjectRequest.h +++ b/src/librbd/io/ObjectRequest.h @@ -126,33 +126,23 @@ public: uint64_t len, librados::snap_t snap_id, int op_flags, bool cache_initiated, const ZTracer::Trace &parent_trace, - Context *completion) { + ceph::bufferlist* read_data, + ExtentMap* extent_map, Context *completion) { return new ObjectReadRequest(ictx, oid, objectno, offset, len, snap_id, op_flags, cache_initiated, - parent_trace, completion); + parent_trace, read_data, extent_map, + completion); } ObjectReadRequest(ImageCtxT *ictx, const std::string &oid, uint64_t objectno, uint64_t offset, uint64_t len, librados::snap_t snap_id, int op_flags, bool cache_initiated, const ZTracer::Trace &parent_trace, + ceph::bufferlist* read_data, ExtentMap* extent_map, Context *completion); void send() override; - inline uint64_t get_offset() const { - return this->m_object_off; - } - inline uint64_t get_length() const { - return this->m_object_len; - } - ceph::bufferlist &data() { - return m_read_data; - } - ExtentMap &get_extent_map() { - return m_ext_map; - } - const char *get_op_type() const override { return "read"; } @@ -187,8 +177,8 @@ private: int m_op_flags; bool m_cache_initiated; - ceph::bufferlist m_read_data; - ExtentMap m_ext_map; + ceph::bufferlist* m_read_data; + ExtentMap* m_extent_map; void read_cache(); void handle_read_cache(int r); diff --git a/src/librbd/io/ReadResult.cc b/src/librbd/io/ReadResult.cc index f4d78562b56b0..5a4dc18e9b612 100644 --- a/src/librbd/io/ReadResult.cc +++ b/src/librbd/io/ReadResult.cc @@ -79,15 +79,12 @@ struct ReadResult::AssembleResultVisitor : public boost::static_visitor { } }; -ReadResult::C_ReadRequest::C_ReadRequest(AioCompletion *aio_completion) - : aio_completion(aio_completion) { +ReadResult::C_ImageReadRequest::C_ImageReadRequest( + AioCompletion *aio_completion, const Extents image_extents) + : aio_completion(aio_completion), image_extents(image_extents) { aio_completion->add_request(); } -void ReadResult::C_ReadRequest::finish(int r) { - aio_completion->complete_request(r); -} - void ReadResult::C_ImageReadRequest::finish(int r) { CephContext *cct = aio_completion->ictx->cct; ldout(cct, 10) << "C_ImageReadRequest: r=" << r @@ -106,15 +103,21 @@ void ReadResult::C_ImageReadRequest::finish(int r) { r = length; } - C_ReadRequest::finish(r); + aio_completion->complete_request(r); } -void ReadResult::C_SparseReadRequestBase::finish(ExtentMap &extent_map, - const Extents &buffer_extents, - uint64_t offset, size_t length, - bufferlist &bl, int r) { +ReadResult::C_ObjectReadRequest::C_ObjectReadRequest( + AioCompletion *aio_completion, uint64_t object_off, uint64_t object_len, + Extents&& buffer_extents, bool ignore_enoent) + : aio_completion(aio_completion), object_off(object_off), + object_len(object_len), buffer_extents(std::move(buffer_extents)), + ignore_enoent(ignore_enoent) { + aio_completion->add_request(); +} + +void ReadResult::C_ObjectReadRequest::finish(int r) { CephContext *cct = aio_completion->ictx->cct; - ldout(cct, 10) << "C_SparseReadRequestBase: r = " << r + ldout(cct, 10) << "C_ObjectReadRequest: r=" << r << dendl; if (ignore_enoent && r == -ENOENT) { @@ -126,18 +129,18 @@ void ReadResult::C_SparseReadRequestBase::finish(ExtentMap &extent_map, << " bl " << bl.length() << dendl; // handle the case where a sparse-read wasn't issued if (extent_map.empty()) { - extent_map[offset] = bl.length(); + extent_map[object_off] = bl.length(); } aio_completion->lock.Lock(); aio_completion->read_result.m_destriper.add_partial_sparse_result( - cct, bl, extent_map, offset, buffer_extents); + cct, bl, extent_map, object_off, buffer_extents); aio_completion->lock.Unlock(); - r = length; + r = object_len; } - C_ReadRequest::finish(r); + aio_completion->complete_request(r); } ReadResult::ReadResult() : m_buffer(Empty()) { diff --git a/src/librbd/io/ReadResult.h b/src/librbd/io/ReadResult.h index 4900eeab18abf..5ae6e978aa591 100644 --- a/src/librbd/io/ReadResult.h +++ b/src/librbd/io/ReadResult.h @@ -24,58 +24,33 @@ struct AioCompletion; template struct ObjectReadRequest; class ReadResult { -private: - struct C_ReadRequest : public Context { - AioCompletion *aio_completion; - bufferlist bl; - - C_ReadRequest(AioCompletion *aio_completion); - - void finish(int r) override; - }; - public: - - struct C_ImageReadRequest : public C_ReadRequest { + struct C_ImageReadRequest : public Context { + AioCompletion *aio_completion; Extents image_extents; + bufferlist bl; C_ImageReadRequest(AioCompletion *aio_completion, - const Extents image_extents) - : C_ReadRequest(aio_completion), image_extents(image_extents) { - } + const Extents image_extents); void finish(int r) override; }; - struct C_SparseReadRequestBase : public C_ReadRequest { + struct C_ObjectReadRequest : public Context { + AioCompletion *aio_completion; + uint64_t object_off; + uint64_t object_len; + Extents buffer_extents; bool ignore_enoent; - C_SparseReadRequestBase(AioCompletion *aio_completion, bool ignore_enoent) - : C_ReadRequest(aio_completion), ignore_enoent(ignore_enoent) { - } - - using C_ReadRequest::finish; - void finish(ExtentMap &extent_map, const Extents &buffer_extents, - uint64_t offset, size_t length, bufferlist &bl, int r); - }; - - template - struct C_SparseReadRequest : public C_SparseReadRequestBase { - ObjectReadRequest *request = nullptr; - Extents buffer_extents; + bufferlist bl; + ExtentMap extent_map; - C_SparseReadRequest(AioCompletion *aio_completion, Extents&& buffer_extents, - bool ignore_enoent) - : C_SparseReadRequestBase(aio_completion, ignore_enoent), - buffer_extents(std::move(buffer_extents)) { - } + C_ObjectReadRequest(AioCompletion *aio_completion, uint64_t object_off, + uint64_t object_len, Extents&& buffer_extents, + bool ignore_enoent); - void finish(int r) override { - C_SparseReadRequestBase::finish(request->get_extent_map(), buffer_extents, - request->get_offset(), - request->get_length(), request->data(), - r); - } + void finish(int r) override; }; ReadResult(); diff --git a/src/librbd/io/Types.h b/src/librbd/io/Types.h index 7360bc3af40a9..93bddf8818c46 100644 --- a/src/librbd/io/Types.h +++ b/src/librbd/io/Types.h @@ -43,4 +43,3 @@ typedef std::map ExtentMap; } // namespace librbd #endif // CEPH_LIBRBD_IO_TYPES_H - diff --git a/src/test/librbd/io/test_mock_ObjectRequest.cc b/src/test/librbd/io/test_mock_ObjectRequest.cc index a9f8df05eba5a..a90031e6eb04d 100644 --- a/src/test/librbd/io/test_mock_ObjectRequest.cc +++ b/src/test/librbd/io/test_mock_ObjectRequest.cc @@ -339,10 +339,12 @@ TEST_F(TestMockIoObjectRequest, Read) { expect_read(mock_image_ctx, ictx->get_object_name(0), 0, 4096, std::string(4096, '1'), 0); + bufferlist bl; + ExtentMap extent_map; C_SaferCond ctx; auto req = MockObjectReadRequest::create( &mock_image_ctx, ictx->get_object_name(0), 0, 0, 4096, CEPH_NOSNAP, 0, - false, {}, &ctx); + false, {}, &bl, &extent_map, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } @@ -367,10 +369,13 @@ TEST_F(TestMockIoObjectRequest, SparseReadThreshold) { ictx->sparse_read_threshold_bytes, std::string(ictx->sparse_read_threshold_bytes, '1'), 0); + bufferlist bl; + ExtentMap extent_map; C_SaferCond ctx; auto req = MockObjectReadRequest::create( &mock_image_ctx, ictx->get_object_name(0), 0, 0, - ictx->sparse_read_threshold_bytes, CEPH_NOSNAP, 0, false, {}, &ctx); + ictx->sparse_read_threshold_bytes, CEPH_NOSNAP, 0, false, {}, + &bl, &extent_map, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } @@ -393,10 +398,12 @@ TEST_F(TestMockIoObjectRequest, ReadError) { expect_get_read_flags(mock_image_ctx, CEPH_NOSNAP, 0); expect_read(mock_image_ctx, ictx->get_object_name(0), 0, 4096, "", -EPERM); + bufferlist bl; + ExtentMap extent_map; C_SaferCond ctx; auto req = MockObjectReadRequest::create( &mock_image_ctx, ictx->get_object_name(0), 0, 0, 4096, CEPH_NOSNAP, 0, - false, {}, &ctx); + false, {}, &bl, &extent_map, &ctx); req->send(); ASSERT_EQ(-EPERM, ctx.wait()); } @@ -418,10 +425,12 @@ TEST_F(TestMockIoObjectRequest, CacheRead) { expect_cache_read(mock_image_ctx, ictx->get_object_name(0), 0, 0, 4096, std::string(4096, '1'), 0); + bufferlist bl; + ExtentMap extent_map; C_SaferCond ctx; auto req = MockObjectReadRequest::create( &mock_image_ctx, ictx->get_object_name(0), 0, 0, 4096, CEPH_NOSNAP, 0, - false, {}, &ctx); + false, {}, &bl, &extent_map, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } @@ -443,10 +452,12 @@ TEST_F(TestMockIoObjectRequest, CacheReadError) { expect_cache_read(mock_image_ctx, ictx->get_object_name(0), 0, 0, 4096, "", -EPERM); + bufferlist bl; + ExtentMap extent_map; C_SaferCond ctx; auto req = MockObjectReadRequest::create( &mock_image_ctx, ictx->get_object_name(0), 0, 0, 4096, CEPH_NOSNAP, 0, - false, {}, &ctx); + false, {}, &bl, &extent_map, &ctx); req->send(); ASSERT_EQ(-EPERM, ctx.wait()); } @@ -490,10 +501,12 @@ TEST_F(TestMockIoObjectRequest, ParentRead) { expect_prune_parent_extents(mock_image_ctx, {{0, 4096}}, 4096, 4096); expect_aio_read(mock_image_request, {{0, 4096}}, 0); + bufferlist bl; + ExtentMap extent_map; C_SaferCond ctx; auto req = MockObjectReadRequest::create( &mock_image_ctx, ictx->get_object_name(0), 0, 0, 4096, CEPH_NOSNAP, 0, - false, {}, &ctx); + false, {}, &bl, &extent_map, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } @@ -537,10 +550,12 @@ TEST_F(TestMockIoObjectRequest, ParentReadError) { expect_prune_parent_extents(mock_image_ctx, {{0, 4096}}, 4096, 4096); expect_aio_read(mock_image_request, {{0, 4096}}, -EPERM); + bufferlist bl; + ExtentMap extent_map; C_SaferCond ctx; auto req = MockObjectReadRequest::create( &mock_image_ctx, ictx->get_object_name(0), 0, 0, 4096, CEPH_NOSNAP, 0, - false, {}, &ctx); + false, {}, &bl, &extent_map, &ctx); req->send(); ASSERT_EQ(-EPERM, ctx.wait()); } @@ -578,10 +593,12 @@ TEST_F(TestMockIoObjectRequest, CacheInitiated) { expect_get_read_flags(mock_image_ctx, CEPH_NOSNAP, 0); expect_read(mock_image_ctx, ictx->get_object_name(0), 0, 4096, "", -ENOENT); + bufferlist bl; + ExtentMap extent_map; C_SaferCond ctx; auto req = MockObjectReadRequest::create( &mock_image_ctx, ictx->get_object_name(0), 0, 0, 4096, CEPH_NOSNAP, 0, true, - {}, &ctx); + {}, &bl, &extent_map, &ctx); req->send(); ASSERT_EQ(-ENOENT, ctx.wait()); } @@ -630,10 +647,12 @@ TEST_F(TestMockIoObjectRequest, CopyOnRead) { expect_prune_parent_extents(mock_image_ctx, {{0, 4096}}, 4096, 4096); expect_copyup(mock_copyup_request, 0); + bufferlist bl; + ExtentMap extent_map; C_SaferCond ctx; auto req = MockObjectReadRequest::create( &mock_image_ctx, ictx->get_object_name(0), 0, 0, 4096, CEPH_NOSNAP, 0, - false, {}, &ctx); + false, {}, &bl, &extent_map, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); }