From 49f4f331d9f9030b92855eec3965fb63d92c18f4 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Mon, 19 Feb 2018 16:09:51 -0500 Subject: [PATCH] librbd: track the originating source of a flush request This will be used in a later cleanup to remove the IO flush handling from ImageCtx. Signed-off-by: Jason Dillaman --- src/librbd/cache/ImageWriteback.cc | 3 +- src/librbd/io/ImageDispatchSpec.cc | 2 +- src/librbd/io/ImageRequest.cc | 50 +++++++++++--------- src/librbd/io/ImageRequest.h | 13 +++-- src/librbd/io/ImageRequestWQ.cc | 2 +- src/librbd/io/Types.h | 6 +++ src/librbd/journal/Replay.cc | 21 +++++--- src/test/librbd/io/test_mock_ImageRequest.cc | 11 ++++- src/test/librbd/journal/test_mock_Replay.cc | 2 +- 9 files changed, 71 insertions(+), 39 deletions(-) diff --git a/src/librbd/cache/ImageWriteback.cc b/src/librbd/cache/ImageWriteback.cc index be366935b36..54b964d9091 100644 --- a/src/librbd/cache/ImageWriteback.cc +++ b/src/librbd/cache/ImageWriteback.cc @@ -75,7 +75,8 @@ void ImageWriteback::aio_flush(Context *on_finish) { auto aio_comp = io::AioCompletion::create_and_start(on_finish, &m_image_ctx, io::AIO_TYPE_FLUSH); - io::ImageFlushRequest req(m_image_ctx, aio_comp, {}); + io::ImageFlushRequest req(m_image_ctx, aio_comp, io::FLUSH_SOURCE_INTERNAL, + {}); req.set_bypass_image_cache(); req.send(); } diff --git a/src/librbd/io/ImageDispatchSpec.cc b/src/librbd/io/ImageDispatchSpec.cc index 897617d1812..6064bae6657 100644 --- a/src/librbd/io/ImageDispatchSpec.cc +++ b/src/librbd/io/ImageDispatchSpec.cc @@ -53,7 +53,7 @@ struct ImageDispatchSpec::SendVisitor void operator()(Flush& flush) const { ImageRequest::aio_flush( - &spec->m_image_ctx, spec->m_aio_comp, + &spec->m_image_ctx, spec->m_aio_comp, FLUSH_SOURCE_USER, spec->m_parent_trace); } }; diff --git a/src/librbd/io/ImageRequest.cc b/src/librbd/io/ImageRequest.cc index 0cfc786cbdb..fdd7b18db43 100644 --- a/src/librbd/io/ImageRequest.cc +++ b/src/librbd/io/ImageRequest.cc @@ -113,8 +113,9 @@ void ImageRequest::aio_discard(I *ictx, AioCompletion *c, template void ImageRequest::aio_flush(I *ictx, AioCompletion *c, - const ZTracer::Trace &parent_trace) { - ImageFlushRequest req(*ictx, c, parent_trace); + FlushSource flush_source, + const ZTracer::Trace &parent_trace) { + ImageFlushRequest req(*ictx, c, flush_source, parent_trace); req.send(); } @@ -593,47 +594,50 @@ void ImageDiscardRequest::update_stats(size_t length) { template void ImageFlushRequest::send_request() { I &image_ctx = this->m_image_ctx; - image_ctx.user_flushed(); + if (m_flush_source == FLUSH_SOURCE_USER) { + // flag cache for writeback mode if configured + image_ctx.user_flushed(); + } bool journaling = false; { RWLock::RLocker snap_locker(image_ctx.snap_lock); - journaling = (image_ctx.journal != nullptr && + journaling = (m_flush_source == FLUSH_SOURCE_USER && + image_ctx.journal != nullptr && image_ctx.journal->is_journal_appending()); } AioCompletion *aio_comp = this->m_aio_comp; + aio_comp->set_request_count(1); + + Context *ctx; if (journaling) { // in-flight ops are flushed prior to closing the journal uint64_t journal_tid = image_ctx.journal->append_io_event( journal::EventEntry(journal::AioFlushEvent()), ObjectRequests(), 0, 0, false, 0); - - aio_comp->set_request_count(1); aio_comp->associate_journal_event(journal_tid); - FunctionContext *flush_ctx = new FunctionContext( - [aio_comp, &image_ctx, journal_tid] (int r) { - auto ctx = new C_FlushJournalCommit(image_ctx, aio_comp, - journal_tid); + ctx = new C_FlushJournalCommit(image_ctx, aio_comp, journal_tid); + ctx = new FunctionContext( + [&image_ctx, journal_tid, ctx] (int r) { image_ctx.journal->flush_event(journal_tid, ctx); - - // track flush op for block writes - aio_comp->start_op(true); - aio_comp->put(); - }); - - image_ctx.flush_async_operations(flush_ctx); + }); } else { // flush rbd cache only when journaling is not enabled - aio_comp->set_request_count(1); - C_AioRequest *req_comp = new C_AioRequest(aio_comp); - image_ctx.flush(req_comp); - - aio_comp->start_op(true); - aio_comp->put(); + ctx = new C_AioRequest(aio_comp); + if (image_ctx.object_cacher != nullptr) { + ctx = new FunctionContext([aio_comp, &image_ctx, ctx](int r) { + image_ctx.flush_cache(ctx); + }); + } } + // ensure all in-flight IOs are settled if non-user flush request + image_ctx.flush_async_operations(ctx); + aio_comp->start_op(true); + aio_comp->put(); + image_ctx.perfcounter->inc(l_librbd_flush); } diff --git a/src/librbd/io/ImageRequest.h b/src/librbd/io/ImageRequest.h index a24f566c230..ec91582cebb 100644 --- a/src/librbd/io/ImageRequest.h +++ b/src/librbd/io/ImageRequest.h @@ -43,7 +43,8 @@ public: 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); + FlushSource flush_source, + 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); @@ -255,8 +256,10 @@ template class ImageFlushRequest : public ImageRequest { public: ImageFlushRequest(ImageCtxT &image_ctx, AioCompletion *aio_comp, - const ZTracer::Trace &parent_trace) - : ImageRequest(image_ctx, aio_comp, {}, "flush", parent_trace) { + FlushSource flush_source, + const ZTracer::Trace &parent_trace) + : ImageRequest(image_ctx, aio_comp, {}, "flush", parent_trace), + m_flush_source(flush_source) { } protected: @@ -274,6 +277,10 @@ protected: const char *get_request_type() const override { return "aio_flush"; } + +private: + FlushSource m_flush_source; + }; template diff --git a/src/librbd/io/ImageRequestWQ.cc b/src/librbd/io/ImageRequestWQ.cc index 0ce9171e59f..ca77c684314 100644 --- a/src/librbd/io/ImageRequestWQ.cc +++ b/src/librbd/io/ImageRequestWQ.cc @@ -369,7 +369,7 @@ void ImageRequestWQ::aio_flush(AioCompletion *c, bool native_async) { if (m_image_ctx.non_blocking_aio || writes_blocked() || !writes_empty()) { queue(ImageDispatchSpec::create_flush_request(m_image_ctx, c, trace)); } else { - ImageRequest::aio_flush(&m_image_ctx, c, trace); + ImageRequest::aio_flush(&m_image_ctx, c, FLUSH_SOURCE_USER, trace); finish_in_flight_io(); } trace.event("finish"); diff --git a/src/librbd/io/Types.h b/src/librbd/io/Types.h index 2b69d7bbcf0..7360bc3af40 100644 --- a/src/librbd/io/Types.h +++ b/src/librbd/io/Types.h @@ -24,6 +24,12 @@ typedef enum { AIO_TYPE_COMPARE_AND_WRITE, } aio_type_t; +enum FlushSource { + FLUSH_SOURCE_USER, + FLUSH_SOURCE_INTERNAL, + FLUSH_SOURCE_SHUTDOWN +}; + enum Direction { DIRECTION_READ, DIRECTION_WRITE, diff --git a/src/librbd/journal/Replay.cc b/src/librbd/journal/Replay.cc index 993d6f230b8..da91f28bc0d 100644 --- a/src/librbd/journal/Replay.cc +++ b/src/librbd/journal/Replay.cc @@ -271,7 +271,8 @@ void Replay::shut_down(bool cancel_ops, Context *on_finish) { // execute the following outside of lock scope if (flush_comp != nullptr) { RWLock::RLocker owner_locker(m_image_ctx.owner_lock); - io::ImageRequest::aio_flush(&m_image_ctx, flush_comp, {}); + io::ImageRequest::aio_flush(&m_image_ctx, flush_comp, + io::FLUSH_SOURCE_INTERNAL, {}); } if (on_finish != nullptr) { on_finish->complete(0); @@ -291,7 +292,8 @@ void Replay::flush(Context *on_finish) { } RWLock::RLocker owner_locker(m_image_ctx.owner_lock); - io::ImageRequest::aio_flush(&m_image_ctx, aio_comp, {}); + io::ImageRequest::aio_flush(&m_image_ctx, aio_comp, + io::FLUSH_SOURCE_INTERNAL, {}); } template @@ -357,7 +359,8 @@ void Replay::handle_event(const journal::AioDiscardEvent &event, m_lock.Unlock(); if (flush_comp != nullptr) { - io::ImageRequest::aio_flush(&m_image_ctx, flush_comp, {}); + io::ImageRequest::aio_flush(&m_image_ctx, flush_comp, + io::FLUSH_SOURCE_INTERNAL, {}); } } } @@ -387,7 +390,8 @@ void Replay::handle_event(const journal::AioWriteEvent &event, m_lock.Unlock(); if (flush_comp != nullptr) { - io::ImageRequest::aio_flush(&m_image_ctx, flush_comp, {}); + io::ImageRequest::aio_flush(&m_image_ctx, flush_comp, + io::FLUSH_SOURCE_INTERNAL, {}); } } } @@ -405,7 +409,8 @@ void Replay::handle_event(const journal::AioFlushEvent &event, } if (aio_comp != nullptr) { - io::ImageRequest::aio_flush(&m_image_ctx, aio_comp, {}); + io::ImageRequest::aio_flush(&m_image_ctx, aio_comp, + io::FLUSH_SOURCE_INTERNAL, {}); } on_ready->complete(0); } @@ -435,7 +440,8 @@ void Replay::handle_event(const journal::AioWriteSameEvent &event, m_lock.Unlock(); if (flush_comp != nullptr) { - io::ImageRequest::aio_flush(&m_image_ctx, flush_comp, {}); + io::ImageRequest::aio_flush(&m_image_ctx, flush_comp, + io::FLUSH_SOURCE_INTERNAL, {}); } } } @@ -463,7 +469,8 @@ void Replay::handle_event(const journal::AioWriteSameEvent &event, auto flush_comp = create_aio_flush_completion(nullptr); m_lock.Unlock(); - io::ImageRequest::aio_flush(&m_image_ctx, flush_comp, {}); + io::ImageRequest::aio_flush(&m_image_ctx, flush_comp, + io::FLUSH_SOURCE_INTERNAL, {}); } } diff --git a/src/test/librbd/io/test_mock_ImageRequest.cc b/src/test/librbd/io/test_mock_ImageRequest.cc index bfea417ef24..17ca3abd604 100644 --- a/src/test/librbd/io/test_mock_ImageRequest.cc +++ b/src/test/librbd/io/test_mock_ImageRequest.cc @@ -197,7 +197,12 @@ struct TestMockIoImageRequest : public TestMockFixture { } void expect_flush(MockImageCtx &mock_image_ctx, int r) { - EXPECT_CALL(mock_image_ctx, flush(_)) + EXPECT_CALL(mock_image_ctx, flush_cache(_)) + .WillOnce(CompleteContext(r, mock_image_ctx.image_ctx->op_work_queue)); + } + + void expect_flush_async_operations(MockImageCtx &mock_image_ctx, int r) { + EXPECT_CALL(mock_image_ctx, flush_async_operations(_)) .WillOnce(CompleteContext(r, mock_image_ctx.image_ctx->op_work_queue)); } }; @@ -281,12 +286,14 @@ TEST_F(TestMockIoImageRequest, AioFlushJournalAppendDisabled) { InSequence seq; expect_user_flushed(mock_image_ctx); expect_is_journal_appending(mock_journal, false); + expect_flush_async_operations(mock_image_ctx, 0); expect_flush(mock_image_ctx, 0); C_SaferCond aio_comp_ctx; AioCompletion *aio_comp = AioCompletion::create_and_start( &aio_comp_ctx, ictx, AIO_TYPE_FLUSH); - MockImageFlushRequest mock_aio_image_flush(mock_image_ctx, aio_comp, {}); + MockImageFlushRequest mock_aio_image_flush(mock_image_ctx, aio_comp, + FLUSH_SOURCE_USER, {}); { RWLock::RLocker owner_locker(mock_image_ctx.owner_lock); mock_aio_image_flush.send(); diff --git a/src/test/librbd/journal/test_mock_Replay.cc b/src/test/librbd/journal/test_mock_Replay.cc index 4e91bb564b7..d5b5da852e4 100644 --- a/src/test/librbd/journal/test_mock_Replay.cc +++ b/src/test/librbd/journal/test_mock_Replay.cc @@ -48,7 +48,7 @@ struct ImageRequest { MOCK_METHOD1(aio_flush, void(AioCompletion *c)); static void aio_flush(MockReplayImageCtx *ictx, AioCompletion *c, - const ZTracer::Trace &parent_trace) { + FlushSource, const ZTracer::Trace &parent_trace) { assert(s_instance != nullptr); s_instance->aio_flush(c); } -- 2.39.5