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>

diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc
index 556b1dba9077c66448ff7f01df9f4a700ab03dba..b39ef9302a54256383f00fe8ca36531347f1b5a3 100644 (file)
--- 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 2f2047ff594f4471da71a90cc5f426337fc3835b..25a1300ce8ed92f603bd7b945901f99c49a865c1 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 1bad485a93f5626c7c73489e59115a156f0c65e5..bfaa2ff49bc03e2f963546410873599d3412e1d6 100644 (file)
--- a/src/librbd/io/ImageRequest.cc
+++ b/src/librbd/io/ImageRequest.cc
@@ -708,8 +708,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 03a668da260d5641b7fb4df4ed951f91ab26dec8..e9034918ee7bdc287d9038eb9fca31707bc09818 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, 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 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 00733d2e48a2134ac8b147e7f55a0532880dfbbb..1975a842db00671e8bc8afc39be22fc6f2082ace 100644 (file)
--- 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));