librbd: possible deadlock with flush if refresh in-progress

Fixes: http://tracker.ceph.com/issues/18419
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
(cherry picked from commit b95f92a5572d3035c20eba07e76d2c825a9853f7)

diff --git a/src/librbd/ImageState.cc b/src/librbd/ImageState.cc
index 28cf4274ef9ec02fc853d702ccf40dbe4b1f79b2..e1ba4a312dc6333a8dc2187ea428142af8bcc21a 100644 (file)
--- a/src/librbd/ImageState.cc
+++ b/src/librbd/ImageState.cc
@@ -306,7 +306,7 @@ void ImageState<I>::handle_update_notification() {
 template <typename I>
 bool ImageState<I>::is_refresh_required() const {
   Mutex::Locker locker(m_lock);
-  return (m_last_refresh != m_refresh_seq);
+  return (m_last_refresh != m_refresh_seq || find_pending_refresh() != nullptr);
 }
 
 template <typename I>
@@ -338,7 +338,14 @@ int ImageState<I>::refresh_if_required() {
   C_SaferCond ctx;
   {
     m_lock.Lock();
-    if (m_last_refresh == m_refresh_seq) {
+    Action action(ACTION_TYPE_REFRESH);
+    action.refresh_seq = m_refresh_seq;
+
+    auto refresh_action = find_pending_refresh();
+    if (refresh_action != nullptr) {
+      // if a refresh is in-flight, delay until it is finished
+      action = *refresh_action;
+    } else if (m_last_refresh == m_refresh_seq) {
       m_lock.Unlock();
       return 0;
     } else if (is_closed()) {
@@ -346,14 +353,28 @@ int ImageState<I>::refresh_if_required() {
       return -ESHUTDOWN;
     }
 
-    Action action(ACTION_TYPE_REFRESH);
-    action.refresh_seq = m_refresh_seq;
     execute_action_unlock(action, &ctx);
   }
 
   return ctx.wait();
 }
 
+template <typename I>
+const typename ImageState<I>::Action *
+ImageState<I>::find_pending_refresh() const {
+  assert(m_lock.is_locked());
+
+  auto it = std::find_if(m_actions_contexts.rbegin(),
+                         m_actions_contexts.rend(),
+                         [](const ActionContexts& action_contexts) {
+      return (action_contexts.first == ACTION_TYPE_REFRESH);
+    });
+  if (it != m_actions_contexts.rend()) {
+    return &it->first;
+  }
+  return nullptr;
+}
+
 template <typename I>
 void ImageState<I>::snap_set(const std::string &snap_name, Context *on_finish) {
   CephContext *cct = m_image_ctx->cct;

diff --git a/src/librbd/ImageState.h b/src/librbd/ImageState.h
index b6e7ce63b4426d9134b1aa8b4fcc83d662c6272e..73433ba8fc8d205ee3519bb13417f8cfa05f16a0 100644 (file)
--- a/src/librbd/ImageState.h
+++ b/src/librbd/ImageState.h
@@ -114,6 +114,8 @@ private:
   bool is_transition_state() const;
   bool is_closed() const;
 
+  const Action *find_pending_refresh() const;
+
   void append_context(const Action &action, Context *context);
   void execute_next_action_unlock();
   void execute_action_unlock(const Action &action, Context *context);

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index 84b4d977a5f79cec5b803c9af0cfa2a9c314766c..9c518ff7fad72d3f93d4d859afe0fe32200d1e38 100644 (file)
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -2364,10 +2364,13 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
     }
 
     ictx->user_flushed();
+    C_SaferCond ctx;
     {
       RWLock::RLocker owner_locker(ictx->owner_lock);
-      r = ictx->flush();
+      ictx->flush(&ctx);
     }
+    r = ctx.wait();
+
     ictx->perfcounter->inc(l_librbd_flush);
     return r;
   }