From c352bcdc0f63eea55677fe3c3b5f0c61347c0db3 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Thu, 30 Apr 2015 15:41:59 -0400 Subject: [PATCH] librbd: AsyncObjectThrottle should always hold owner_lock Signed-off-by: Jason Dillaman --- src/librbd/AsyncFlattenRequest.cc | 9 ++++----- src/librbd/AsyncObjectThrottle.cc | 17 ++++++++++++++--- src/librbd/AsyncObjectThrottle.h | 19 +++++++++++-------- src/librbd/AsyncTrimRequest.cc | 7 +++---- src/librbd/CopyupRequest.cc | 8 ++++---- src/librbd/RebuildObjectMapRequest.cc | 10 ++++------ 6 files changed, 40 insertions(+), 30 deletions(-) diff --git a/src/librbd/AsyncFlattenRequest.cc b/src/librbd/AsyncFlattenRequest.cc index de90dc13765d7..b6184974ca11a 100644 --- 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 9fd5d38a2a1cb..e6eb4aba96259 100644 --- 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 3ac9561dc94c5..222baf00d1bdd 100644 --- 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 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 1a712bcfd0728..73507907a314c 100644 --- 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(), 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 6e8f922c54d05..8f98165d47b18 100644 --- 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 *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 &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(), 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 4c7c562d557dc..8891609ebdc37 100644 --- 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(), 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); } -- 2.47.3