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)

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 ca8daa4c098b18da111da17695548f715253d56f..40c2038c264208753e18c5fb8c479c673b71c95f 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 {
@@ -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<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
 } // namespace librbd

diff --git a/src/librbd/io/AsyncOperation.h b/src/librbd/io/AsyncOperation.h
index 5839a6964ba3de920c77d6656afec315b220b555..70da3359ecb17f0ce3d30d47bfb5d5fb2d0672fd 100644 (file)
--- 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 b75f5aa06c128932ac23a7cf50c3d5cf82c1ad14..cff22f7c016fd66b4444e5ae4839515f54616937 100644 (file)
--- a/src/librbd/io/ImageRequest.cc
+++ b/src/librbd/io/ImageRequest.cc
@@ -100,28 +100,6 @@ struct C_DiscardJournalCommit : public Context {
   }
 };
 
-template <typename ImageCtxT = ImageCtx>
-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 <typename I>
@@ -710,37 +688,37 @@ void ImageFlushRequest<I>::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<I>(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 8460926d19568ea59eb01209f3e7cee931ff7da5..e2da7d52d7e49ed1c2cfbe502674db43a823b26b 100644 (file)
--- 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(