From: Jason Dillaman Date: Mon, 29 Apr 2019 14:13:21 +0000 (-0400) Subject: librbd: simplify IO flush handling through AsyncOperation X-Git-Tag: v12.2.13~182^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=ef4fdf78da2d2fb0a05e59409b26d5f9597187c0;p=ceph.git librbd: simplify IO flush handling through AsyncOperation Allow ImageFlushRequest to directly execute a flush call through AsyncOperation. This will allow the flush to be directly linked to its preceeding IOs. Signed-off-by: Jason Dillaman (cherry picked from commit 09e4127d5df1e2a79f2536dc784ec3730feea9ce) Conflicts: src/librbd/ImageCtx.h/cc: dropped changes src/librbd/io/AsyncOperation.cc: kept original helper src/librbd/io/ImageRequest.cc: merged new logic in flush request src/test/librbd/io/test_mock_CopyupRequest.cc: DNE src/test/librbd/io/test_mock_ImageRequest.cc: tweaked to match new behavior --- diff --git a/src/librbd/io/AsyncOperation.cc b/src/librbd/io/AsyncOperation.cc index ca8daa4c098..40c2038c264 100644 --- a/src/librbd/io/AsyncOperation.cc +++ b/src/librbd/io/AsyncOperation.cc @@ -20,7 +20,8 @@ struct C_CompleteFlushes : public Context { ImageCtx *image_ctx; std::list flush_contexts; - explicit C_CompleteFlushes(ImageCtx *image_ctx, std::list &&flush_contexts) + explicit C_CompleteFlushes(ImageCtx *image_ctx, + std::list &&flush_contexts) : image_ctx(image_ctx), flush_contexts(std::move(flush_contexts)) { } void finish(int r) override { @@ -80,5 +81,21 @@ void AsyncOperation::add_flush_context(Context *on_finish) { m_flush_contexts.push_back(on_finish); } +void AsyncOperation::flush(Context* on_finish) { + { + Mutex::Locker locker(m_image_ctx->async_ops_lock); + xlist::iterator iter(&m_xlist_item); + ++iter; + + // linked list stored newest -> oldest ops + if (!iter.end()) { + (*iter)->m_flush_contexts.push_back(on_finish); + return; + } + } + + m_image_ctx->op_work_queue->queue(on_finish); +} + } // namespace io } // namespace librbd diff --git a/src/librbd/io/AsyncOperation.h b/src/librbd/io/AsyncOperation.h index 5839a6964ba..70da3359ecb 100644 --- a/src/librbd/io/AsyncOperation.h +++ b/src/librbd/io/AsyncOperation.h @@ -37,6 +37,7 @@ public: void finish_op(); void add_flush_context(Context *on_finish); + void flush(Context *on_finish); private: diff --git a/src/librbd/io/ImageRequest.cc b/src/librbd/io/ImageRequest.cc index b75f5aa06c1..cff22f7c016 100644 --- a/src/librbd/io/ImageRequest.cc +++ b/src/librbd/io/ImageRequest.cc @@ -100,28 +100,6 @@ struct C_DiscardJournalCommit : public Context { } }; -template -struct C_FlushJournalCommit : public Context { - ImageCtxT &image_ctx; - AioCompletion *aio_comp; - - C_FlushJournalCommit(ImageCtxT &_image_ctx, AioCompletion *_aio_comp, - uint64_t tid) - : image_ctx(_image_ctx), aio_comp(_aio_comp) { - CephContext *cct = image_ctx.cct; - ldout(cct, 20) << "delaying flush until journal tid " << tid << " " - << "safe" << dendl; - - aio_comp->add_request(); - } - - void finish(int r) override { - CephContext *cct = image_ctx.cct; - ldout(cct, 20) << "C_FlushJournalCommit: journal committed" << dendl; - aio_comp->complete_request(r); - } -}; - } // anonymous namespace template @@ -710,37 +688,37 @@ void ImageFlushRequest::send_request() { } AioCompletion *aio_comp = this->m_aio_comp; + aio_comp->set_request_count(1); + + Context* ctx = new C_AioRequest(aio_comp); + + // ensure no locks are held when flush is complete + ctx = util::create_async_context_callback(image_ctx, 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); - image_ctx.journal->flush_event(journal_tid, ctx); - - // track flush op for block writes - aio_comp->start_op(true); - aio_comp->put(); + ctx = new FunctionContext([ctx, &image_ctx, journal_tid](int r) { + image_ctx.journal->flush_event(journal_tid, ctx); }); - image_ctx.flush_async_operations(flush_ctx); - } else { + } else if (image_ctx.object_cacher != nullptr) { // 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 FunctionContext([ctx, &image_ctx](int r) { + image_ctx.flush_cache(ctx); + }); } + // ensure all in-flight IOs are settled if non-user flush request + aio_comp->start_op(true); + aio_comp->async_op.flush(ctx); + aio_comp->put(); + image_ctx.perfcounter->inc(l_librbd_aio_flush); } diff --git a/src/test/librbd/io/test_mock_ImageRequest.cc b/src/test/librbd/io/test_mock_ImageRequest.cc index 8460926d195..e2da7d52d7e 100644 --- a/src/test/librbd/io/test_mock_ImageRequest.cc +++ b/src/test/librbd/io/test_mock_ImageRequest.cc @@ -196,9 +196,11 @@ struct TestMockIoImageRequest : public TestMockFixture { EXPECT_CALL(mock_image_ctx, user_flushed()); } - void expect_flush(MockImageCtx &mock_image_ctx, int r) { - EXPECT_CALL(mock_image_ctx, flush(_)) - .WillOnce(CompleteContext(r, mock_image_ctx.image_ctx->op_work_queue)); + void expect_flush_cache(MockImageCtx &mock_image_ctx, int r) { + if (mock_image_ctx.object_cacher != nullptr) { + EXPECT_CALL(mock_image_ctx, flush_cache(_)) + .WillOnce(CompleteContext(r, mock_image_ctx.image_ctx->op_work_queue)); + } } }; @@ -280,10 +282,12 @@ TEST_F(TestMockIoImageRequest, AioFlushJournalAppendDisabled) { MockJournal mock_journal; mock_image_ctx.journal = &mock_journal; + expect_op_work_queue(mock_image_ctx); + InSequence seq; expect_user_flushed(mock_image_ctx); expect_is_journal_appending(mock_journal, false); - expect_flush(mock_image_ctx, 0); + expect_flush_cache(mock_image_ctx, 0); C_SaferCond aio_comp_ctx; AioCompletion *aio_comp = AioCompletion::create_and_start(