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 <dillaman@redhat.com>
(cherry picked from commit 09e4127d5df1e2a79f2536dc784ec3730feea9ce)

diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc
index f6daba7d3d4f219fcc2d95bf3bbd39874d3d4473..19d3ce70c9bee57e7deb846b4be9a38771368705 100644 (file)
--- a/src/librbd/ImageCtx.cc
+++ b/src/librbd/ImageCtx.cc
@@ -693,25 +693,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 6e6eb3403b0e3ceb0a2e1853f81af32910ffd041..fe1c4828cabbd85dc030bf8009e63301bc36f89b 100644 (file)
--- a/src/librbd/ImageCtx.h
+++ b/src/librbd/ImageCtx.h
@@ -291,9 +291,6 @@ namespace librbd {
     uint64_t prune_parent_extents(vector<pair<uint64_t,uint64_t> >& 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 e2bdc3b306017815226f17017ad429b320580f88..c5a3bc932e0a6535b02de9f0780c45bd086854e3 100644 (file)
--- 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<Context *> flush_contexts;
 
-  explicit C_CompleteFlushes(ImageCtx *image_ctx, std::list<Context *> &&flush_contexts)
+  explicit C_CompleteFlushes(ImageCtx *image_ctx,
+                             std::list<Context *> &&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<AsyncOperation *>::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 8a01e5c742e307e839e09f2ad4acafe8e236bedc..b0a37c4b89a70d1bb01b1b40975c229fe106fdd3 100644 (file)
--- 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 5a74fabf0e96c7d96fb3f53daead53b006db74b6..2ddc2c3a5459f1c4abce1262bcbbd079f50e2ae7 100644 (file)
--- a/src/librbd/io/ImageRequest.cc
+++ b/src/librbd/io/ImageRequest.cc
@@ -639,8 +639,8 @@ void ImageFlushRequest<I>::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 823e5b0fe0bf70c3cbbd2942baf57875f6fc2ba6..53cdff8684e8cb8176babccab4d94a390fcec822 100644 (file)
--- 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, DeepCopyWithSnaps) {
@@ -723,7 +728,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) {
@@ -980,7 +985,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 5f5851c3751f689bb3509d39b9cc140b1ad667f3..0340580285132bb3b268dc1277b3e3b2bc504a38 100644 (file)
--- 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 5d2c3d28a5d781793b71c5ad3e6d9c25614c2773..4c45011d3ce54aa620f09c107ff2103f2802fecb 100644 (file)
--- a/src/test/librbd/mock/MockImageCtx.h
+++ b/src/test/librbd/mock/MockImageCtx.h
@@ -186,7 +186,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));