librbd: AsyncObjectThrottle should always hold owner_lock

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
(cherry picked from commit c352bcdc0f63eea55677fe3c3b5f0c61347c0db3)

diff --git a/src/librbd/AsyncFlattenRequest.cc b/src/librbd/AsyncFlattenRequest.cc
index ebaf511ca29ff5efcc57d6a84bdb1895229ca595..1d6d6cf696b1aac31f3c32d71d3d0969c735f4b8 100644 (file)
--- a/src/librbd/AsyncFlattenRequest.cc
+++ b/src/librbd/AsyncFlattenRequest.cc
@@ -23,8 +23,8 @@ public:
   AsyncFlattenObjectContext(AsyncObjectThrottle &throttle, ImageCtx *image_ctx,
                             uint64_t object_size, ::SnapContext snapc,
                             uint64_t object_no)
-    : C_AsyncObjectThrottle(throttle), m_image_ctx(*image_ctx),
-      m_object_size(object_size), m_snapc(snapc), m_object_no(object_no)
+    : C_AsyncObjectThrottle(throttle, *image_ctx), m_object_size(object_size),
+      m_snapc(snapc), m_object_no(object_no)
   {
   }
 
@@ -76,7 +76,6 @@ public:
   }
 
 private:
-  ImageCtx &m_image_ctx;
   uint64_t m_object_size;
   ::SnapContext m_snapc;
   uint64_t m_object_no;
@@ -121,8 +120,8 @@ void AsyncFlattenRequest::send() {
       boost::lambda::_1, &m_image_ctx, m_object_size, m_snapc,
       boost::lambda::_2));
   AsyncObjectThrottle *throttle = new AsyncObjectThrottle(
-    *this, context_factory, create_callback_context(), m_prog_ctx, 0,
-    m_overlap_objects);
+    this, m_image_ctx, context_factory, create_callback_context(), m_prog_ctx,
+    0, m_overlap_objects);
   throttle->start_ops(cct->_conf->rbd_concurrent_management_ops);
 }
 

diff --git a/src/librbd/AsyncObjectThrottle.cc b/src/librbd/AsyncObjectThrottle.cc
index 85cfec8e0db1252ace0deb3d43321d553cc74326..2c7ccd10d68d60f3a8ea528e29094b677e7843af 100644 (file)
--- a/src/librbd/AsyncObjectThrottle.cc
+++ b/src/librbd/AsyncObjectThrottle.cc
@@ -2,25 +2,35 @@
 // vim: ts=8 sw=2 smarttab
 #include "librbd/AsyncObjectThrottle.h"
 #include "include/rbd/librbd.hpp"
+#include "common/RWLock.h"
 #include "librbd/AsyncRequest.h"
