From 9454f04e62ece39fdcdbb4eb5a83945f76bcc0a5 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Thu, 30 Apr 2015 15:32:38 -0400 Subject: [PATCH] librbd: ObjectMap::aio_update can acquire snap_lock out-of-order Detected during an fsx run where a refresh and CoR were occurring concurrently. The refresh held the snap_lock and was waiting on the object_map_lock, while the CoR held object_map_lock and was waiting for snap_lock. Fixes: #11577 Signed-off-by: Jason Dillaman (cherry picked from commit 8cbd92b1fe835b1eb3a898976f9507f51cc115b2) --- src/librbd/AioRequest.cc | 2 ++ src/librbd/AsyncTrimRequest.cc | 2 ++ src/librbd/ImageWatcher.cc | 4 +--- src/librbd/ObjectMap.cc | 3 ++- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/librbd/AioRequest.cc b/src/librbd/AioRequest.cc index 486d0de78b85e..b4b72da295b24 100644 --- a/src/librbd/AioRequest.cc +++ b/src/librbd/AioRequest.cc @@ -407,6 +407,7 @@ namespace librbd { m_state = LIBRBD_AIO_WRITE_PRE; FunctionContext *ctx = new FunctionContext( boost::bind(&AioRequest::complete, this, _1)); + RWLock::RLocker snap_locker(m_ictx->snap_lock); RWLock::WLocker object_map_locker(m_ictx->object_map_lock); if (!m_ictx->object_map.aio_update(m_object_no, new_state, current_state, ctx)) { @@ -442,6 +443,7 @@ namespace librbd { m_state = LIBRBD_AIO_WRITE_POST; FunctionContext *ctx = new FunctionContext( boost::bind(&AioRequest::complete, this, _1)); + RWLock::RLocker snap_locker(m_ictx->snap_lock); RWLock::WLocker object_map_locker(m_ictx->object_map_lock); if (!m_ictx->object_map.aio_update(m_object_no, OBJECT_NONEXISTENT, OBJECT_PENDING, ctx)) { diff --git a/src/librbd/AsyncTrimRequest.cc b/src/librbd/AsyncTrimRequest.cc index cfcdb1554f5aa..a8bda30996b3c 100644 --- a/src/librbd/AsyncTrimRequest.cc +++ b/src/librbd/AsyncTrimRequest.cc @@ -172,6 +172,7 @@ void AsyncTrimRequest::send_pre_remove() { } else { // flag the objects as pending deletion Context *ctx = create_callback_context(); + RWLock::RLocker snap_locker(m_image_ctx.snap_lock); RWLock::WLocker object_map_locker(m_image_ctx.object_map_lock); if (!m_image_ctx.object_map.aio_update(m_delete_start, m_num_objects, OBJECT_PENDING, OBJECT_EXISTS, @@ -210,6 +211,7 @@ bool AsyncTrimRequest::send_post_remove() { } else { // flag the pending objects as removed Context *ctx = create_callback_context(); + RWLock::RLocker snap_lock(m_image_ctx.snap_lock); RWLock::WLocker object_map_locker(m_image_ctx.object_map_lock); if (!m_image_ctx.object_map.aio_update(m_delete_start, m_num_objects, OBJECT_NONEXISTENT, diff --git a/src/librbd/ImageWatcher.cc b/src/librbd/ImageWatcher.cc index a80e632469ddb..53bbd3034c98d 100644 --- a/src/librbd/ImageWatcher.cc +++ b/src/librbd/ImageWatcher.cc @@ -63,9 +63,7 @@ bool ImageWatcher::is_lock_supported() const { bool ImageWatcher::is_lock_supported(const RWLock &) const { assert(m_image_ctx.owner_lock.is_locked()); assert(m_image_ctx.snap_lock.is_locked()); - uint64_t snap_features; - m_image_ctx.get_features(m_image_ctx.snap_id, &snap_features); - return ((snap_features & RBD_FEATURE_EXCLUSIVE_LOCK) != 0 && + return ((m_image_ctx.features & RBD_FEATURE_EXCLUSIVE_LOCK) != 0 && !m_image_ctx.read_only && m_image_ctx.snap_id == CEPH_NOSNAP); } diff --git a/src/librbd/ObjectMap.cc b/src/librbd/ObjectMap.cc index 3505041d67753..9e7aae2d92025 100644 --- a/src/librbd/ObjectMap.cc +++ b/src/librbd/ObjectMap.cc @@ -302,7 +302,8 @@ bool ObjectMap::aio_update(uint64_t start_object_no, uint64_t end_object_no, const boost::optional ¤t_state, Context *on_finish) { - assert(m_image_ctx.test_features(RBD_FEATURE_OBJECT_MAP)); + assert(m_image_ctx.snap_lock.is_locked()); + assert((m_image_ctx.features & RBD_FEATURE_OBJECT_MAP) != 0); assert(m_image_ctx.owner_lock.is_locked()); assert(m_image_ctx.image_watcher != NULL); assert(m_image_ctx.image_watcher->is_lock_owner()); -- 2.39.5