librbd: flush all queued object IO from simple scheduler

Normally IO is tracked via the AioCompletion's async_op but the
scheduler will "complete" writes while the IO might be still
executing. Therefore, prior to shutting down this dispatch layer
we need to wait for all IO to complete.

Fixes: https://tracker.ceph.com/issues/46668
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
(cherry picked from commit 75ff8fd14dccaa7d2f11ba8a561ad3c0f410585c)

diff --git a/src/librbd/io/SimpleSchedulerObjectDispatch.cc b/src/librbd/io/SimpleSchedulerObjectDispatch.cc
index 6b6a60c9862bf343863d0b36d51307d6794470d8..808b3b14d27c29d2674c2633ef9a7f0af79eb48a 100644 (file)
--- a/src/librbd/io/SimpleSchedulerObjectDispatch.cc
+++ b/src/librbd/io/SimpleSchedulerObjectDispatch.cc
@@ -7,6 +7,7 @@
 #include "common/errno.h"
 #include "librbd/ImageCtx.h"
 #include "librbd/Utils.h"
+#include "librbd/io/FlushTracker.h"
 #include "librbd/io/ObjectDispatchSpec.h"
 #include "librbd/io/ObjectDispatcher.h"
 #include "librbd/io/Utils.h"
@@ -174,6 +175,7 @@ template <typename I>
 SimpleSchedulerObjectDispatch<I>::SimpleSchedulerObjectDispatch(
     I* image_ctx)
   : m_image_ctx(image_ctx),