+#include "librbd/ImageCtx.h"
 #include "librbd/internal.h"
 
 namespace librbd
 {
 
-AsyncObjectThrottle::AsyncObjectThrottle(const AsyncRequest& async_request,
+void C_AsyncObjectThrottle::finish(int r) {
+  RWLock::RLocker l(m_image_ctx.owner_lock);
+  m_finisher.finish_op(r);
+}
+
+AsyncObjectThrottle::AsyncObjectThrottle(const AsyncRequest* async_request,
+                                         ImageCtx &image_ctx,
                                          const ContextFactory& context_factory,
                                         Context *ctx, ProgressContext &prog_ctx,
                                         uint64_t object_no,
                                         uint64_t end_object_no)
   : m_lock(unique_lock_name("librbd::AsyncThrottle::m_lock", this)),
-    m_async_request(async_request), m_context_factory(context_factory),
-    m_ctx(ctx), m_prog_ctx(prog_ctx), m_object_no(object_no),
-    m_end_object_no(end_object_no), m_current_ops(0), m_ret(0)
+    m_async_request(async_request), m_image_ctx(image_ctx),
+    m_context_factory(context_factory), m_ctx(ctx), m_prog_ctx(prog_ctx),
+    m_object_no(object_no), m_end_object_no(end_object_no), m_current_ops(0),
+    m_ret(0)
 {
 }
 
 void AsyncObjectThrottle::start_ops(uint64_t max_concurrent) {
+  assert(m_image_ctx.owner_lock.is_locked());
   bool complete;
   {
     Mutex::Locker l(m_lock);
@@ -39,6 +49,7 @@ void AsyncObjectThrottle::start_ops(uint64_t max_concurrent) {
 }
 
 void AsyncObjectThrottle::finish_op(int r) {
+  assert(m_image_ctx.owner_lock.is_locked());
   bool complete;
   {
     Mutex::Locker l(m_lock);
@@ -59,7 +70,7 @@ void AsyncObjectThrottle::finish_op(int r) {
 void AsyncObjectThrottle::start_next_op() {
   bool done = false;
   while (!done) {
-    if (m_async_request.is_canceled() && m_ret == 0) {
+    if (m_async_request->is_canceled() && m_ret == 0) {
       // allow in-flight ops to complete, but don't start new ops
       m_ret = -ERESTART;
       return;

diff --git a/src/librbd/AsyncObjectThrottle.h b/src/librbd/AsyncObjectThrottle.h
index 83d69d8c773c1cdd5d4c36cd3feb933e58bf9fd7..f7f254fb44617c32f58343131342d4e6e7871b35 100644 (file)
--- a/src/librbd/AsyncObjectThrottle.h
+++ b/src/librbd/AsyncObjectThrottle.h
@@ -13,6 +13,7 @@ namespace librbd
 {
 class AsyncRequest;
 class ProgressContext;
+struct ImageCtx;
 
 class AsyncObjectThrottleFinisher {
 public:
@@ -22,18 +23,19 @@ public:
 
 class C_AsyncObjectThrottle : public Context {
 public:
-  C_AsyncObjectThrottle(AsyncObjectThrottleFinisher &finisher)
-    : m_finisher(finisher)
+  C_AsyncObjectThrottle(AsyncObjectThrottleFinisher &finisher,
+                        ImageCtx &image_ctx)
+    : m_image_ctx(image_ctx), m_finisher(finisher)
   {
   }
 
-  virtual void finish(int r)
-  {
-    m_finisher.finish_op(r);
-  }
-
   virtual int send() = 0;
 
+protected:
+  ImageCtx &m_image_ctx;
+
+  virtual void finish(int r);
+
 private:
   AsyncObjectThrottleFinisher &m_finisher;
 };
@@ -43,7 +45,7 @@ public:
   typedef boost::function<C_AsyncObjectThrottle*(AsyncObjectThrottle&,
                                           uint64_t)> ContextFactory;
 
-  AsyncObjectThrottle(const AsyncRequest &async_request,
+  AsyncObjectThrottle(const AsyncRequest *async_request, ImageCtx &image_ctx,
                       const ContextFactory& context_factory, Context *ctx,
                      ProgressContext &prog_ctx, uint64_t object_no,
                      uint64_t end_object_no);
@@ -53,7 +55,8 @@ public:
 
 private:
   Mutex m_lock;
-  const AsyncRequest &m_async_request;
+  const AsyncRequest *m_async_request;
+  ImageCtx &m_image_ctx;
   ContextFactory m_context_factory;
   Context *m_ctx;
   ProgressContext &m_prog_ctx;

diff --git a/src/librbd/AsyncTrimRequest.cc b/src/librbd/AsyncTrimRequest.cc
index af325f612f57f574f776fbf2b955039bd205c2dc..1a52c317f2910733c6af143af5d1dfc620025bde 100644 (file)
--- a/src/librbd/AsyncTrimRequest.cc
+++ b/src/librbd/AsyncTrimRequest.cc
@@ -28,8 +28,7 @@ class AsyncTrimObjectContext : public C_AsyncObjectThrottle {
 public:
   AsyncTrimObjectContext(AsyncObjectThrottle &throttle, ImageCtx *image_ctx,
                         uint64_t object_no)
-    : C_AsyncObjectThrottle(throttle), m_image_ctx(*image_ctx),
-      m_object_no(object_no)
+    : C_AsyncObjectThrottle(throttle, *image_ctx), m_object_no(object_no)
   {
   }
 
@@ -56,7 +55,6 @@ public:
   }
 
 private:
-  ImageCtx &m_image_ctx;
   uint64_t m_object_no;
 };
 
@@ -150,7 +148,8 @@ void AsyncTrimRequest::send_remove_objects() {
     boost::lambda::bind(boost::lambda::new_ptr<AsyncTrimObjectContext>(),
       boost::lambda::_1, &m_image_ctx, boost::lambda::_2));
   AsyncObjectThrottle *throttle = new AsyncObjectThrottle(
-    *this, context_factory, ctx, m_prog_ctx, m_delete_start, m_num_objects);
+    this, m_image_ctx, context_factory, ctx, m_prog_ctx, m_delete_start,
+    m_num_objects);
   throttle->start_ops(cct->_conf->rbd_concurrent_management_ops);
 }