From: Jason Dillaman Date: Mon, 29 Apr 2019 14:13:21 +0000 (-0400) Subject: librbd: simplify IO flush handling through AsyncOperation X-Git-Tag: v15.1.0~2767^2~9 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=09e4127d5df1e2a79f2536dc784ec3730feea9ce;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 --- diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc index 556b1dba9077..b39ef9302a54 100644 --- a/src/librbd/ImageCtx.cc +++ b/src/librbd/ImageCtx.cc @@ -688,25 +688,6 @@ public: return len; } - void ImageCtx::flush_async_operations() { - C_SaferCond ctx; - flush_async_operations(&ctx); - ctx.wait(); - } - - void ImageCtx::flush_async_operations(Context *on_finish) { - { - Mutex::Locker l(async_ops_lock); - if (!async_ops.empty()) { - ldout(cct, 20) << "flush async operations: " << on_finish << " " - << "count=" << async_ops.size() << dendl; - async_ops.front()->add_flush_context(on_finish); - return; - } - } - on_finish->complete(0); - } - void ImageCtx::cancel_async_requests() { C_SaferCond ctx; cancel_async_requests(&ctx); diff --git a/src/librbd/ImageCtx.h b/src/librbd/ImageCtx.h index 2f2047ff594f..25a1300ce8ed 100644 --- a/src/librbd/ImageCtx.h +++ b/src/librbd/ImageCtx.h @@ -291,9 +291,6 @@ namespace librbd { uint64_t prune_parent_extents(vector >& objectx, uint64_t overlap); - void flush_async_operations(); - void flush_async_operations(Context *on_finish); - void cancel_async_requests(); void cancel_async_requests(Context *on_finish); diff --git a/src/librbd/io/AsyncOperation.cc b/src/librbd/io/AsyncOperation.cc index e2bdc3b30601..c5a3bc932e0a 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 { @@ -73,11 +74,20 @@ void AsyncOperation::finish_op() { } } -void AsyncOperation::add_flush_context(Context *on_finish) { - ceph_assert(m_image_ctx->async_ops_lock.is_locked()); - ldout(m_image_ctx->cct, 20) << this << " " << __func__ << ": " - << "flush=" << on_finish << dendl; - 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 diff --git a/src/librbd/io/AsyncOperation.h b/src/librbd/io/AsyncOperation.h index 8a01e5c742e3..b0a37c4b89a7 100644 --- a/src/librbd/io/AsyncOperation.h +++ b/src/librbd/io/AsyncOperation.h @@ -36,7 +36,7 @@ public: void start_op(ImageCtx &image_ctx); 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 1bad485a93f5..bfaa2ff49bc0 100644 --- a/src/librbd/io/ImageRequest.cc +++ b/src/librbd/io/ImageRequest.cc @@ -708,8 +708,8 @@ void ImageFlushRequest::send_request() { }); // 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->async_op.flush(ctx); aio_comp->put(); // might be flushing during image shutdown diff --git a/src/test/librbd/io/test_mock_CopyupRequest.cc b/src/test/librbd/io/test_mock_CopyupRequest.cc index 03a668da260d..e9034918ee7b 100644 --- a/src/test/librbd/io/test_mock_CopyupRequest.cc +++ b/src/test/librbd/io/test_mock_CopyupRequest.cc @@ -13,6 +13,7 @@ #include "librbd/deep_copy/ObjectCopyRequest.h" #include "librbd/io/CopyupRequest.h" #include "librbd/io/ImageRequest.h" +#include "librbd/io/ImageRequestWQ.h" #include "librbd/io/ObjectRequest.h" #include "librbd/io/ReadResult.h" @@ -310,6 +311,10 @@ struct TestMockIoCopyupRequest : public TestMockFixture { })); } + void flush_async_operations(librbd::ImageCtx* ictx) { + ictx->io_work_queue->flush(); + } + std::string m_parent_image_name; }; @@ -450,7 +455,7 @@ TEST_F(TestMockIoCopyupRequest, CopyOnRead) { {{0, 4096}}, {}); mock_image_ctx.copyup_list[0] = req; req->send(); - ictx->flush_async_operations(); + flush_async_operations(ictx); } TEST_F(TestMockIoCopyupRequest, CopyOnReadWithSnaps) { @@ -496,7 +501,7 @@ TEST_F(TestMockIoCopyupRequest, CopyOnReadWithSnaps) { {{0, 4096}}, {}); mock_image_ctx.copyup_list[0] = req; req->send(); - ictx->flush_async_operations(); + flush_async_operations(ictx); } TEST_F(TestMockIoCopyupRequest, DeepCopy) { @@ -580,7 +585,7 @@ TEST_F(TestMockIoCopyupRequest, DeepCopyOnRead) { {{0, 4096}}, {}); mock_image_ctx.copyup_list[0] = req; req->send(); - ictx->flush_async_operations(); + flush_async_operations(ictx); } TEST_F(TestMockIoCopyupRequest, DeepCopyWithPostSnaps) { @@ -797,7 +802,7 @@ TEST_F(TestMockIoCopyupRequest, ZeroedCopyOnRead) { {{0, 4096}}, {}); mock_image_ctx.copyup_list[0] = req; req->send(); - ictx->flush_async_operations(); + flush_async_operations(ictx); } TEST_F(TestMockIoCopyupRequest, NoOpCopyup) { @@ -1054,7 +1059,7 @@ TEST_F(TestMockIoCopyupRequest, CopyupError) { req->send(); ASSERT_EQ(-EPERM, mock_write_request.ctx.wait()); - ictx->flush_async_operations(); + flush_async_operations(ictx); } } // namespace io diff --git a/src/test/librbd/io/test_mock_ImageRequest.cc b/src/test/librbd/io/test_mock_ImageRequest.cc index 5f5851c3751f..034058028513 100644 --- a/src/test/librbd/io/test_mock_ImageRequest.cc +++ b/src/test/librbd/io/test_mock_ImageRequest.cc @@ -113,11 +113,6 @@ struct TestMockIoImageRequest : public TestMockFixture { mock_image_ctx.image_ctx->op_work_queue->queue(&spec->dispatcher_ctx, r); })); } - - 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)); - } }; TEST_F(TestMockIoImageRequest, AioWriteModifyTimestamp) { @@ -388,7 +383,6 @@ TEST_F(TestMockIoImageRequest, AioFlushJournalAppendDisabled) { InSequence seq; expect_is_journal_appending(mock_journal, false); - expect_flush_async_operations(mock_image_ctx, 0); expect_object_request_send(mock_image_ctx, 0); C_SaferCond aio_comp_ctx; diff --git a/src/test/librbd/mock/MockImageCtx.h b/src/test/librbd/mock/MockImageCtx.h index 00733d2e48a2..1975a842db00 100644 --- a/src/test/librbd/mock/MockImageCtx.h +++ b/src/test/librbd/mock/MockImageCtx.h @@ -185,7 +185,6 @@ struct MockImageCtx { librados::snap_t id)); MOCK_METHOD0(user_flushed, void()); - MOCK_METHOD1(flush_async_operations, void(Context *)); MOCK_METHOD1(flush_copyup, void(Context *)); MOCK_CONST_METHOD1(test_features, bool(uint64_t test_features));