librbd: cancel ops before blocking IO

Ops might have in-flight IO -- blocking IO after canceling the ops
will result in the in-flight IO being flushed.  Shutdown also requires
an intermediate state where is still acts like it downs the lock
until after all ops are canceled and all IO is flushed.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/librbd/ExclusiveLock.cc b/src/librbd/ExclusiveLock.cc
index 5c065fe4428a6702d0c55cacd9fcc848c0c354e3..fab631c6bccc9f92631c803144e186670d93d399 100644 (file)
--- a/src/librbd/ExclusiveLock.cc
+++ b/src/librbd/ExclusiveLock.cc
@@ -61,6 +61,7 @@ bool ExclusiveLock<I>::is_lock_owner() const {
   case STATE_LOCKED:
   case STATE_POST_ACQUIRING:
   case STATE_PRE_RELEASING:
+  case STATE_PRE_SHUTTING_DOWN:
     lock_owner = true;
     break;
   default:
@@ -214,6 +215,7 @@ bool ExclusiveLock<I>::is_transition_state() const {
   case STATE_POST_ACQUIRING:
   case STATE_PRE_RELEASING:
   case STATE_RELEASING:
+  case STATE_PRE_SHUTTING_DOWN:
   case STATE_SHUTTING_DOWN:
     return true;
   case STATE_UNINITIALIZED:
@@ -471,13 +473,13 @@ void ExclusiveLock<I>::send_shutdown() {
   if (m_state == STATE_UNLOCKED) {
     m_state = STATE_SHUTTING_DOWN;
     m_image_ctx.op_work_queue->queue(util::create_context_callback<
-      ExclusiveLock<I>, &ExclusiveLock<I>::handle_unlocked_shutdown>(this), 0);
+      ExclusiveLock<I>, &ExclusiveLock<I>::handle_shutdown>(this), 0);
     return;
   }
 
   ldout(m_image_ctx.cct, 10) << this << " " << __func__ << dendl;
   assert(m_state == STATE_LOCKED);
-  m_state = STATE_SHUTTING_DOWN;
+  m_state = STATE_PRE_SHUTTING_DOWN;
 
   m_lock.Unlock();
   m_image_ctx.op_work_queue->queue(new C_ShutDownRelease(this), 0);
@@ -494,13 +496,26 @@ void ExclusiveLock<I>::send_shutdown_release() {
 
   using el = ExclusiveLock<I>;
   ReleaseRequest<I>* req = ReleaseRequest<I>::create(
-    m_image_ctx, cookie, nullptr,
-    util::create_context_callback<el, &el::handle_locked_shutdown>(this));
+    m_image_ctx, cookie,
+    util::create_context_callback<el, &el::handle_shutdown_releasing>(this),
+    util::create_context_callback<el, &el::handle_shutdown_released>(this));
   req->send();
 }
 
 template <typename I>
-void ExclusiveLock<I>::handle_locked_shutdown(int r) {
+void ExclusiveLock<I>::handle_shutdown_releasing(int r) {
+  CephContext *cct = m_image_ctx.cct;
+  ldout(cct, 10) << this << " " << __func__ << ": r=" << r << dendl;
+
+  assert(r == 0);
+  assert(m_state == STATE_PRE_SHUTTING_DOWN);
+
+  // all IO and ops should be blocked/canceled by this point
+  m_state = STATE_SHUTTING_DOWN;
+}
+
+template <typename I>
+void ExclusiveLock<I>::handle_shutdown_released(int r) {
   CephContext *cct = m_image_ctx.cct;
   ldout(cct, 10) << this << " " << __func__ << ": r=" << r << dendl;
 
@@ -522,7 +537,7 @@ void ExclusiveLock<I>::handle_locked_shutdown(int r) {
 }
 
 template <typename I>
-void ExclusiveLock<I>::handle_unlocked_shutdown(int r) {
+void ExclusiveLock<I>::handle_shutdown(int r) {
   CephContext *cct = m_image_ctx.cct;
   ldout(cct, 10) << this << " " << __func__ << ": r=" << r << dendl;
 

diff --git a/src/librbd/ExclusiveLock.h b/src/librbd/ExclusiveLock.h
index c547372dd1af316787f5034c3ab9c56888457230..e2e14160a9cafb07c327c3ebb61641e449ad0c23 100644 (file)
--- a/src/librbd/ExclusiveLock.h
+++ b/src/librbd/ExclusiveLock.h
@@ -67,7 +67,7 @@ private:
    *    |
    *    |
    *    v
-   * SHUTTING_DOWN ---> SHUTDOWN ---> <finish>
+   * PRE_SHUTTING_DOWN ---> SHUTTING_DOWN ---> SHUTDOWN ---> <finish>
    */
   enum State {
     STATE_UNINITIALIZED,
@@ -79,6 +79,7 @@ private:
     STATE_WAITING_FOR_PEER,
     STATE_PRE_RELEASING,
     STATE_RELEASING,
+    STATE_PRE_SHUTTING_DOWN,
     STATE_SHUTTING_DOWN,
     STATE_SHUTDOWN,
   };
@@ -151,8 +152,9 @@ private:
 
   void send_shutdown();
   void send_shutdown_release();
-  void handle_locked_shutdown(int r);
-  void handle_unlocked_shutdown(int r);
+  void handle_shutdown_releasing(int r);
+  void handle_shutdown_released(int r);
+  void handle_shutdown(int r);
   void complete_shutdown(int r);
 };
 

diff --git a/src/librbd/exclusive_lock/ReleaseRequest.cc b/src/librbd/exclusive_lock/ReleaseRequest.cc
index 356beb1d12b09bb0d65bd8fbd3ac5fd9e7cd6892..0583c266d03891cb45aba34359bf8a0b66e0e19c 100644 (file)
--- a/src/librbd/exclusive_lock/ReleaseRequest.cc
+++ b/src/librbd/exclusive_lock/ReleaseRequest.cc
@@ -50,58 +50,58 @@ ReleaseRequest<I>::~ReleaseRequest() {
 
 template <typename I>
 void ReleaseRequest<I>::send() {
-  send_block_writes();
+  send_cancel_op_requests();
 }
 
 template <typename I>
-void ReleaseRequest<I>::send_block_writes() {
+void ReleaseRequest<I>::send_cancel_op_requests() {
   CephContext *cct = m_image_ctx.cct;
   ldout(cct, 10) << __func__ << dendl;
 
   using klass = ReleaseRequest<I>;
   Context *ctx = create_context_callback<
-    klass, &klass::handle_block_writes>(this);
-
-  {
-    RWLock::RLocker owner_locker(m_image_ctx.owner_lock);
-    if (m_image_ctx.test_features(RBD_FEATURE_JOURNALING)) {
-      m_image_ctx.aio_work_queue->set_require_lock_on_read();
-    }
-    m_image_ctx.aio_work_queue->block_writes(ctx);
-  }
+    klass, &klass::handle_cancel_op_requests>(this);
+  m_image_ctx.cancel_async_requests(ctx);
 }
 
 template <typename I>
-Context *ReleaseRequest<I>::handle_block_writes(int *ret_val) {
+Context *ReleaseRequest<I>::handle_cancel_op_requests(int *ret_val) {
   CephContext *cct = m_image_ctx.cct;
   ldout(cct, 10) << __func__ << ": r=" << *ret_val << dendl;
 
-  if (*ret_val < 0) {
-    m_image_ctx.aio_work_queue->unblock_writes();
-    return m_on_finish;
-  }
+  assert(*ret_val == 0);
 
-  send_cancel_op_requests();
+  send_block_writes();
   return nullptr;
 }
 
 template <typename I>
-void ReleaseRequest<I>::send_cancel_op_requests() {
+void ReleaseRequest<I>::send_block_writes() {
   CephContext *cct = m_image_ctx.cct;
   ldout(cct, 10) << __func__ << dendl;
 
   using klass = ReleaseRequest<I>;
   Context *ctx = create_context_callback<
-    klass, &klass::handle_cancel_op_requests>(this);
-  m_image_ctx.cancel_async_requests(ctx);
+    klass, &klass::handle_block_writes>(this);
+
+  {
+    RWLock::RLocker owner_locker(m_image_ctx.owner_lock);
+    if (m_image_ctx.test_features(RBD_FEATURE_JOURNALING)) {
+      m_image_ctx.aio_work_queue->set_require_lock_on_read();
+    }
+    m_image_ctx.aio_work_queue->block_writes(ctx);
+  }
 }
 
 template <typename I>
-Context *ReleaseRequest<I>::handle_cancel_op_requests(int *ret_val) {
+Context *ReleaseRequest<I>::handle_block_writes(int *ret_val) {
   CephContext *cct = m_image_ctx.cct;
   ldout(cct, 10) << __func__ << ": r=" << *ret_val << dendl;
 
-  assert(*ret_val == 0);
+  if (*ret_val < 0) {
+    m_image_ctx.aio_work_queue->unblock_writes();
+    return m_on_finish;
+  }
 
   if (m_on_releasing != nullptr) {
     // alert caller that we no longer own the exclusive lock

diff --git a/src/librbd/exclusive_lock/ReleaseRequest.h b/src/librbd/exclusive_lock/ReleaseRequest.h
index c5bf91cca99f6f782ef5107f9d2e3c1be69de52d..8712bc963cc8c8b234ae6d5a47968fcc8932dd82 100644 (file)
--- a/src/librbd/exclusive_lock/ReleaseRequest.h
+++ b/src/librbd/exclusive_lock/ReleaseRequest.h
@@ -33,10 +33,10 @@ private:
    * <start>
    *    |
    *    v
-   * BLOCK_WRITES
+   * CANCEL_OP_REQUESTS
    *    |
    *    v
-   * CANCEL_OP_REQUESTS
+   * BLOCK_WRITES
    *    |
    *    v
    * FLUSH_NOTIFIES . . . . . . . . . . . . . .
@@ -67,12 +67,12 @@ private:
   decltype(m_image_ctx.object_map) m_object_map;
   decltype(m_image_ctx.journal) m_journal;
 
-  void send_block_writes();
-  Context *handle_block_writes(int *ret_val);
-
   void send_cancel_op_requests();
   Context *handle_cancel_op_requests(int *ret_val);
 
+  void send_block_writes();
+  Context *handle_block_writes(int *ret_val);
+
   void send_flush_notifies();
   Context *handle_flush_notifies(int *ret_val);
 

diff --git a/src/test/librbd/exclusive_lock/test_mock_ReleaseRequest.cc b/src/test/librbd/exclusive_lock/test_mock_ReleaseRequest.cc
index eb42feb715ffd0e148823d711169b20406607590..2ed8b8ef9a54dd7178612ffde9927b15237f42c4 100644 (file)
--- a/src/test/librbd/exclusive_lock/test_mock_ReleaseRequest.cc
+++ b/src/test/librbd/exclusive_lock/test_mock_ReleaseRequest.cc
@@ -93,8 +93,8 @@ TEST_F(TestMockExclusiveLockReleaseRequest, Success) {
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
-  expect_block_writes(mock_image_ctx, 0);
   expect_cancel_op_requests(mock_image_ctx, 0);
+  expect_block_writes(mock_image_ctx, 0);
   expect_flush_notifies(mock_image_ctx);
 
   MockJournal *mock_journal = new MockJournal();
@@ -183,6 +183,7 @@ TEST_F(TestMockExclusiveLockReleaseRequest, BlockWritesError) {
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
+  expect_cancel_op_requests(mock_image_ctx, 0);
   expect_block_writes(mock_image_ctx, -EINVAL);
   expect_unblock_writes(mock_image_ctx);
 
@@ -204,8 +205,8 @@ TEST_F(TestMockExclusiveLockReleaseRequest, UnlockError) {
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
-  expect_block_writes(mock_image_ctx, 0);
   expect_cancel_op_requests(mock_image_ctx, 0);
+  expect_block_writes(mock_image_ctx, 0);
   expect_flush_notifies(mock_image_ctx);
 
   expect_unlock(mock_image_ctx, -EINVAL);