diff --git a/src/librbd/AioCompletion.h b/src/librbd/AioCompletion.h index 758b1d8a669..e259a484698 100644 --- a/src/librbd/AioCompletion.h +++ b/src/librbd/AioCompletion.h @@ -50,7 +50,7 @@ namespace librbd { * context or via a thread pool context for cache read hits). */ struct AioCompletion { - Mutex lock; + mutable Mutex lock; Cond cond; aio_state_t state; ssize_t rval; @@ -100,6 +100,15 @@ namespace librbd { return comp; } + template + static AioCompletion *create_and_start(T *obj, ImageCtx *image_ctx, + aio_type_t type) { + AioCompletion *comp = create(obj); + comp->init_time(image_ctx, type); + comp->start_op(); + return comp; + } + AioCompletion() : lock("AioCompletion::lock", true, false), state(STATE_PENDING), rval(0), complete_cb(NULL), complete_arg(NULL), rbd_comp(NULL), @@ -117,6 +126,15 @@ namespace librbd { void finalize(ssize_t rval); + inline bool is_initialized(aio_type_t type) const { + Mutex::Locker locker(lock); + return ((ictx != nullptr) && (aio_type == type)); + } + inline bool is_started() const { + Mutex::Locker locker(lock); + return async_op.started(); + } + void init_time(ImageCtx *i, aio_type_t t); void start_op(bool ignore_type = false); void fail(int r); diff --git a/src/librbd/AioImageRequest.cc b/src/librbd/AioImageRequest.cc index 0c5a916cde0..1c2a4b140c0 100644 --- a/src/librbd/AioImageRequest.cc +++ b/src/librbd/AioImageRequest.cc @@ -79,10 +79,7 @@ void AioImageRequest::aio_read( I *ictx, AioCompletion *c, const std::vector > &extents, char *buf, bufferlist *pbl, int op_flags) { - c->init_time(ictx, librbd::AIO_TYPE_READ); - AioImageRead req(*ictx, c, extents, buf, pbl, op_flags); - req.start_op(); req.send(); } @@ -90,10 +87,7 @@ template void AioImageRequest::aio_read(I *ictx, AioCompletion *c, uint64_t off, size_t len, char *buf, bufferlist *pbl, int op_flags) { - c->init_time(ictx, librbd::AIO_TYPE_READ); - AioImageRead req(*ictx, c, off, len, buf, pbl, op_flags); - req.start_op(); req.send(); } @@ -101,35 +95,29 @@ template void AioImageRequest::aio_write(I *ictx, AioCompletion *c, uint64_t off, size_t len, const char *buf, int op_flags) { - c->init_time(ictx, librbd::AIO_TYPE_WRITE); - AioImageWrite req(*ictx, c, off, len, buf, op_flags); - req.start_op(); req.send(); } template void AioImageRequest::aio_discard(I *ictx, AioCompletion *c, uint64_t off, uint64_t len) { - c->init_time(ictx, librbd::AIO_TYPE_DISCARD); - AioImageDiscard req(*ictx, c, off, len); - req.start_op(); req.send(); } template void AioImageRequest::aio_flush(I *ictx, AioCompletion *c) { - c->init_time(ictx, librbd::AIO_TYPE_FLUSH); - + assert(c->is_initialized(AIO_TYPE_FLUSH)); AioImageFlush req(*ictx, c); - req.start_op(); req.send(); } template void AioImageRequest::send() { assert(m_image_ctx.owner_lock.is_locked()); + assert(m_aio_comp->is_initialized(get_aio_type())); + assert(m_aio_comp->is_started() ^ (get_aio_type() == AIO_TYPE_FLUSH)); CephContext *cct = m_image_ctx.cct; ldout(cct, 20) << get_request_type() << ": ictx=" << &m_image_ctx << ", " diff --git a/src/librbd/AioImageRequest.h b/src/librbd/AioImageRequest.h index b3fc754f889..b36c09b2713 100644 --- a/src/librbd/AioImageRequest.h +++ b/src/librbd/AioImageRequest.h @@ -58,6 +58,7 @@ protected: : m_image_ctx(image_ctx), m_aio_comp(aio_comp) {} virtual void send_request() = 0; + virtual aio_type_t get_aio_type() const = 0; virtual const char *get_request_type() const = 0; }; @@ -79,6 +80,9 @@ public: protected: virtual void send_request(); + virtual aio_type_t get_aio_type() const { + return AIO_TYPE_READ; + } virtual const char *get_request_type() const { return "aio_read"; } @@ -145,6 +149,9 @@ public: } protected: + virtual aio_type_t get_aio_type() const { + return AIO_TYPE_WRITE; + } virtual const char *get_request_type() const { return "aio_write"; } @@ -177,6 +184,9 @@ public: } protected: + virtual aio_type_t get_aio_type() const { + return AIO_TYPE_DISCARD; + } virtual const char *get_request_type() const { return "aio_discard"; } @@ -207,6 +217,9 @@ public: protected: virtual void send_request(); + virtual aio_type_t get_aio_type() const { + return AIO_TYPE_FLUSH; + } virtual const char *get_request_type() const { return "aio_flush"; } diff --git a/src/librbd/AioImageRequestWQ.cc b/src/librbd/AioImageRequestWQ.cc index 725e96a8770..b145edf94bc 100644 --- a/src/librbd/AioImageRequestWQ.cc +++ b/src/librbd/AioImageRequestWQ.cc @@ -122,6 +122,7 @@ void AioImageRequestWQ::aio_read(AioCompletion *c, uint64_t off, uint64_t len, lock_required) { queue(new AioImageRead(m_image_ctx, c, off, len, buf, pbl, op_flags)); } else { + c->start_op(); AioImageRequest<>::aio_read(&m_image_ctx, c, off, len, buf, pbl, op_flags); finish_in_flight_op(); } @@ -148,6 +149,7 @@ void AioImageRequestWQ::aio_write(AioCompletion *c, uint64_t off, uint64_t len, if (m_image_ctx.non_blocking_aio || writes_blocked()) { queue(new AioImageWrite(m_image_ctx, c, off, len, buf, op_flags)); } else { + c->start_op(); AioImageRequest<>::aio_write(&m_image_ctx, c, off, len, buf, op_flags); finish_in_flight_op(); } @@ -173,6 +175,7 @@ void AioImageRequestWQ::aio_discard(AioCompletion *c, uint64_t off, if (m_image_ctx.non_blocking_aio || writes_blocked()) { queue(new AioImageDiscard(m_image_ctx, c, off, len)); } else { + c->start_op(); AioImageRequest<>::aio_discard(&m_image_ctx, c, off, len); finish_in_flight_op(); } diff --git a/src/librbd/AioObjectRequest.cc b/src/librbd/AioObjectRequest.cc index dd55c8e6503..934375ec040 100644 --- a/src/librbd/AioObjectRequest.cc +++ b/src/librbd/AioObjectRequest.cc @@ -268,7 +268,8 @@ namespace librbd { void AioObjectRead::read_from_parent(const vector >& parent_extents) { assert(!m_parent_completion); - m_parent_completion = AioCompletion::create(this); + m_parent_completion = AioCompletion::create_and_start( + this, m_ictx, AIO_TYPE_READ); // prevent the parent image from being deleted while this // request is still in-progress diff --git a/src/librbd/CopyupRequest.cc b/src/librbd/CopyupRequest.cc index 484aea8ae2f..da7c43ae286 100644 --- a/src/librbd/CopyupRequest.cc +++ b/src/librbd/CopyupRequest.cc @@ -181,7 +181,8 @@ private: void CopyupRequest::send() { m_state = STATE_READ_FROM_PARENT; - AioCompletion *comp = AioCompletion::create(this); + AioCompletion *comp = AioCompletion::create_and_start( + this, m_ictx, AIO_TYPE_READ); ldout(m_ictx->cct, 20) << __func__ << " " << this << ": completion " << comp diff --git a/src/librbd/Utils.h b/src/librbd/Utils.h index 6e231bb928e..dcdad1d6fbb 100644 --- a/src/librbd/Utils.h +++ b/src/librbd/Utils.h @@ -154,6 +154,10 @@ Context *create_async_context_callback(I &image_ctx, Context *on_finish) { image_ctx.op_work_queue, on_finish); } +inline ImageCtx *get_image_ctx(ImageCtx *image_ctx) { + return image_ctx; +} + } // namespace util } // namespace librbd diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc index 5e6f42c63c0..af000b7464b 100644 --- a/src/librbd/internal.cc +++ b/src/librbd/internal.cc @@ -2581,7 +2581,8 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force, uint64_t len = min(period, src_size - offset); bufferlist *bl = new bufferlist(); Context *ctx = new C_CopyRead(&throttle, dest, offset, bl); - AioCompletion *comp = AioCompletion::create(ctx); + AioCompletion *comp = AioCompletion::create_and_start(ctx, src, + AIO_TYPE_READ); AioImageRequest<>::aio_read(src, comp, offset, len, NULL, bl, fadvise_flags); prog_ctx.update_progress(offset, src_size); @@ -2806,7 +2807,8 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force, bufferlist bl; C_SaferCond ctx; - AioCompletion *c = AioCompletion::create(&ctx); + AioCompletion *c = AioCompletion::create_and_start(&ctx, ictx, + AIO_TYPE_READ); AioImageRequest<>::aio_read(ictx, c, off, read_len, NULL, &bl, 0); int ret = ctx.wait(); diff --git a/src/librbd/journal/Replay.cc b/src/librbd/journal/Replay.cc index e306b708466..177a4532a1b 100644 --- a/src/librbd/journal/Replay.cc +++ b/src/librbd/journal/Replay.cc @@ -297,6 +297,7 @@ void Replay::handle_event(const journal::AioDiscardEvent &event, bool flush_required; AioCompletion *aio_comp = create_aio_modify_completion(on_ready, on_safe, + AIO_TYPE_DISCARD, &flush_required); AioImageRequest::aio_discard(&m_image_ctx, aio_comp, event.offset, event.length); @@ -318,6 +319,7 @@ void Replay::handle_event(const journal::AioWriteEvent &event, bufferlist data = event.data; bool flush_required; AioCompletion *aio_comp = create_aio_modify_completion(on_ready, on_safe, + AIO_TYPE_WRITE, &flush_required); AioImageRequest::aio_write(&m_image_ctx, aio_comp, event.offset, event.length, data.c_str(), 0); @@ -837,6 +839,7 @@ void Replay::handle_op_complete(uint64_t op_tid, int r) { template AioCompletion *Replay::create_aio_modify_completion(Context *on_ready, Context *on_safe, + aio_type_t aio_type, bool *flush_required) { Mutex::Locker locker(m_lock); CephContext *cct = m_image_ctx.cct; @@ -871,8 +874,9 @@ AioCompletion *Replay::create_aio_modify_completion(Context *on_ready, // when the modification is ACKed by librbd, we can process the next // event. when flushed, the completion of the next flush will fire the // on_safe callback - AioCompletion *aio_comp = AioCompletion::create( - new C_AioModifyComplete(this, on_ready, on_safe)); + AioCompletion *aio_comp = AioCompletion::create_and_start( + new C_AioModifyComplete(this, on_ready, on_safe), + util::get_image_ctx(&m_image_ctx), aio_type); return aio_comp; } @@ -883,9 +887,10 @@ AioCompletion *Replay::create_aio_flush_completion(Context *on_safe) { ++m_in_flight_aio_flush; // associate all prior write/discard ops to this flush request - AioCompletion *aio_comp = AioCompletion::create( + AioCompletion *aio_comp = AioCompletion::create_and_start( new C_AioFlushComplete(this, on_safe, - std::move(m_aio_modify_unsafe_contexts))); + std::move(m_aio_modify_unsafe_contexts)), + util::get_image_ctx(&m_image_ctx), AIO_TYPE_FLUSH); m_aio_modify_unsafe_contexts.clear(); return aio_comp; } diff --git a/src/librbd/journal/Replay.h b/src/librbd/journal/Replay.h index a4faeb7d0d9..8e5ef6bf33e 100644 --- a/src/librbd/journal/Replay.h +++ b/src/librbd/journal/Replay.h @@ -8,6 +8,7 @@ #include "include/buffer_fwd.h" #include "include/Context.h" #include "common/Mutex.h" +#include "librbd/AioCompletion.h" #include "librbd/journal/Types.h" #include #include @@ -171,6 +172,7 @@ private: AioCompletion *create_aio_modify_completion(Context *on_ready, Context *on_safe, + aio_type_t aio_type, bool *flush_required); AioCompletion *create_aio_flush_completion(Context *on_safe); void handle_aio_completion(AioCompletion *aio_comp); diff --git a/src/test/librbd/journal/test_Replay.cc b/src/test/librbd/journal/test_Replay.cc index 61cdce059ff..819b456cbd2 100644 --- a/src/test/librbd/journal/test_Replay.cc +++ b/src/test/librbd/journal/test_Replay.cc @@ -255,37 +255,12 @@ TEST_F(TestJournalReplay, AioFlushEvent) { // inject a flush operation into the journal inject_into_journal(ictx, librbd::journal::AioFlushEvent()); - - // start an AIO write op - librbd::Journal<> *journal = ictx->journal; - ictx->journal = NULL; - - std::string payload(m_image_size, '1'); - librbd::AioCompletion *aio_comp = new librbd::AioCompletion(); - { - RWLock::RLocker owner_lock(ictx->owner_lock); - librbd::AioImageRequest<>::aio_write(ictx, aio_comp, 0, payload.size(), - payload.c_str(), 0); - } - ictx->journal = journal; close_image(ictx); // re-open the journal so that it replays the new entry ASSERT_EQ(0, open_image(m_image_name, &ictx)); ASSERT_EQ(0, when_acquired_lock(ictx)); - ASSERT_TRUE(aio_comp->is_complete()); - ASSERT_EQ(0, aio_comp->wait_for_complete()); - aio_comp->release(); - - std::string read_payload(m_image_size, '\0'); - aio_comp = new librbd::AioCompletion(); - ictx->aio_work_queue->aio_read(aio_comp, 0, read_payload.size(), - &read_payload[0], NULL, 0); - ASSERT_EQ(0, aio_comp->wait_for_complete()); - aio_comp->release(); - ASSERT_EQ(payload, read_payload); - // check the commit position is properly updated int64_t current_tag; int64_t current_entry; @@ -305,7 +280,7 @@ TEST_F(TestJournalReplay, AioFlushEvent) { ASSERT_EQ(1, current_entry); // verify lock ordering constraints - aio_comp = new librbd::AioCompletion(); + librbd::AioCompletion *aio_comp = new librbd::AioCompletion(); ictx->aio_work_queue->aio_flush(aio_comp); ASSERT_EQ(0, aio_comp->wait_for_complete()); aio_comp->release(); diff --git a/src/test/librbd/journal/test_mock_Replay.cc b/src/test/librbd/journal/test_mock_Replay.cc index 016f217ae21..750f256226f 100644 --- a/src/test/librbd/journal/test_mock_Replay.cc +++ b/src/test/librbd/journal/test_mock_Replay.cc @@ -54,8 +54,16 @@ struct AioImageRequest { AioImageRequest *AioImageRequest::s_instance = nullptr; +namespace util { + +inline ImageCtx *get_image_ctx(librbd::MockReplayImageCtx *image_ctx) { + return image_ctx->image_ctx; } +} // namespace util + +} // namespace librbd + // template definitions #include "librbd/journal/Replay.cc" template class librbd::journal::Replay;