+    m_flush_tracker(new FlushTracker<I>(image_ctx)),
     m_lock(ceph::make_mutex(librbd::util::unique_lock_name(
       "librbd::io::SimpleSchedulerObjectDispatch::lock", this))),
     m_max_delay(image_ctx->config.template get_val<uint64_t>(
@@ -190,6 +192,7 @@ SimpleSchedulerObjectDispatch<I>::SimpleSchedulerObjectDispatch(
 
 template <typename I>
 SimpleSchedulerObjectDispatch<I>::~SimpleSchedulerObjectDispatch() {
+  delete m_flush_tracker;
 }
 
 template <typename I>
@@ -206,6 +209,7 @@ void SimpleSchedulerObjectDispatch<I>::shut_down(Context* on_finish) {
   auto cct = m_image_ctx->cct;
   ldout(cct, 5) << dendl;
 
+  m_flush_tracker->shut_down();
   on_finish->complete(0);
 }
 
@@ -260,6 +264,15 @@ bool SimpleSchedulerObjectDispatch<I>::write(
   std::lock_guard locker{m_lock};
   if (try_delay_write(object_no, object_off, std::move(data), snapc, op_flags,
                       *object_dispatch_flags, on_dispatched)) {
+
+    auto dispatch_seq = ++m_dispatch_seq;
+    m_flush_tracker->start_io(dispatch_seq);
+    *on_finish = new LambdaContext(
+      [this, dispatch_seq, ctx=*on_finish](int r) {
+        ctx->complete(r);
+        m_flush_tracker->finish_io(dispatch_seq);
+      });
+
     *dispatch_result = DISPATCH_RESULT_COMPLETE;
     return true;
   }
@@ -316,10 +329,15 @@ bool SimpleSchedulerObjectDispatch<I>::flush(
   auto cct = m_image_ctx->cct;
   ldout(cct, 20) << dendl;
 
-  std::lock_guard locker{m_lock};
-  dispatch_all_delayed_requests();
+  {
+    std::lock_guard locker{m_lock};
+    dispatch_all_delayed_requests();
+  }
 
-  return false;
+  *dispatch_result = DISPATCH_RESULT_CONTINUE;
+  m_flush_tracker->flush(on_dispatched);
+
+  return true;
 }
 
 template <typename I>
@@ -403,24 +421,30 @@ void SimpleSchedulerObjectDispatch<I>::register_in_flight_request(
   auto it = res.first;
 
   auto dispatch_seq = ++m_dispatch_seq;
+  m_flush_tracker->start_io(dispatch_seq);
+
   it->second->set_dispatch_seq(dispatch_seq);
   *on_finish = new LambdaContext(
     [this, object_no, dispatch_seq, start_time, ctx=*on_finish](int r) {
       ctx->complete(r);
 
-      std::lock_guard locker{m_lock};
+      std::unique_lock locker{m_lock};
       if (m_latency_stats && start_time != utime_t()) {
         auto latency = ceph_clock_now() - start_time;
         m_latency_stats->add(latency.to_nsec());
       }
+
       auto it = m_requests.find(object_no);
       if (it == m_requests.end() ||
           it->second->get_dispatch_seq() != dispatch_seq) {
         ldout(m_image_ctx->cct, 20) << "already dispatched" << dendl;
-        return;
+      } else {
+        dispatch_delayed_requests(it->second);
+        m_requests.erase(it);
       }
-      dispatch_delayed_requests(it->second);
-      m_requests.erase(it);
+      locker.unlock();
+
+      m_flush_tracker->finish_io(dispatch_seq);
     });
 }
 

diff --git a/src/librbd/io/SimpleSchedulerObjectDispatch.h b/src/librbd/io/SimpleSchedulerObjectDispatch.h
index e5a88371d7f3a19f2f1d34b9690adbbb05e2cca6..0cab97ff0ada961f8c978e96a64ea037cdc58df5 100644 (file)
--- a/src/librbd/io/SimpleSchedulerObjectDispatch.h
+++ b/src/librbd/io/SimpleSchedulerObjectDispatch.h
@@ -22,6 +22,7 @@ class ImageCtx;
 
 namespace io {
 
+template <typename> class FlushTracker;
 class LatencyStats;
 
 /**
@@ -176,6 +177,8 @@ private:
 
   ImageCtxT *m_image_ctx;
 
+  FlushTracker<ImageCtxT>* m_flush_tracker;
+
   ceph::mutex m_lock;
   SafeTimer *m_timer;
   ceph::mutex *m_timer_lock;

diff --git a/src/test/librbd/io/test_mock_SimpleSchedulerObjectDispatch.cc b/src/test/librbd/io/test_mock_SimpleSchedulerObjectDispatch.cc
index da376a9acd4457b645722f73ed9f17097c8fd833..ab2b0ba1d7c02d904731aede47121973eab58b4e 100644 (file)
--- a/src/test/librbd/io/test_mock_SimpleSchedulerObjectDispatch.cc
+++ b/src/test/librbd/io/test_mock_SimpleSchedulerObjectDispatch.cc
@@ -28,6 +28,25 @@ struct TypeTraits<MockTestImageCtx> {
   typedef ::MockSafeTimer SafeTimer;
 };
 
+template <>
+struct FlushTracker<MockTestImageCtx> {
+  FlushTracker(MockTestImageCtx*) {
+  }
+
+  void shut_down() {
+  }
+
+  void flush(Context*) {
+  }
+
+  void start_io(uint64_t) {
+  }
+
+  void finish_io(uint64_t) {
+  }
+
+};
+
 } // namespace io
 } // namespace librbd
 
@@ -208,10 +227,12 @@ TEST_F(TestMockIoSimpleSchedulerObjectDispatch, Flush) {
   MockSimpleSchedulerObjectDispatch
       mock_simple_scheduler_object_dispatch(&mock_image_ctx);
 
+  io::DispatchResult dispatch_result;
   C_SaferCond cond;
   Context *on_finish = &cond;
-  ASSERT_FALSE(mock_simple_scheduler_object_dispatch.flush(
-      FLUSH_SOURCE_USER, {}, nullptr, nullptr, &on_finish, nullptr));
+  ASSERT_TRUE(mock_simple_scheduler_object_dispatch.flush(
+      FLUSH_SOURCE_USER, {}, nullptr, &dispatch_result, &on_finish, nullptr));
+  ASSERT_EQ(io::DISPATCH_RESULT_CONTINUE, dispatch_result);
   ASSERT_EQ(on_finish, &cond); // not modified
   on_finish->complete(0);
   ASSERT_EQ(0, cond.wait());
@@ -251,7 +272,7 @@ TEST_F(TestMockIoSimpleSchedulerObjectDispatch, WriteDelayed) {
       &object_dispatch_flags, nullptr, &dispatch_result, &on_finish2,
       &on_dispatched));
   ASSERT_EQ(dispatch_result, io::DISPATCH_RESULT_COMPLETE);
-  ASSERT_EQ(on_finish2, &cond2);
+  ASSERT_NE(on_finish2, &cond2);
   ASSERT_NE(timer_task, nullptr);
 
   expect_dispatch_delayed_requests(mock_image_ctx, 0);
@@ -298,7 +319,7 @@ TEST_F(TestMockIoSimpleSchedulerObjectDispatch, WriteDelayedFlush) {
       &object_dispatch_flags, nullptr, &dispatch_result, &on_finish2,
       &on_dispatched));
   ASSERT_EQ(dispatch_result, io::DISPATCH_RESULT_COMPLETE);
-  ASSERT_EQ(on_finish2, &cond2);
+  ASSERT_NE(on_finish2, &cond2);
   ASSERT_NE(timer_task, nullptr);
 
   expect_dispatch_delayed_requests(mock_image_ctx, 0);
@@ -306,8 +327,9 @@ TEST_F(TestMockIoSimpleSchedulerObjectDispatch, WriteDelayedFlush) {
 
   C_SaferCond cond3;
   Context *on_finish3 = &cond3;
-  ASSERT_FALSE(mock_simple_scheduler_object_dispatch.flush(
-      FLUSH_SOURCE_USER, {}, nullptr, nullptr, &on_finish3, nullptr));
+  ASSERT_TRUE(mock_simple_scheduler_object_dispatch.flush(
+      FLUSH_SOURCE_USER, {}, nullptr, &dispatch_result, &on_finish3, nullptr));
+  ASSERT_EQ(io::DISPATCH_RESULT_CONTINUE, dispatch_result);
   ASSERT_EQ(on_finish3, &cond3);
 
   on_finish1->complete(0);
@@ -356,7 +378,7 @@ TEST_F(TestMockIoSimpleSchedulerObjectDispatch, WriteMerged) {
       &object_dispatch_flags, nullptr, &dispatch_result, &on_finish2,
       &on_dispatched2));
   ASSERT_EQ(dispatch_result, io::DISPATCH_RESULT_COMPLETE);
-  ASSERT_EQ(on_finish2, &cond2);
+  ASSERT_NE(on_finish2, &cond2);
   ASSERT_NE(timer_task, nullptr);
 
   object_off = 0;
@@ -370,7 +392,7 @@ TEST_F(TestMockIoSimpleSchedulerObjectDispatch, WriteMerged) {
       &object_dispatch_flags, nullptr, &dispatch_result, &on_finish3,
       &on_dispatched3));
   ASSERT_EQ(dispatch_result, io::DISPATCH_RESULT_COMPLETE);
-  ASSERT_EQ(on_finish3, &cond3);
+  ASSERT_NE(on_finish3, &cond3);
 
   object_off = 10;
   data.clear();
@@ -383,7 +405,7 @@ TEST_F(TestMockIoSimpleSchedulerObjectDispatch, WriteMerged) {
       &object_dispatch_flags, nullptr, &dispatch_result, &on_finish4,
       &on_dispatched4));
   ASSERT_EQ(dispatch_result, io::DISPATCH_RESULT_COMPLETE);
-  ASSERT_EQ(on_finish4, &cond4);
+  ASSERT_NE(on_finish4, &cond4);
 
   object_off = 30;
   data.clear();
@@ -396,7 +418,7 @@ TEST_F(TestMockIoSimpleSchedulerObjectDispatch, WriteMerged) {
       &object_dispatch_flags, nullptr, &dispatch_result, &on_finish5,
       &on_dispatched5));
   ASSERT_EQ(dispatch_result, io::DISPATCH_RESULT_COMPLETE);
-  ASSERT_EQ(on_finish5, &cond5);
+  ASSERT_NE(on_finish5, &cond5);
 
   object_off = 50;
   data.clear();
@@ -409,7 +431,7 @@ TEST_F(TestMockIoSimpleSchedulerObjectDispatch, WriteMerged) {
       &object_dispatch_flags, nullptr, &dispatch_result, &on_finish6,
       &on_dispatched6));
   ASSERT_EQ(dispatch_result, io::DISPATCH_RESULT_COMPLETE);
-  ASSERT_EQ(on_finish6, &cond6);
+  ASSERT_NE(on_finish6, &cond6);
 
   // expect two requests dispatched:
   // 0~40 (merged 0~10, 10~10, 20~10, 30~10) and 50~10
@@ -472,7 +494,7 @@ TEST_F(TestMockIoSimpleSchedulerObjectDispatch, WriteNonSequential) {
       &object_dispatch_flags, nullptr, &dispatch_result, &on_finish2,
       &on_dispatched2));
   ASSERT_EQ(dispatch_result, io::DISPATCH_RESULT_COMPLETE);
-  ASSERT_EQ(on_finish2, &cond2);
+  ASSERT_NE(on_finish2, &cond2);
   ASSERT_NE(timer_task, nullptr);
 
   expect_dispatch_delayed_requests(mock_image_ctx, 0);
@@ -536,7 +558,7 @@ TEST_F(TestMockIoSimpleSchedulerObjectDispatch, Mixed) {
       &object_dispatch_flags, nullptr, &dispatch_result, &on_finish2,
       &on_dispatched2));
   ASSERT_EQ(dispatch_result, io::DISPATCH_RESULT_COMPLETE);
-  ASSERT_EQ(on_finish2, &cond2);
+  ASSERT_NE(on_finish2, &cond2);
   ASSERT_NE(timer_task, nullptr);
 
   // write (3) 10~10 (delayed)
@@ -551,7 +573,7 @@ TEST_F(TestMockIoSimpleSchedulerObjectDispatch, Mixed) {
       0, object_off, std::move(data), mock_image_ctx.snapc, 0, {},
       &object_dispatch_flags, nullptr, &dispatch_result, &on_finish3,
       &on_dispatched3));
-  ASSERT_EQ(on_finish3, &cond3);
+  ASSERT_NE(on_finish3, &cond3);
 
   // discard (1) (non-seq io)
   // will dispatch the delayed writes (2) and (3) and wrap on_finish
@@ -581,7 +603,7 @@ TEST_F(TestMockIoSimpleSchedulerObjectDispatch, Mixed) {
       0, object_off, std::move(data), mock_image_ctx.snapc, 0, {},
       &object_dispatch_flags, nullptr, &dispatch_result, &on_finish5,
       &on_dispatched5));
-  ASSERT_EQ(on_finish5, &cond5);
+  ASSERT_NE(on_finish5, &cond5);
   ASSERT_NE(timer_task, nullptr);
 
   // discard (2) (non-seq io)
@@ -611,7 +633,7 @@ TEST_F(TestMockIoSimpleSchedulerObjectDispatch, Mixed) {
       0, object_off, std::move(data), mock_image_ctx.snapc, 0, {},
       &object_dispatch_flags, nullptr, &dispatch_result, &on_finish7,
       &on_dispatched7));
-  ASSERT_EQ(on_finish7, &cond7);
+  ASSERT_NE(on_finish7, &cond7);
   ASSERT_NE(timer_task, nullptr);
 
   // write (1) finishes
@@ -688,7 +710,7 @@ TEST_F(TestMockIoSimpleSchedulerObjectDispatch, DispatchQueue) {
       &object_dispatch_flags, nullptr, &dispatch_result, &on_finish2,
       &on_dispatched2));
   ASSERT_EQ(dispatch_result, io::DISPATCH_RESULT_COMPLETE);
-  ASSERT_EQ(on_finish2, &cond2);
+  ASSERT_NE(on_finish2, &cond2);
   ASSERT_NE(timer_task, nullptr);
 
   // send 2 writes to object 1
@@ -711,7 +733,7 @@ TEST_F(TestMockIoSimpleSchedulerObjectDispatch, DispatchQueue) {
       &object_dispatch_flags, nullptr, &dispatch_result, &on_finish4,
       &on_dispatched4));
   ASSERT_EQ(dispatch_result, io::DISPATCH_RESULT_COMPLETE);
-  ASSERT_EQ(on_finish4, &cond4);
+  ASSERT_NE(on_finish4, &cond4);
 
   // finish write (1) to object 0
   expect_dispatch_delayed_requests(mock_image_ctx, 0);
@@ -771,7 +793,7 @@ TEST_F(TestMockIoSimpleSchedulerObjectDispatch, Timer) {
       &object_dispatch_flags, nullptr, &dispatch_result, &on_finish2,
       &on_dispatched));
   ASSERT_EQ(dispatch_result, io::DISPATCH_RESULT_COMPLETE);
-  ASSERT_EQ(on_finish2, &cond2);
+  ASSERT_NE(on_finish2, &cond2);
   ASSERT_NE(timer_task, nullptr);
 
   expect_dispatch_delayed_requests(mock_image_ctx, 0);