From baa0b682607a25126517fef3bac0e9d13b0b23c5 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Thu, 15 Feb 2018 10:44:38 -0500 Subject: [PATCH] librbd: separated queued image IO requests from state machine This breaks the tight coupling between the IO work queue and the actual dispatch of IO requests. Signed-off-by: Jason Dillaman --- src/librbd/CMakeLists.txt | 1 + src/librbd/cache/ImageWriteback.cc | 4 +- src/librbd/io/ImageDispatchSpec.cc | 98 ++++++++++ src/librbd/io/ImageDispatchSpec.h | 170 ++++++++++++++++++ src/librbd/io/ImageRequest.cc | 78 +------- src/librbd/io/ImageRequest.h | 74 ++------ src/librbd/io/ImageRequestWQ.cc | 58 +++--- src/librbd/io/ImageRequestWQ.h | 23 ++- src/librbd/journal/Replay.cc | 11 +- src/test/librbd/io/test_mock_ImageRequest.cc | 4 +- .../librbd/io/test_mock_ImageRequestWQ.cc | 95 +++++----- src/test/librbd/journal/test_mock_Replay.cc | 27 +-- 12 files changed, 408 insertions(+), 235 deletions(-) create mode 100644 src/librbd/io/ImageDispatchSpec.cc create mode 100644 src/librbd/io/ImageDispatchSpec.h diff --git a/src/librbd/CMakeLists.txt b/src/librbd/CMakeLists.txt index 230b7240d39..141f53cd5e5 100644 --- a/src/librbd/CMakeLists.txt +++ b/src/librbd/CMakeLists.txt @@ -57,6 +57,7 @@ set(librbd_internal_srcs io/AioCompletion.cc io/AsyncOperation.cc io/CopyupRequest.cc + io/ImageDispatchSpec.cc io/ImageRequest.cc io/ImageRequestWQ.cc io/ObjectRequest.cc diff --git a/src/librbd/cache/ImageWriteback.cc b/src/librbd/cache/ImageWriteback.cc index d5c9c759c85..be366935b36 100644 --- a/src/librbd/cache/ImageWriteback.cc +++ b/src/librbd/cache/ImageWriteback.cc @@ -62,7 +62,7 @@ void ImageWriteback::aio_discard(uint64_t offset, uint64_t length, auto aio_comp = io::AioCompletion::create_and_start(on_finish, &m_image_ctx, io::AIO_TYPE_DISCARD); - io::ImageDiscardRequest req(m_image_ctx, aio_comp, offset, length, + io::ImageDiscardRequest req(m_image_ctx, aio_comp, {{offset, length}}, skip_partial_discard, {}); req.set_bypass_image_cache(); req.send(); @@ -92,7 +92,7 @@ void ImageWriteback::aio_writesame(uint64_t offset, uint64_t length, auto aio_comp = io::AioCompletion::create_and_start(on_finish, &m_image_ctx, io::AIO_TYPE_WRITESAME); - io::ImageWriteSameRequest req(m_image_ctx, aio_comp, offset, length, + io::ImageWriteSameRequest req(m_image_ctx, aio_comp, {{offset, length}}, std::move(bl), fadvise_flags, {}); req.set_bypass_image_cache(); req.send(); diff --git a/src/librbd/io/ImageDispatchSpec.cc b/src/librbd/io/ImageDispatchSpec.cc new file mode 100644 index 00000000000..897617d1812 --- /dev/null +++ b/src/librbd/io/ImageDispatchSpec.cc @@ -0,0 +1,98 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#include "librbd/io/ImageDispatchSpec.h" +#include "librbd/ImageCtx.h" +#include "librbd/io/AioCompletion.h" +#include "librbd/io/ImageRequest.h" +#include + +namespace librbd { +namespace io { + +template +struct ImageDispatchSpec::SendVisitor + : public boost::static_visitor { + ImageDispatchSpec* spec; + + SendVisitor(ImageDispatchSpec* spec) + : spec(spec) { + } + + void operator()(Read& read) const { + ImageRequest::aio_read( + &spec->m_image_ctx, spec->m_aio_comp, std::move(spec->m_image_extents), + std::move(read.read_result), spec->m_op_flags, spec->m_parent_trace); + } + + void operator()(Discard& discard) const { + ImageRequest::aio_discard( + &spec->m_image_ctx, spec->m_aio_comp, std::move(spec->m_image_extents), + discard.skip_partial_discard, spec->m_parent_trace); + } + + void operator()(Write& write) const { + ImageRequest::aio_write( + &spec->m_image_ctx, spec->m_aio_comp, std::move(spec->m_image_extents), + std::move(write.bl), spec->m_op_flags, spec->m_parent_trace); + } + + void operator()(WriteSame& write_same) const { + ImageRequest::aio_writesame( + &spec->m_image_ctx, spec->m_aio_comp, std::move(spec->m_image_extents), + std::move(write_same.bl), spec->m_op_flags, spec->m_parent_trace); + } + + void operator()(CompareAndWrite& compare_and_write) const { + ImageRequest::aio_compare_and_write( + &spec->m_image_ctx, spec->m_aio_comp, std::move(spec->m_image_extents), + std::move(compare_and_write.cmp_bl), std::move(compare_and_write.bl), + compare_and_write.mismatch_offset, spec->m_op_flags, + spec->m_parent_trace); + } + + void operator()(Flush& flush) const { + ImageRequest::aio_flush( + &spec->m_image_ctx, spec->m_aio_comp, + spec->m_parent_trace); + } +}; + +template +struct ImageDispatchSpec::IsWriteOpVisitor + : public boost::static_visitor { + bool operator()(const Read&) const { + return false; + } + + template + bool operator()(const T&) const { + return true; + } +}; + +template +void ImageDispatchSpec::send() { + boost::apply_visitor(SendVisitor{this}, m_request); +} + +template +void ImageDispatchSpec::fail(int r) { + m_aio_comp->get(); + m_aio_comp->fail(r); +} + +template +bool ImageDispatchSpec::is_write_op() const { + return boost::apply_visitor(IsWriteOpVisitor{}, m_request); +} + +template +void ImageDispatchSpec::start_op() { + m_aio_comp->start_op(); +} + +} // namespace io +} // namespace librbd + +template class librbd::io::ImageDispatchSpec; diff --git a/src/librbd/io/ImageDispatchSpec.h b/src/librbd/io/ImageDispatchSpec.h new file mode 100644 index 00000000000..b8f2319185f --- /dev/null +++ b/src/librbd/io/ImageDispatchSpec.h @@ -0,0 +1,170 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#ifndef CEPH_LIBRBD_IO_IMAGE_DISPATCH_SPEC_H +#define CEPH_LIBRBD_IO_IMAGE_DISPATCH_SPEC_H + +#include "include/int_types.h" +#include "include/buffer.h" +#include "common/zipkin_trace.h" +#include "librbd/io/Types.h" +#include "librbd/io/ReadResult.h" +#include + +namespace librbd { + +class ImageCtx; + +namespace io { + +class AioCompletion; + +template +class ImageDispatchSpec { +public: + struct Read { + ReadResult read_result; + + Read(ReadResult &&read_result) : read_result(std::move(read_result)) { + } + }; + + struct Discard { + bool skip_partial_discard; + + Discard(bool skip_partial_discard) + : skip_partial_discard(skip_partial_discard) { + } + }; + + struct Write { + bufferlist bl; + + Write(bufferlist&& bl) : bl(std::move(bl)) { + } + }; + + struct WriteSame { + bufferlist bl; + + WriteSame(bufferlist&& bl) : bl(std::move(bl)) { + } + }; + + struct CompareAndWrite { + bufferlist cmp_bl; + bufferlist bl; + uint64_t *mismatch_offset; + + CompareAndWrite(bufferlist&& cmp_bl, bufferlist&& bl, + uint64_t *mismatch_offset) + : cmp_bl(std::move(cmp_bl)), bl(std::move(bl)), + mismatch_offset(mismatch_offset) { + } + }; + + struct Flush { + }; + + static ImageDispatchSpec* create_read_request( + ImageCtxT &image_ctx, AioCompletion *aio_comp, Extents &&image_extents, + ReadResult &&read_result, int op_flags, + const ZTracer::Trace &parent_trace) { + return new ImageDispatchSpec(image_ctx, aio_comp, + std::move(image_extents), + Read{std::move(read_result)}, + op_flags, parent_trace); + } + + static ImageDispatchSpec* create_discard_request( + ImageCtxT &image_ctx, AioCompletion *aio_comp, uint64_t off, uint64_t len, + bool skip_partial_discard, const ZTracer::Trace &parent_trace) { + return new ImageDispatchSpec(image_ctx, aio_comp, {{off, len}}, + Discard{skip_partial_discard}, 0, + parent_trace); + } + + static ImageDispatchSpec* create_write_request( + ImageCtxT &image_ctx, AioCompletion *aio_comp, Extents &&image_extents, + bufferlist &&bl, int op_flags, const ZTracer::Trace &parent_trace) { + return new ImageDispatchSpec(image_ctx, aio_comp, std::move(image_extents), + Write{std::move(bl)}, op_flags, parent_trace); + } + + static ImageDispatchSpec* create_write_same_request( + ImageCtxT &image_ctx, AioCompletion *aio_comp, uint64_t off, uint64_t len, + bufferlist &&bl, int op_flags, const ZTracer::Trace &parent_trace) { + return new ImageDispatchSpec(image_ctx, aio_comp, {{off, len}}, + WriteSame{std::move(bl)}, op_flags, + parent_trace); + } + + static ImageDispatchSpec* create_compare_and_write_request( + ImageCtxT &image_ctx, AioCompletion *aio_comp, Extents &&image_extents, + bufferlist &&cmp_bl, bufferlist &&bl, uint64_t *mismatch_offset, + int op_flags, const ZTracer::Trace &parent_trace) { + return new ImageDispatchSpec(image_ctx, aio_comp, + std::move(image_extents), + CompareAndWrite{std::move(cmp_bl), + std::move(bl), + mismatch_offset}, + op_flags, parent_trace); + } + + static ImageDispatchSpec* create_flush_request( + ImageCtxT &image_ctx, AioCompletion *aio_comp, + const ZTracer::Trace &parent_trace) { + return new ImageDispatchSpec(image_ctx, aio_comp, {}, Flush{}, 0, + parent_trace); + } + + void send(); + void fail(int r); + + bool is_write_op() const; + + void start_op(); + + bool was_throttled() { + return m_throttled; + } + void set_throttled() { + m_throttled = true; + } + +private: + typedef boost::variant Request; + + struct SendVisitor; + struct IsWriteOpVisitor; + + ImageDispatchSpec(ImageCtxT& image_ctx, AioCompletion* aio_comp, + Extents&& image_extents, Request&& request, + int op_flags, const ZTracer::Trace& parent_trace) + : m_image_ctx(image_ctx), m_aio_comp(aio_comp), + m_image_extents(std::move(image_extents)), m_request(std::move(request)), + m_op_flags(op_flags), m_parent_trace(parent_trace) { + } + + ImageCtxT& m_image_ctx; + AioCompletion* m_aio_comp; + Extents m_image_extents; + Request m_request; + int m_op_flags; + ZTracer::Trace m_parent_trace; + + bool m_throttled = false; + +}; + +} // namespace io +} // namespace librbd + +extern template class librbd::io::ImageDispatchSpec; + +#endif // CEPH_LIBRBD_IO_IMAGE_DISPATCH_SPEC_H diff --git a/src/librbd/io/ImageRequest.cc b/src/librbd/io/ImageRequest.cc index 4b9f5408517..0cfc786cbdb 100644 --- a/src/librbd/io/ImageRequest.cc +++ b/src/librbd/io/ImageRequest.cc @@ -81,60 +81,6 @@ struct C_FlushJournalCommit : public Context { } // anonymous namespace -template -ImageRequest* ImageRequest::create_read_request( - I &image_ctx, AioCompletion *aio_comp, Extents &&image_extents, - ReadResult &&read_result, int op_flags, - const ZTracer::Trace &parent_trace) { - return new ImageReadRequest(image_ctx, aio_comp, - std::move(image_extents), - std::move(read_result), op_flags, - parent_trace); -} - -template -ImageRequest* ImageRequest::create_write_request( - I &image_ctx, AioCompletion *aio_comp, Extents &&image_extents, - bufferlist &&bl, int op_flags, const ZTracer::Trace &parent_trace) { - return new ImageWriteRequest(image_ctx, aio_comp, std::move(image_extents), - std::move(bl), op_flags, parent_trace); -} - -template -ImageRequest* ImageRequest::create_discard_request( - I &image_ctx, AioCompletion *aio_comp, uint64_t off, uint64_t len, - bool skip_partial_discard, const ZTracer::Trace &parent_trace) { - return new ImageDiscardRequest(image_ctx, aio_comp, off, len, - skip_partial_discard, parent_trace); -} - -template -ImageRequest* ImageRequest::create_flush_request( - I &image_ctx, AioCompletion *aio_comp, - const ZTracer::Trace &parent_trace) { - return new ImageFlushRequest(image_ctx, aio_comp, parent_trace); -} - -template -ImageRequest* ImageRequest::create_writesame_request( - I &image_ctx, AioCompletion *aio_comp, uint64_t off, uint64_t len, - bufferlist &&bl, int op_flags, const ZTracer::Trace &parent_trace) { - return new ImageWriteSameRequest(image_ctx, aio_comp, off, len, - std::move(bl), op_flags, parent_trace); -} - -template -ImageRequest* ImageRequest::create_compare_and_write_request( - I &image_ctx, AioCompletion *c, Extents &&image_extents, - bufferlist &&cmp_bl, bufferlist &&bl, uint64_t *mismatch_offset, - int op_flags, const ZTracer::Trace &parent_trace) { - return new ImageCompareAndWriteRequest(image_ctx, c, - std::move(image_extents), - std::move(cmp_bl), - std::move(bl), mismatch_offset, - op_flags, parent_trace); -} - template void ImageRequest::aio_read(I *ictx, AioCompletion *c, Extents &&image_extents, @@ -157,11 +103,11 @@ void ImageRequest::aio_write(I *ictx, AioCompletion *c, template void ImageRequest::aio_discard(I *ictx, AioCompletion *c, - uint64_t off, uint64_t len, + Extents &&image_extents, bool skip_partial_discard, const ZTracer::Trace &parent_trace) { - ImageDiscardRequest req(*ictx, c, off, len, skip_partial_discard, - parent_trace); + ImageDiscardRequest req(*ictx, c, std::move(image_extents), + skip_partial_discard, parent_trace); req.send(); } @@ -174,11 +120,11 @@ void ImageRequest::aio_flush(I *ictx, AioCompletion *c, template void ImageRequest::aio_writesame(I *ictx, AioCompletion *c, - uint64_t off, uint64_t len, + Extents &&image_extents, bufferlist &&bl, int op_flags, const ZTracer::Trace &parent_trace) { - ImageWriteSameRequest req(*ictx, c, off, len, std::move(bl), op_flags, - parent_trace); + ImageWriteSameRequest req(*ictx, c, std::move(image_extents), + std::move(bl), op_flags, parent_trace); req.send(); } @@ -237,18 +183,6 @@ int ImageRequest::clip_request() { return 0; } -template -void ImageRequest::start_op() { - m_aio_comp->start_op(); -} - -template -void ImageRequest::fail(int r) { - AioCompletion *aio_comp = this->m_aio_comp; - aio_comp->get(); - aio_comp->fail(r); -} - template ImageReadRequest::ImageReadRequest(I &image_ctx, AioCompletion *aio_comp, Extents &&image_extents, diff --git a/src/librbd/io/ImageRequest.h b/src/librbd/io/ImageRequest.h index 1abada16b58..a24f566c230 100644 --- a/src/librbd/io/ImageRequest.h +++ b/src/librbd/io/ImageRequest.h @@ -33,63 +33,27 @@ public: m_trace.event("finish"); } - static ImageRequest* create_read_request(ImageCtxT &image_ctx, - AioCompletion *aio_comp, - Extents &&image_extents, - ReadResult &&read_result, - int op_flags, - const ZTracer::Trace &parent_trace); - static ImageRequest* create_write_request(ImageCtxT &image_ctx, - AioCompletion *aio_comp, - Extents &&image_extents, - bufferlist &&bl, int op_flags, - const ZTracer::Trace &parent_trace); - static ImageRequest* create_discard_request(ImageCtxT &image_ctx, - AioCompletion *aio_comp, - uint64_t off, uint64_t len, - bool skip_partial_discard, - const ZTracer::Trace &parent_trace); - static ImageRequest* create_flush_request(ImageCtxT &image_ctx, - AioCompletion *aio_comp, - const ZTracer::Trace &parent_trace); - static ImageRequest* create_writesame_request(ImageCtxT &image_ctx, - AioCompletion *aio_comp, - uint64_t off, uint64_t len, - bufferlist &&bl, int op_flags, - const ZTracer::Trace &parent_trace); - static ImageRequest* create_compare_and_write_request( - ImageCtxT &image_ctx, AioCompletion *c, Extents &&image_extents, - bufferlist &&cmp_bl, bufferlist &&bl, uint64_t *mismatch_offset, - int op_flags, const ZTracer::Trace &parent_trace); - static void aio_read(ImageCtxT *ictx, AioCompletion *c, Extents &&image_extents, ReadResult &&read_result, int op_flags, const ZTracer::Trace &parent_trace); static void aio_write(ImageCtxT *ictx, AioCompletion *c, Extents &&image_extents, bufferlist &&bl, int op_flags, const ZTracer::Trace &parent_trace); - static void aio_discard(ImageCtxT *ictx, AioCompletion *c, uint64_t off, - uint64_t len, bool skip_partial_discard, + static void aio_discard(ImageCtxT *ictx, AioCompletion *c, + Extents &&image_extents, bool skip_partial_discard, const ZTracer::Trace &parent_trace); static void aio_flush(ImageCtxT *ictx, AioCompletion *c, const ZTracer::Trace &parent_trace); - static void aio_writesame(ImageCtxT *ictx, AioCompletion *c, uint64_t off, - uint64_t len, bufferlist &&bl, int op_flags, - const ZTracer::Trace &parent_trace); + static void aio_writesame(ImageCtxT *ictx, AioCompletion *c, + Extents &&image_extents, bufferlist &&bl, + int op_flags, const ZTracer::Trace &parent_trace); static void aio_compare_and_write(ImageCtxT *ictx, AioCompletion *c, Extents &&image_extents, bufferlist &&cmp_bl, bufferlist &&bl, uint64_t *mismatch_offset, int op_flags, const ZTracer::Trace &parent_trace); - virtual bool is_write_op() const { - return false; - } - - void start_op(); - void send(); - void fail(int r); void set_bypass_image_cache() { m_bypass_image_cache = true; @@ -99,14 +63,6 @@ public: return m_trace; } - bool was_throttled() { - return m_throttled; - } - - void set_throttled() { - m_throttled = true; - } - protected: typedef std::list ObjectRequests; @@ -115,7 +71,6 @@ protected: Extents m_image_extents; ZTracer::Trace m_trace; bool m_bypass_image_cache = false; - bool m_throttled = false; ImageRequest(ImageCtxT &image_ctx, AioCompletion *aio_comp, Extents &&image_extents, const char *trace_name, @@ -125,7 +80,7 @@ protected: m_trace(util::create_trace(image_ctx, trace_name, parent_trace)) { m_trace.event("start"); } - + virtual int clip_request(); virtual void send_request() = 0; @@ -163,10 +118,6 @@ private: template class AbstractImageWriteRequest : public ImageRequest { public: - bool is_write_op() const override { - return true; - } - inline void flag_synchronous() { m_synchronous = true; } @@ -263,10 +214,10 @@ template class ImageDiscardRequest : public AbstractImageWriteRequest { public: ImageDiscardRequest(ImageCtxT &image_ctx, AioCompletion *aio_comp, - uint64_t off, uint64_t len, bool skip_partial_discard, + Extents&& image_extents, bool skip_partial_discard, const ZTracer::Trace &parent_trace) : AbstractImageWriteRequest( - image_ctx, aio_comp, {{off, len}}, "discard", parent_trace), + image_ctx, aio_comp, std::move(image_extents), "discard", parent_trace), m_skip_partial_discard(skip_partial_discard) { } @@ -308,10 +259,6 @@ public: : ImageRequest(image_ctx, aio_comp, {}, "flush", parent_trace) { } - bool is_write_op() const override { - return true; - } - protected: using typename ImageRequest::ObjectRequests; @@ -333,10 +280,11 @@ template class ImageWriteSameRequest : public AbstractImageWriteRequest { public: ImageWriteSameRequest(ImageCtxT &image_ctx, AioCompletion *aio_comp, - uint64_t off, uint64_t len, bufferlist &&bl, + Extents&& image_extents, bufferlist &&bl, int op_flags, const ZTracer::Trace &parent_trace) : AbstractImageWriteRequest( - image_ctx, aio_comp, {{off, len}}, "writesame", parent_trace), + image_ctx, aio_comp, std::move(image_extents), "writesame", + parent_trace), m_data_bl(std::move(bl)), m_op_flags(op_flags) { } diff --git a/src/librbd/io/ImageRequestWQ.cc b/src/librbd/io/ImageRequestWQ.cc index a5a498643fe..0ce9171e59f 100644 --- a/src/librbd/io/ImageRequestWQ.cc +++ b/src/librbd/io/ImageRequestWQ.cc @@ -12,6 +12,7 @@ #include "librbd/exclusive_lock/Policy.h" #include "librbd/io/AioCompletion.h" #include "librbd/io/ImageRequest.h" +#include "librbd/io/ImageDispatchSpec.h" #include "common/EventTrace.h" #define dout_subsys ceph_subsys_rbd @@ -25,9 +26,9 @@ namespace io { template struct ImageRequestWQ::C_AcquireLock : public Context { ImageRequestWQ *work_queue; - ImageRequest *image_request; + ImageDispatchSpec *image_request; - C_AcquireLock(ImageRequestWQ *work_queue, ImageRequest *image_request) + C_AcquireLock(ImageRequestWQ *work_queue, ImageDispatchSpec *image_request) : work_queue(work_queue), image_request(image_request) { } @@ -51,10 +52,10 @@ struct ImageRequestWQ::C_BlockedWrites : public Context { template struct ImageRequestWQ::C_RefreshFinish : public Context { ImageRequestWQ *work_queue; - ImageRequest *image_request; + ImageDispatchSpec *image_request; C_RefreshFinish(ImageRequestWQ *work_queue, - ImageRequest *image_request) + ImageDispatchSpec *image_request) : work_queue(work_queue), image_request(image_request) { } void finish(int r) override { @@ -65,7 +66,7 @@ struct ImageRequestWQ::C_RefreshFinish : public Context { template ImageRequestWQ::ImageRequestWQ(I *image_ctx, const string &name, time_t ti, ThreadPool *tp) - : ThreadPool::PointerWQ >(name, ti, 0, tp), + : ThreadPool::PointerWQ >(name, ti, 0, tp), m_image_ctx(*image_ctx), m_lock(util::unique_lock_name("ImageRequestWQ::m_lock", this)) { CephContext *cct = m_image_ctx.cct; @@ -254,7 +255,7 @@ void ImageRequestWQ::aio_read(AioCompletion *c, uint64_t off, uint64_t len, RWLock::RLocker owner_locker(m_image_ctx.owner_lock); if (m_image_ctx.non_blocking_aio || writes_blocked() || !writes_empty() || require_lock_on_read()) { - queue(ImageRequest::create_read_request( + queue(ImageDispatchSpec::create_read_request( m_image_ctx, c, {{off, len}}, std::move(read_result), op_flags, trace)); } else { @@ -293,7 +294,7 @@ void ImageRequestWQ::aio_write(AioCompletion *c, uint64_t off, uint64_t len, RWLock::RLocker owner_locker(m_image_ctx.owner_lock); if (m_image_ctx.non_blocking_aio || writes_blocked()) { - queue(ImageRequest::create_write_request( + queue(ImageDispatchSpec::create_write_request( m_image_ctx, c, {{off, len}}, std::move(bl), op_flags, trace)); } else { c->start_op(); @@ -331,11 +332,11 @@ void ImageRequestWQ::aio_discard(AioCompletion *c, uint64_t off, RWLock::RLocker owner_locker(m_image_ctx.owner_lock); if (m_image_ctx.non_blocking_aio || writes_blocked()) { - queue(ImageRequest::create_discard_request( + queue(ImageDispatchSpec::create_discard_request( m_image_ctx, c, off, len, skip_partial_discard, trace)); } else { c->start_op(); - ImageRequest::aio_discard(&m_image_ctx, c, off, len, + ImageRequest::aio_discard(&m_image_ctx, c, {{off, len}}, skip_partial_discard, trace); finish_in_flight_io(); } @@ -366,7 +367,7 @@ void ImageRequestWQ::aio_flush(AioCompletion *c, bool native_async) { RWLock::RLocker owner_locker(m_image_ctx.owner_lock); if (m_image_ctx.non_blocking_aio || writes_blocked() || !writes_empty()) { - queue(ImageRequest::create_flush_request(m_image_ctx, c, trace)); + queue(ImageDispatchSpec::create_flush_request(m_image_ctx, c, trace)); } else { ImageRequest::aio_flush(&m_image_ctx, c, trace); finish_in_flight_io(); @@ -402,11 +403,11 @@ void ImageRequestWQ::aio_writesame(AioCompletion *c, uint64_t off, RWLock::RLocker owner_locker(m_image_ctx.owner_lock); if (m_image_ctx.non_blocking_aio || writes_blocked()) { - queue(ImageRequest::create_writesame_request( + queue(ImageDispatchSpec::create_write_same_request( m_image_ctx, c, off, len, std::move(bl), op_flags, trace)); } else { c->start_op(); - ImageRequest::aio_writesame(&m_image_ctx, c, off, len, std::move(bl), + ImageRequest::aio_writesame(&m_image_ctx, c, {{off, len}}, std::move(bl), op_flags, trace); finish_in_flight_io(); } @@ -443,7 +444,7 @@ void ImageRequestWQ::aio_compare_and_write(AioCompletion *c, RWLock::RLocker owner_locker(m_image_ctx.owner_lock); if (m_image_ctx.non_blocking_aio || writes_blocked()) { - queue(ImageRequest::create_compare_and_write_request( + queue(ImageDispatchSpec::create_compare_and_write_request( m_image_ctx, c, {{off, len}}, std::move(cmp_bl), std::move(bl), mismatch_off, op_flags, trace)); } else { @@ -567,8 +568,8 @@ void ImageRequestWQ::apply_qos_iops_limit(uint64_t limit) { } template -void ImageRequestWQ::handle_iops_throttle_ready(int r, - ImageRequest *item) { +void ImageRequestWQ::handle_iops_throttle_ready( + int r, ImageDispatchSpec *item) { CephContext *cct = m_image_ctx.cct; ldout(cct, 15) << "r=" << r << ", " << "req=" << item << dendl; @@ -582,7 +583,7 @@ void ImageRequestWQ::handle_iops_throttle_ready(int r, template void *ImageRequestWQ::_void_dequeue() { CephContext *cct = m_image_ctx.cct; - ImageRequest *peek_item = this->front(); + ImageDispatchSpec *peek_item = this->front(); // no queued IO requests or all IO is blocked/stalled if (peek_item == nullptr || m_io_blockers.load() > 0) { @@ -591,12 +592,12 @@ void *ImageRequestWQ::_void_dequeue() { if (!peek_item->was_throttled() && iops_throttle->get< - ImageRequestWQ, ImageRequest, + ImageRequestWQ, ImageDispatchSpec, &ImageRequestWQ::handle_iops_throttle_ready>(1, this, peek_item)) { ldout(cct, 15) << "throttling IO " << peek_item << dendl; // dequeue the throttled item and block future IO - ThreadPool::PointerWQ >::_void_dequeue(); + ThreadPool::PointerWQ >::_void_dequeue(); ++m_io_blockers; return nullptr; } @@ -620,8 +621,8 @@ void *ImageRequestWQ::_void_dequeue() { } } - ImageRequest *item = reinterpret_cast *>( - ThreadPool::PointerWQ >::_void_dequeue()); + auto item = reinterpret_cast *>( + ThreadPool::PointerWQ >::_void_dequeue()); assert(peek_item == item); if (lock_required) { @@ -669,7 +670,7 @@ void *ImageRequestWQ::_void_dequeue() { } template -void ImageRequestWQ::process(ImageRequest *req) { +void ImageRequestWQ::process(ImageDispatchSpec *req) { CephContext *cct = m_image_ctx.cct; ldout(cct, 20) << "ictx=" << &m_image_ctx << ", " << "req=" << req << dendl; @@ -686,7 +687,7 @@ void ImageRequestWQ::process(ImageRequest *req) { } template -void ImageRequestWQ::finish_queued_io(ImageRequest *req) { +void ImageRequestWQ::finish_queued_io(ImageDispatchSpec *req) { RWLock::RLocker locker(m_lock); if (req->is_write_op()) { assert(m_queued_writes > 0); @@ -750,7 +751,8 @@ void ImageRequestWQ::finish_in_flight_io() { } template -void ImageRequestWQ::fail_in_flight_io(int r, ImageRequest *req) { +void ImageRequestWQ::fail_in_flight_io( + int r, ImageDispatchSpec *req) { this->process_finish(); req->fail(r); finish_queued_io(req); @@ -766,7 +768,7 @@ bool ImageRequestWQ::is_lock_required(bool write_op) const { } template -void ImageRequestWQ::queue(ImageRequest *req) { +void ImageRequestWQ::queue(ImageDispatchSpec *req) { assert(m_image_ctx.owner_lock.is_locked()); CephContext *cct = m_image_ctx.cct; @@ -779,11 +781,12 @@ void ImageRequestWQ::queue(ImageRequest *req) { m_queued_reads++; } - ThreadPool::PointerWQ >::queue(req); + ThreadPool::PointerWQ >::queue(req); } template -void ImageRequestWQ::handle_acquire_lock(int r, ImageRequest *req) { +void ImageRequestWQ::handle_acquire_lock( + int r, ImageDispatchSpec *req) { CephContext *cct = m_image_ctx.cct; ldout(cct, 5) << "r=" << r << ", " << "req=" << req << dendl; @@ -801,7 +804,8 @@ void ImageRequestWQ::handle_acquire_lock(int r, ImageRequest *req) { } template -void ImageRequestWQ::handle_refreshed(int r, ImageRequest *req) { +void ImageRequestWQ::handle_refreshed( + int r, ImageDispatchSpec *req) { CephContext *cct = m_image_ctx.cct; ldout(cct, 5) << "resuming IO after image refresh: r=" << r << ", " << "req=" << req << dendl; diff --git a/src/librbd/io/ImageRequestWQ.h b/src/librbd/io/ImageRequestWQ.h index fd6bec30dcd..0d24be18279 100644 --- a/src/librbd/io/ImageRequestWQ.h +++ b/src/librbd/io/ImageRequestWQ.h @@ -20,12 +20,12 @@ class ImageCtx; namespace io { class AioCompletion; -template class ImageRequest; +template class ImageDispatchSpec; class ReadResult; template class ImageRequestWQ - : public ThreadPool::PointerWQ > { + : public ThreadPool::PointerWQ > { public: ImageRequestWQ(ImageCtxT *image_ctx, const string &name, time_t ti, ThreadPool *tp); @@ -55,9 +55,8 @@ public: bufferlist &&bl, uint64_t *mismatch_off, int op_flags, bool native_async=true); - using ThreadPool::PointerWQ >::drain; - - using ThreadPool::PointerWQ >::empty; + using ThreadPool::PointerWQ >::drain; + using ThreadPool::PointerWQ >::empty; void shut_down(Context *on_shutdown); @@ -76,7 +75,7 @@ public: protected: void *_void_dequeue() override; - void process(ImageRequest *req) override; + void process(ImageDispatchSpec *req) override; private: typedef std::list Contexts; @@ -113,20 +112,20 @@ private: return (m_queued_writes == 0); } - void finish_queued_io(ImageRequest *req); + void finish_queued_io(ImageDispatchSpec *req); void finish_in_flight_write(); int start_in_flight_io(AioCompletion *c); void finish_in_flight_io(); - void fail_in_flight_io(int r, ImageRequest *req); + void fail_in_flight_io(int r, ImageDispatchSpec *req); - void queue(ImageRequest *req); + void queue(ImageDispatchSpec *req); - void handle_acquire_lock(int r, ImageRequest *req); - void handle_refreshed(int r, ImageRequest *req); + void handle_acquire_lock(int r, ImageDispatchSpec *req); + void handle_refreshed(int r, ImageDispatchSpec *req); void handle_blocked_writes(int r); - void handle_iops_throttle_ready(int r, ImageRequest *item); + void handle_iops_throttle_ready(int r, ImageDispatchSpec *item); }; } // namespace io diff --git a/src/librbd/journal/Replay.cc b/src/librbd/journal/Replay.cc index e71b37c58b4..993d6f230b8 100644 --- a/src/librbd/journal/Replay.cc +++ b/src/librbd/journal/Replay.cc @@ -348,9 +348,9 @@ void Replay::handle_event(const journal::AioDiscardEvent &event, return; } - io::ImageRequest::aio_discard(&m_image_ctx, aio_comp, event.offset, - event.length, event.skip_partial_discard, - {}); + io::ImageRequest::aio_discard(&m_image_ctx, aio_comp, + {{event.offset, event.length}}, + event.skip_partial_discard, {}); if (flush_required) { m_lock.Lock(); auto flush_comp = create_aio_flush_completion(nullptr); @@ -426,8 +426,9 @@ void Replay::handle_event(const journal::AioWriteSameEvent &event, return; } - io::ImageRequest::aio_writesame(&m_image_ctx, aio_comp, event.offset, - event.length, std::move(data), 0, {}); + io::ImageRequest::aio_writesame(&m_image_ctx, aio_comp, + {{event.offset, event.length}}, + std::move(data), 0, {}); if (flush_required) { m_lock.Lock(); auto flush_comp = create_aio_flush_completion(nullptr); diff --git a/src/test/librbd/io/test_mock_ImageRequest.cc b/src/test/librbd/io/test_mock_ImageRequest.cc index e85ae259cd0..bfea417ef24 100644 --- a/src/test/librbd/io/test_mock_ImageRequest.cc +++ b/src/test/librbd/io/test_mock_ImageRequest.cc @@ -258,7 +258,7 @@ TEST_F(TestMockIoImageRequest, AioDiscardJournalAppendDisabled) { AioCompletion *aio_comp = AioCompletion::create_and_start( &aio_comp_ctx, ictx, AIO_TYPE_DISCARD); MockImageDiscardRequest mock_aio_image_discard(mock_image_ctx, aio_comp, - 0, 1, + {{0, 1}}, ictx->skip_partial_discard, {}); { @@ -322,7 +322,7 @@ TEST_F(TestMockIoImageRequest, AioWriteSameJournalAppendDisabled) { bufferlist bl; bl.append("1"); MockImageWriteSameRequest mock_aio_image_writesame(mock_image_ctx, aio_comp, - 0, 1, std::move(bl), 0, + {{0, 1}}, std::move(bl), 0, {}); { RWLock::RLocker owner_locker(mock_image_ctx.owner_lock); diff --git a/src/test/librbd/io/test_mock_ImageRequestWQ.cc b/src/test/librbd/io/test_mock_ImageRequestWQ.cc index 47f2182b980..77a321dc041 100644 --- a/src/test/librbd/io/test_mock_ImageRequestWQ.cc +++ b/src/test/librbd/io/test_mock_ImageRequestWQ.cc @@ -5,6 +5,7 @@ #include "test/librbd/test_support.h" #include "test/librbd/mock/MockImageCtx.h" #include "test/librbd/mock/exclusive_lock/MockPolicy.h" +#include "librbd/io/ImageDispatchSpec.h" #include "librbd/io/ImageRequestWQ.h" #include "librbd/io/ImageRequest.h" @@ -25,20 +26,29 @@ struct ImageRequest { static ImageRequest* s_instance; AioCompletion *aio_comp = nullptr; - static ImageRequest* create_write_request(librbd::MockTestImageCtx &image_ctx, - AioCompletion *aio_comp, - Extents &&image_extents, - bufferlist &&bl, int op_flags, - const ZTracer::Trace &parent_trace) { - assert(s_instance != nullptr); - s_instance->aio_comp = aio_comp; - return s_instance; - } static void aio_write(librbd::MockTestImageCtx *ictx, AioCompletion *c, Extents &&image_extents, bufferlist &&bl, int op_flags, const ZTracer::Trace &parent_trace) { } + ImageRequest() { + s_instance = this; + } +}; + +template <> +struct ImageDispatchSpec { + static ImageDispatchSpec* s_instance; + AioCompletion *aio_comp = nullptr; + + static ImageDispatchSpec* create_write_request( + librbd::MockTestImageCtx &image_ctx, AioCompletion *aio_comp, + Extents &&image_extents, bufferlist &&bl, int op_flags, + const ZTracer::Trace &parent_trace) { + assert(s_instance != nullptr); + s_instance->aio_comp = aio_comp; + return s_instance; + } MOCK_CONST_METHOD0(is_write_op, bool()); MOCK_CONST_METHOD0(start_op, void()); @@ -47,11 +57,10 @@ struct ImageRequest { MOCK_CONST_METHOD0(was_throttled, bool()); MOCK_CONST_METHOD0(set_throttled, void()); - ImageRequest() { + ImageDispatchSpec() { s_instance = this; } }; - } // namespace io namespace util { @@ -65,8 +74,8 @@ inline ImageCtx *get_image_ctx(MockTestImageCtx *image_ctx) { } // namespace librbd template <> -struct ThreadPool::PointerWQ> { - typedef librbd::io::ImageRequest ImageRequest; +struct ThreadPool::PointerWQ> { + typedef librbd::io::ImageDispatchSpec ImageDispatchSpec; static PointerWQ* s_instance; Mutex m_lock; @@ -83,11 +92,11 @@ struct ThreadPool::PointerWQ> MOCK_METHOD0(signal, void()); MOCK_METHOD0(process_finish, void()); - MOCK_METHOD0(front, ImageRequest*()); - MOCK_METHOD1(requeue, void(ImageRequest*)); + MOCK_METHOD0(front, ImageDispatchSpec*()); + MOCK_METHOD1(requeue, void(ImageDispatchSpec*)); MOCK_METHOD0(dequeue, void*()); - MOCK_METHOD1(queue, void(ImageRequest*)); + MOCK_METHOD1(queue, void(ImageDispatchSpec*)); void register_work_queue() { // no-op @@ -100,21 +109,23 @@ struct ThreadPool::PointerWQ> Mutex::Locker locker(m_lock); return _void_dequeue(); } - void invoke_process(ImageRequest *image_request) { + void invoke_process(ImageDispatchSpec *image_request) { process(image_request); } virtual void *_void_dequeue() { return dequeue(); } - virtual void process(ImageRequest *req) = 0; + virtual void process(ImageDispatchSpec *req) = 0; }; -ThreadPool::PointerWQ>* - ThreadPool::PointerWQ>::s_instance = nullptr; +ThreadPool::PointerWQ>* + ThreadPool::PointerWQ>::s_instance = nullptr; librbd::io::ImageRequest* librbd::io::ImageRequest::s_instance = nullptr; +librbd::io::ImageDispatchSpec* + librbd::io::ImageDispatchSpec::s_instance = nullptr; #include "librbd/io/ImageRequestWQ.cc" @@ -130,8 +141,10 @@ using ::testing::WithArg; struct TestMockIoImageRequestWQ : public TestMockFixture { typedef ImageRequestWQ MockImageRequestWQ; typedef ImageRequest MockImageRequest; + typedef ImageDispatchSpec MockImageDispatchSpec; - void expect_is_write_op(MockImageRequest &image_request, bool write_op) { + void expect_is_write_op(MockImageDispatchSpec &image_request, + bool write_op) { EXPECT_CALL(image_request, is_write_op()).WillOnce(Return(write_op)); } @@ -144,7 +157,7 @@ struct TestMockIoImageRequestWQ : public TestMockFixture { } void expect_front(MockImageRequestWQ &image_request_wq, - MockImageRequest *image_request) { + MockImageDispatchSpec *image_request) { EXPECT_CALL(image_request_wq, front()).WillOnce(Return(image_request)); } @@ -155,7 +168,7 @@ struct TestMockIoImageRequestWQ : public TestMockFixture { } void expect_dequeue(MockImageRequestWQ &image_request_wq, - MockImageRequest *image_request) { + MockImageDispatchSpec *image_request) { EXPECT_CALL(image_request_wq, dequeue()).WillOnce(Return(image_request)); } @@ -182,7 +195,7 @@ struct TestMockIoImageRequestWQ : public TestMockFixture { EXPECT_CALL(mock_image_request_wq, process_finish()).Times(1); } - void expect_fail(MockImageRequest &mock_image_request, int r) { + void expect_fail(MockImageDispatchSpec &mock_image_request, int r) { EXPECT_CALL(mock_image_request, fail(r)) .WillOnce(Invoke([&mock_image_request](int r) { mock_image_request.aio_comp->get(); @@ -197,7 +210,7 @@ struct TestMockIoImageRequestWQ : public TestMockFixture { })); } - void expect_was_throttled(MockImageRequest &mock_image_request, + void expect_was_throttled(MockImageDispatchSpec &mock_image_request, bool throttled) { EXPECT_CALL(mock_image_request, was_throttled()) .WillOnce(Return(throttled)); @@ -219,18 +232,18 @@ TEST_F(TestMockIoImageRequestWQ, AcquireLockError) { expect_signal(mock_image_request_wq); mock_image_request_wq.set_require_lock(DIRECTION_WRITE, true); - auto mock_image_request = new MockImageRequest(); - expect_is_write_op(*mock_image_request, true); + auto mock_queued_image_request = new MockImageDispatchSpec(); + expect_is_write_op(*mock_queued_image_request, true); expect_queue(mock_image_request_wq); auto *aio_comp = new librbd::io::AioCompletion(); mock_image_request_wq.aio_write(aio_comp, 0, 0, {}, 0); librbd::exclusive_lock::MockPolicy mock_exclusive_lock_policy; - expect_front(mock_image_request_wq, mock_image_request); - expect_was_throttled(*mock_image_request, false); + expect_front(mock_image_request_wq, mock_queued_image_request); + expect_was_throttled(*mock_queued_image_request, false); expect_is_refresh_request(mock_image_ctx, false); - expect_is_write_op(*mock_image_request, true); - expect_dequeue(mock_image_request_wq, mock_image_request); + expect_is_write_op(*mock_queued_image_request, true); + expect_dequeue(mock_image_request_wq, mock_queued_image_request); expect_get_exclusive_lock_policy(mock_image_ctx, mock_exclusive_lock_policy); expect_may_auto_request_lock(mock_exclusive_lock_policy, true); Context *on_acquire = nullptr; @@ -239,8 +252,8 @@ TEST_F(TestMockIoImageRequestWQ, AcquireLockError) { ASSERT_TRUE(on_acquire != nullptr); expect_process_finish(mock_image_request_wq); - expect_fail(*mock_image_request, -EPERM); - expect_is_write_op(*mock_image_request, true); + expect_fail(*mock_queued_image_request, -EPERM); + expect_is_write_op(*mock_queued_image_request, true); expect_signal(mock_image_request_wq); on_acquire->complete(-EPERM); @@ -258,25 +271,25 @@ TEST_F(TestMockIoImageRequestWQ, RefreshError) { InSequence seq; MockImageRequestWQ mock_image_request_wq(&mock_image_ctx, "io", 60, nullptr); - auto mock_image_request = new MockImageRequest(); - expect_is_write_op(*mock_image_request, true); + auto mock_queued_image_request = new MockImageDispatchSpec(); + expect_is_write_op(*mock_queued_image_request, true); expect_queue(mock_image_request_wq); auto *aio_comp = new librbd::io::AioCompletion(); mock_image_request_wq.aio_write(aio_comp, 0, 0, {}, 0); - expect_front(mock_image_request_wq, mock_image_request); - expect_was_throttled(*mock_image_request, false); + expect_front(mock_image_request_wq, mock_queued_image_request); + expect_was_throttled(*mock_queued_image_request, false); expect_is_refresh_request(mock_image_ctx, true); - expect_is_write_op(*mock_image_request, true); - expect_dequeue(mock_image_request_wq, mock_image_request); + expect_is_write_op(*mock_queued_image_request, true); + expect_dequeue(mock_image_request_wq, mock_queued_image_request); Context *on_refresh = nullptr; expect_refresh(mock_image_ctx, &on_refresh); ASSERT_TRUE(mock_image_request_wq.invoke_dequeue() == nullptr); ASSERT_TRUE(on_refresh != nullptr); expect_process_finish(mock_image_request_wq); - expect_fail(*mock_image_request, -EPERM); - expect_is_write_op(*mock_image_request, true); + expect_fail(*mock_queued_image_request, -EPERM); + expect_is_write_op(*mock_queued_image_request, true); expect_signal(mock_image_request_wq); on_refresh->complete(-EPERM); diff --git a/src/test/librbd/journal/test_mock_Replay.cc b/src/test/librbd/journal/test_mock_Replay.cc index 914f4552a0f..4e91bb564b7 100644 --- a/src/test/librbd/journal/test_mock_Replay.cc +++ b/src/test/librbd/journal/test_mock_Replay.cc @@ -37,14 +37,13 @@ struct ImageRequest { s_instance->aio_write(c, image_extents, bl, op_flags); } - MOCK_METHOD4(aio_discard, void(AioCompletion *c, uint64_t off, uint64_t len, + MOCK_METHOD3(aio_discard, void(AioCompletion *c, const Extents& image_extents, bool skip_partial_discard)); static void aio_discard(MockReplayImageCtx *ictx, AioCompletion *c, - uint64_t off, uint64_t len, - bool skip_partial_discard, + Extents&& image_extents, bool skip_partial_discard, const ZTracer::Trace &parent_trace) { assert(s_instance != nullptr); - s_instance->aio_discard(c, off, len, skip_partial_discard); + s_instance->aio_discard(c, image_extents, skip_partial_discard); } MOCK_METHOD1(aio_flush, void(AioCompletion *c)); @@ -54,13 +53,14 @@ struct ImageRequest { s_instance->aio_flush(c); } - MOCK_METHOD5(aio_writesame, void(AioCompletion *c, uint64_t off, uint64_t len, + MOCK_METHOD4(aio_writesame, void(AioCompletion *c, + const Extents& image_extents, const bufferlist &bl, int op_flags)); static void aio_writesame(MockReplayImageCtx *ictx, AioCompletion *c, - uint64_t off, uint64_t len, bufferlist &&bl, + Extents&& image_extents, bufferlist &&bl, int op_flags, const ZTracer::Trace &parent_trace) { assert(s_instance != nullptr); - s_instance->aio_writesame(c, off, len, bl, op_flags); + s_instance->aio_writesame(c, image_extents, bl, op_flags); } MOCK_METHOD6(aio_compare_and_write, void(AioCompletion *c, const Extents &image_extents, @@ -148,7 +148,8 @@ public: void expect_aio_discard(MockIoImageRequest &mock_io_image_request, io::AioCompletion **aio_comp, uint64_t off, uint64_t len, bool skip_partial_discard) { - EXPECT_CALL(mock_io_image_request, aio_discard(_, off, len, skip_partial_discard)) + EXPECT_CALL(mock_io_image_request, aio_discard(_, io::Extents{{off, len}}, + skip_partial_discard)) .WillOnce(SaveArg<0>(aio_comp)); } @@ -176,17 +177,21 @@ public: io::AioCompletion **aio_comp, uint64_t off, uint64_t len, const char *data) { EXPECT_CALL(mock_io_image_request, - aio_writesame(_, off, len, BufferlistEqual(data), _)) + aio_writesame(_, io::Extents{{off, len}}, + BufferlistEqual(data), _)) .WillOnce(SaveArg<0>(aio_comp)); } void expect_aio_compare_and_write(MockIoImageRequest &mock_io_image_request, io::AioCompletion **aio_comp, uint64_t off, - uint64_t len, const char *cmp_data, const char *data, + uint64_t len, const char *cmp_data, + const char *data, uint64_t *mismatch_offset) { EXPECT_CALL(mock_io_image_request, aio_compare_and_write(_, io::Extents{{off, len}}, - BufferlistEqual(cmp_data), BufferlistEqual(data), mismatch_offset, _)) + BufferlistEqual(cmp_data), + BufferlistEqual(data), + mismatch_offset, _)) .WillOnce(SaveArg<0>(aio_comp)); } -- 2.39.5