From: Jason Dillaman <dillaman@redhat.com>
Date: Thu, 5 Jan 2017 17:12:57 +0000 (-0500)
Subject: librbd: possible deadlock with flush if refresh in-progress
X-Git-Tag: v12.0.0~223^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=b95f92a5572d3035c20eba07e76d2c825a9853f7;p=ceph.git

librbd: possible deadlock with flush if refresh in-progress

Fixes: http://tracker.ceph.com/issues/18419
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
---

diff --git a/src/librbd/ImageState.cc b/src/librbd/ImageState.cc
index 32043072769..a94572eaaee 100644
--- a/src/librbd/ImageState.cc
+++ b/src/librbd/ImageState.cc
@@ -304,7 +304,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>
@@ -336,7 +336,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()) {
@@ -344,14 +351,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 412730e7116..85494c4286e 100644
--- a/src/librbd/ImageState.h
+++ b/src/librbd/ImageState.h
@@ -112,6 +112,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 49e4e4b9f19..260ff1a0c3e 100644
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -2410,10 +2410,13 @@ void filter_out_mirror_watchers(ImageCtx *ictx,
     }
 
     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;
   }