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>
(cherry picked from commit 8cbd92b1fe835b1eb3a898976f9507f51cc115b2)

diff --git a/src/librbd/AioRequest.cc b/src/librbd/AioRequest.cc
index 486d0de78b85e616cf8eec3d41272e70f5a07331..b4b72da295b242f20ba9f27952c0b83880c7bca7 100644 (file)
--- 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 cfcdb1554f5aa4900f13019a73098ab3606f0394..a8bda30996b3c2546d9ea3a0ca68db37d93e023e 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/ImageWatcher.cc b/src/librbd/ImageWatcher.cc
index a80e632469ddba22e34d399b545ff38cdbcb4643..53bbd3034c98d89b1610bcba15aff49f21835669 100644 (file)
--- 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 3505041d6775364a15ff4d2838119a3ddaa37b2d..9e7aae2d92025c0a99746909971ed4e863ccd0f6 100644 (file)
--- 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<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 != NULL);
   assert(m_image_ctx.image_watcher->is_lock_owner());