From: Jason Dillaman Date: Mon, 25 Jul 2016 16:43:13 +0000 (-0400) Subject: librbd: ensure that AIO ops are started prior to request X-Git-Tag: v10.2.3~48^2~20 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=67861061fcd85f143e7d26077c71a582c6439745;p=ceph.git librbd: ensure that AIO ops are started prior to request Fixes: http://tracker.ceph.com/issues/16708 Signed-off-by: Jason Dillaman (cherry picked from commit 6064f2346de0a8bf2878bf5bfac9a992cda7c4ca) --- diff --git a/src/librbd/AioImageRequest.cc b/src/librbd/AioImageRequest.cc index 413e0e62f930..a01a3d85d474 100644 --- a/src/librbd/AioImageRequest.cc +++ b/src/librbd/AioImageRequest.cc @@ -82,10 +82,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(); } @@ -93,10 +90,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(); } @@ -104,35 +98,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 b30cc30f417a..0ddbbaf74e4a 100644 --- a/src/librbd/AioImageRequest.h +++ b/src/librbd/AioImageRequest.h @@ -57,6 +57,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; }; @@ -78,6 +79,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"; } @@ -142,6 +146,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"; } @@ -174,6 +181,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"; } @@ -203,6 +213,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 22ad36427356..55889e8f250e 100644 --- a/src/librbd/AioImageRequestWQ.cc +++ b/src/librbd/AioImageRequestWQ.cc @@ -125,6 +125,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(); } @@ -151,6 +152,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(); } @@ -176,6 +178,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 1246a2a65f3e..af34e8237ee9 100644 --- a/src/librbd/AioObjectRequest.cc +++ b/src/librbd/AioObjectRequest.cc @@ -269,7 +269,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 29d8a4ac40fb..4c22f175fc4a 100644 --- a/src/librbd/CopyupRequest.cc +++ b/src/librbd/CopyupRequest.cc @@ -183,7 +183,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 fd881f67f25e..68fd94a6edff 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 579688c8d7ed..e726b9ef5366 100644 --- a/src/librbd/internal.cc +++ b/src/librbd/internal.cc @@ -2391,7 +2391,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); @@ -2616,7 +2617,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 1b174bc9f9a1..fb506ec3d72c 100644 --- a/src/librbd/journal/Replay.cc +++ b/src/librbd/journal/Replay.cc @@ -294,6 +294,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); @@ -315,6 +316,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); @@ -806,6 +808,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; @@ -840,8 +843,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; } @@ -852,9 +856,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 648b5318cfe6..8490d3cc98ad 100644 --- a/src/librbd/journal/Replay.h +++ b/src/librbd/journal/Replay.h @@ -9,6 +9,7 @@ #include "include/Context.h" #include "include/rbd/librbd.hpp" #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 037006e00278..ba63588f3b9f 100644 --- a/src/test/librbd/journal/test_Replay.cc +++ b/src/test/librbd/journal/test_Replay.cc @@ -251,37 +251,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; @@ -301,7 +276,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 ca12bdda9cd2..8f1ee395b531 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;