librbd: moved flush / cache invalidate to resize state machine

Keep async_resize truly async by moving flush and invalidate cache
operations to individual states within the resize state machine.

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

diff --git a/src/librbd/AsyncResizeRequest.cc b/src/librbd/AsyncResizeRequest.cc
index 3edc3324f97bb6a2badc174ca57e24cccfe08669..d79832939595d10e1b6be82402f446a549b01c5f 100644 (file)
--- a/src/librbd/AsyncResizeRequest.cc
+++ b/src/librbd/AsyncResizeRequest.cc
@@ -37,6 +37,16 @@ bool AsyncResizeRequest::should_complete(int r)
   }
 
   switch (m_state) {
+  case STATE_FLUSH:
+    ldout(cct, 5) << "FLUSH" << dendl;
+    send_invalidate_cache();
+    break;
+
+  case STATE_INVALIDATE_CACHE:
+    ldout(cct, 5) << "INVALIDATE_CACHE" << dendl;
+    send_trim_image();
+    break;
+
   case STATE_TRIM_IMAGE:
     ldout(cct, 5) << "TRIM_IMAGE" << dendl;
     send_update_header();
@@ -74,25 +84,25 @@ void AsyncResizeRequest::send() {
   CephContext *cct = m_image_ctx.cct;
   if (m_original_size == m_new_size) {
     ldout(cct, 2) << this << " no change in size (" << m_original_size
-                 << " -> " << m_new_size << ")" << dendl; 
+                 << " -> " << m_new_size << ")" << dendl;
     m_state = STATE_FINISHED;
     complete(0);
   } else if (m_new_size > m_original_size) {
     ldout(cct, 2) << this << " expanding image (" << m_original_size
-                 << " -> " << m_new_size << ")" << dendl; 
+                 << " -> " << m_new_size << ")" << dendl;
     send_grow_object_map();
   } else {
     ldout(cct, 2) << this << " shrinking image (" << m_original_size
-                 << " -> " << m_new_size << ")" << dendl; 
-    send_trim_image();
+                 << " -> " << m_new_size << ")" << dendl;
+    send_flush();
   }
 }
 
-void AsyncResizeRequest::send_trim_image() {
-  ldout(m_image_ctx.cct, 5) << this << " send_trim_image: "
+void AsyncResizeRequest::send_flush() {
+  ldout(m_image_ctx.cct, 5) << this << " send_flush: "
                             << " original_size=" << m_original_size
                             << " new_size=" << m_new_size << dendl;
-  m_state = STATE_TRIM_IMAGE;
+  m_state = STATE_FLUSH;
 
   {
     // update in-memory size to clip concurrent IO operations
@@ -107,6 +117,28 @@ void AsyncResizeRequest::send_trim_image() {
     }
   }
 
+  // with clipping adjusted, ensure that write / copy-on-read operations won't
+  // (re-)create objects that we just removed
+  m_image_ctx.flush_async_operations(create_callback_context());
+}
+
+void AsyncResizeRequest::send_invalidate_cache() {
+  ldout(m_image_ctx.cct, 5) << this << " send_invalidate_cache: "
+                            << " original_size=" << m_original_size
+                            << " new_size=" << m_new_size << dendl;
+  m_state = STATE_INVALIDATE_CACHE;
+
+  // need to invalidate since we're deleting objects, and
+  // ObjectCacher doesn't track non-existent objects
+  m_image_ctx.invalidate_cache(create_callback_context());
+}
+
+void AsyncResizeRequest::send_trim_image() {
+  ldout(m_image_ctx.cct, 5) << this << " send_trim_image: "
+                            << " original_size=" << m_original_size
+                            << " new_size=" << m_new_size << dendl;
+  m_state = STATE_TRIM_IMAGE;
+
   AsyncTrimRequest *req = new AsyncTrimRequest(m_image_ctx,
                                               create_callback_context(),
                                               m_original_size, m_new_size,

diff --git a/src/librbd/AsyncResizeRequest.h b/src/librbd/AsyncResizeRequest.h
index 6884abf003936c0f7d66e57e55ff393143abfbdf..a341a3ae684414cd2ca0b1a8371bc50ffb9d4b2c 100644 (file)
--- a/src/librbd/AsyncResizeRequest.h
+++ b/src/librbd/AsyncResizeRequest.h
@@ -31,25 +31,35 @@ protected:
    * Resize goes through the following state machine to resize the image
    * and update the object map:
    *
-   * <start> ----> STATE_FINISHED --------------------------------\
-   *  |  .                                                        |
-   *  |  . . . . . . . . . . . . . . . . . .                      |
-   *  |                                    .                      |
-   *  |                                    v                      |
-   *  |---> STATE_GROW_OBJECT_MAP ---> STATE_UPDATE_HEADER -------|
-   *  |                                                           |
-   *  |                                                           |
-   *  \---> STATE_TRIM_IMAGE --------> STATE_UPDATE_HEADER . . .  |
-   *                                       |                   .  |
-   *                                       |                   .  |
-   *                                       v                   v  v
-   *                            STATE_SHRINK_OBJECT_MAP ---> <finish>
+   * <start> -------------> STATE_FINISHED -----------------------------\
+   *  |  .    (no change)                                               |
+   *  |  .                                                              |
+   *  |  . . . . . . . . . . . . . . . . . . . . .                      |
+   *  |                                          .                      |
+   *  |                                          v                      |
+   *  |----------> STATE_GROW_OBJECT_MAP ---> STATE_UPDATE_HEADER ------|
+   *  | (grow)                                                          |
+   *  |                                                                 |
+   *  |                                                                 |
+   *  \----------> STATE_FLUSH -------------> STATE_INVALIDATE_CACHE    |
+   *    (shrink)                                 |                      |
+   *                                             |                      |
+   *                      /----------------------/                      |
+   *                      |                                             |
+   *                      v                                             |
+   *              STATE_TRIM_IMAGE --------> STATE_UPDATE_HEADER . . .  |
+   *                                             |                   .  |
+   *                                             |                   .  |
+   *                                             v                   v  v
+   *                                  STATE_SHRINK_OBJECT_MAP ---> <finish>
    *
    * The _OBJECT_MAP states are skipped if the object map isn't enabled.
    * The state machine will immediately transition to _FINISHED if there
    * are no objects to trim.
-   */ 
+   */
   enum State {
+    STATE_FLUSH,
+    STATE_INVALIDATE_CACHE,
     STATE_TRIM_IMAGE,
     STATE_GROW_OBJECT_MAP,
     STATE_UPDATE_HEADER,
@@ -66,6 +76,8 @@ protected:
 
   virtual bool should_complete(int r);
 
+  void send_flush();
+  void send_invalidate_cache();
   void send_trim_image();
   void send_grow_object_map();
   bool send_shrink_object_map();

diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc
index 69e007dd60bfc13f850f5c1801d5027f73a5ee5a..0b8d3e6c37dd95469f0e6cdd76795f00c7050b19 100644 (file)
--- a/src/librbd/ImageCtx.cc
+++ b/src/librbd/ImageCtx.cc
@@ -741,15 +741,21 @@ namespace librbd {
   }
 
   void ImageCtx::flush_async_operations(Context *on_finish) {
-    Mutex::Locker l(async_ops_lock);
-    if (async_ops.empty()) {
-      on_finish->complete(0);
-      return;
+    bool complete = false;
+    {
+      Mutex::Locker l(async_ops_lock);
+      if (async_ops.empty()) {
+        complete = true;
+      } else {
+        ldout(cct, 20) << "flush async operations: " << on_finish << " "
+                       << "count=" << async_ops.size() << dendl;
+        async_ops.front()->add_flush_context(on_finish);
+      }
     }
 
-    ldout(cct, 20) << "flush async operations: " << on_finish << " "
-                   << "count=" << async_ops.size() << dendl;
-    async_ops.front()->add_flush_context(on_finish);
+    if (complete) {
+      on_finish->complete(0);
+    }
   }
 
   void ImageCtx::cancel_async_requests() {

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index be071ee65ea34c596027e73d3314e5455ab5e6dd..0fad9ea92ce228dc74f7159d8c88c8d37f68dbee 100644 (file)
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -1681,17 +1681,6 @@ reprotect_and_return_err:
       ictx->snap_lock.get_read();
       original_size = ictx->size;
       ictx->snap_lock.put_read();
-      if (size < original_size) {
-        ictx->flush_async_operations();
-       if (ictx->object_cacher) {
-         // need to invalidate since we're deleting objects, and
-         // ObjectCacher doesn't track non-existent objects
-         r = ictx->invalidate_cache();
-         if (r < 0) {
-           return r;
-         }
-       }
-      }
     }
 
     async_resize_helper(ictx, ctx, original_size, size, prog_ctx);