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 <dillaman@redhat.com>

diff --git a/src/librbd/AioRequest.cc b/src/librbd/AioRequest.cc
index fe97dc987db4856bad69423d6f60ac0bd75cd034..ed48c28f025260dc5bd566fdaeeaa8bb814f75af 100644 (file)
--- a/src/librbd/AioRequest.cc
+++ b/src/librbd/AioRequest.cc
@@ -416,6 +416,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)) {
@@ -451,6 +452,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 e403be2593fb32499d1d1bfa499b32112ec1d33a..971846a96163f1090df365b57e48c7afbf0f9c5a 100644 (file)
--- 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/CopyupRequest.cc b/src/librbd/CopyupRequest.cc
index 1b8ff2529df4ced3ad9708782dad618b530d1ed8..a63a59bc94bcbfb96b5f098753bb53684f36eaab 100644 (file)
--- a/src/librbd/CopyupRequest.cc
+++ b/src/librbd/CopyupRequest.cc
@@ -40,6 +40,7 @@ public:
     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());
       bool sent = m_image_ctx.object_map.aio_update(m_object_no, OBJECT_EXISTS,

diff --git a/src/librbd/ImageWatcher.cc b/src/librbd/ImageWatcher.cc
index 1f904319956049d843a8e02391bdd6820476c85b..2b0d93cabaebffe633b31fe8e82e88c492ee8f68 100644 (file)
--- a/src/librbd/ImageWatcher.cc
+++ b/src/librbd/ImageWatcher.cc
@@ -56,8 +56,13 @@ ImageWatcher::~ImageWatcher()
 }
 
 bool ImageWatcher::is_lock_supported() const {
-  assert(m_image_ctx.owner_lock.is_locked());
   RWLock::RLocker l(m_image_ctx.snap_lock);
+  return is_lock_supported(m_image_ctx.snap_lock);
+}
+
+bool ImageWatcher::is_lock_supported(const RWLock &) const {
+  assert(m_image_ctx.owner_lock.is_locked());
+  assert(m_image_ctx.snap_lock.is_locked());
   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/ImageWatcher.h b/src/librbd/ImageWatcher.h
index 73292b2d98ac25cee172b3904ae3c59ffd6cb5d6..ded4fe92876361bb37d0e5947985d2c53a47f57e 100644 (file)
--- a/src/librbd/ImageWatcher.h
+++ b/src/librbd/ImageWatcher.h
@@ -31,6 +31,7 @@ namespace librbd {
     ~ImageWatcher();
 
     bool is_lock_supported() const;
+    bool is_lock_supported(const RWLock &snap_lock) const;
     bool is_lock_owner() const;
 
     int register_watch();

diff --git a/src/librbd/ObjectMap.cc b/src/librbd/ObjectMap.cc
index d34339b0441c1755a62b7fa32f0242a3ae3c208b..1aa6d7407968ce0836de03af4842123ee5a26bf4 100644 (file)
--- a/src/librbd/ObjectMap.cc
+++ b/src/librbd/ObjectMap.cc
@@ -450,9 +450,10 @@ bool ObjectMap::aio_update(uint64_t start_object_no, uint64_t end_object_no,
                            const boost::optional<uint8_t> &current_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->is_lock_supported() ||
+  assert(!m_image_ctx.image_watcher->is_lock_supported(m_image_ctx.snap_lock) ||
          m_image_ctx.image_watcher->is_lock_owner());
   assert(m_image_ctx.object_map_lock.is_wlocked());
   assert(start_object_no < end_object_no);