librbd: AsyncObjectThrottle should always hold owner_lock

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

diff --git a/src/librbd/AsyncFlattenRequest.cc b/src/librbd/AsyncFlattenRequest.cc
index de90dc13765d7eaf5a6c4f8a83dd565bc2fd93a5..b6184974ca11a10f0961c864bb44218349fa97de 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)
   {
   }
 
@@ -54,7 +54,6 @@ public:
   }
 
 private:
-  ImageCtx &m_image_ctx;
   uint64_t m_object_size;
   ::SnapContext m_snapc;
   uint64_t m_object_no;
@@ -99,8 +98,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(m_image_ctx.concurrent_management_ops);
 }
 

diff --git a/src/librbd/AsyncObjectThrottle.cc b/src/librbd/AsyncObjectThrottle.cc
index 9fd5d38a2a1cbbc054587213eaa8a11f316fb109..e6eb4aba96259f37c837e59532914e3e57e116f2 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
 {
 
+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);

diff --git a/src/librbd/AsyncObjectThrottle.h b/src/librbd/AsyncObjectThrottle.h
index 3ac9561dc94c51a6d9a114daef3742f07f90c3c7..222baf00d1bdd4ad5c62718b14b16915cf3d5f8c 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);
@@ -54,6 +56,7 @@ public:
 private:
   Mutex m_lock;
   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 1a712bcfd0728ff88483e5f24563e6623e0403cc..73507907a314cb287babd06bdcc6200d71611c16 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;
 };
 
@@ -149,7 +147,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(m_image_ctx.concurrent_management_ops);
 }
 

diff --git a/src/librbd/CopyupRequest.cc b/src/librbd/CopyupRequest.cc
index 6e8f922c54d0510d90c13a3bc291908df349e588..8f98165d47b186578d809c6fbf6a4ab873d31428 100644 (file)
--- a/src/librbd/CopyupRequest.cc
+++ b/src/librbd/CopyupRequest.cc
@@ -31,15 +31,15 @@ public:
   UpdateObjectMap(AsyncObjectThrottle &throttle, ImageCtx *image_ctx,
                   uint64_t object_no, const std::vector<uint64_t> *snap_ids,
                   size_t snap_id_idx)
-    : C_AsyncObjectThrottle(throttle), m_image_ctx(*image_ctx),
+    : C_AsyncObjectThrottle(throttle, *image_ctx),
       m_object_no(object_no), m_snap_ids(*snap_ids), m_snap_id_idx(snap_id_idx)
   {
   }
 
   virtual int send() {
+    assert(m_image_ctx.owner_lock.is_locked());
     uint64_t snap_id = m_snap_ids[m_snap_id_idx];
     if (snap_id == CEPH_NOSNAP) {
-      RWLock::RLocker owner_locker(m_image_ctx.owner_lock);
       RWLock::RLocker snap_locker(m_image_ctx.snap_lock);
       RWLock::WLocker object_map_locker(m_image_ctx.object_map_lock);
       assert(m_image_ctx.image_watcher->is_lock_owner());
@@ -62,7 +62,6 @@ public:
   }
 
 private:
-  ImageCtx &m_image_ctx;
   uint64_t m_object_no;
   const std::vector<uint64_t> &m_snap_ids;
   size_t m_snap_id_idx;
@@ -295,12 +294,13 @@ private:
                              << dendl;
       m_state = STATE_OBJECT_MAP;
 
+      RWLock::RLocker owner_locker(m_ictx->owner_lock);
       AsyncObjectThrottle::ContextFactory context_factory(
         boost::lambda::bind(boost::lambda::new_ptr<UpdateObjectMap>(),
         boost::lambda::_1, m_ictx, m_object_no, &m_snap_ids,
         boost::lambda::_2));
       AsyncObjectThrottle *throttle = new AsyncObjectThrottle(
-        NULL, context_factory, create_callback_context(), NULL, 0,
+        NULL, *m_ictx, context_factory, create_callback_context(), NULL, 0,
         m_snap_ids.size());
       throttle->start_ops(m_ictx->concurrent_management_ops);
     }

diff --git a/src/librbd/RebuildObjectMapRequest.cc b/src/librbd/RebuildObjectMapRequest.cc
index 4c7c562d557dcb3ee961d838a8e274c805fbb17a..8891609ebdc372a60ca79713cbec4abcd40b8a30 100644 (file)
--- a/src/librbd/RebuildObjectMapRequest.cc
+++ b/src/librbd/RebuildObjectMapRequest.cc
@@ -26,9 +26,8 @@ class C_VerifyObject : public C_AsyncObjectThrottle {
 public:
   C_VerifyObject(AsyncObjectThrottle &throttle, ImageCtx *image_ctx,
                  uint64_t snap_id, uint64_t object_no)
-    : C_AsyncObjectThrottle(throttle), m_image_ctx(*image_ctx),
-      m_snap_id(snap_id), m_object_no(object_no),
-      m_oid(m_image_ctx.get_object_name(m_object_no))
+    : C_AsyncObjectThrottle(throttle, *image_ctx), m_snap_id(snap_id),
+      m_object_no(object_no), m_oid(m_image_ctx.get_object_name(m_object_no))
   {
     m_io_ctx.dup(m_image_ctx.md_ctx);
     m_io_ctx.snap_set_read(CEPH_SNAPDIR);
@@ -49,7 +48,6 @@ public:
   }
 
 private:
-  ImageCtx &m_image_ctx;
   librados::IoCtx m_io_ctx;
   uint64_t m_snap_id;
   uint64_t m_object_no;
@@ -321,8 +319,8 @@ void RebuildObjectMapRequest::send_verify_objects() {
     boost::lambda::bind(boost::lambda::new_ptr<C_VerifyObject>(),
       boost::lambda::_1, &m_image_ctx, snap_id, boost::lambda::_2));
   AsyncObjectThrottle *throttle = new AsyncObjectThrottle(
-    this, context_factory, create_callback_context(), &m_prog_ctx, 0,
-    num_objects);
+    this, m_image_ctx, context_factory, create_callback_context(), &m_prog_ctx,
+    0, num_objects);
   throttle->start_ops(cct->_conf->rbd_concurrent_management_ops);
 }