librbd: protect object map with a private member lock

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

diff --git a/src/librbd/ObjectMap.cc b/src/librbd/ObjectMap.cc
index 4eedf80bba1f724da73b612aa09947fef273388c..ad86a8c7b162e260a69a353f91e5a32c9288919e 100644 (file)
--- a/src/librbd/ObjectMap.cc
+++ b/src/librbd/ObjectMap.cc
@@ -35,6 +35,7 @@ namespace librbd {
 template <typename I>
 ObjectMap<I>::ObjectMap(I &image_ctx, uint64_t snap_id)
   : m_image_ctx(image_ctx), m_snap_id(snap_id),
+    m_lock(util::unique_lock_name("librbd::ObjectMap::lock", this)),
     m_update_guard(new UpdateGuard(m_image_ctx.cct)) {
 }
 
@@ -68,18 +69,10 @@ bool ObjectMap<I>::is_compatible(const file_layout_t& layout, uint64_t size) {
   return (object_count <= cls::rbd::MAX_OBJECT_MAP_OBJECT_COUNT);
 }
 
-template <typename I>
-ceph::BitVector<2u>::Reference ObjectMap<I>::operator[](uint64_t object_no)
-{
-  ceph_assert(m_image_ctx.object_map_lock.is_wlocked());
-  ceph_assert(object_no < m_object_map.size());
-  return m_object_map[object_no];
-}
-
 template <typename I>
 uint8_t ObjectMap<I>::operator[](uint64_t object_no) const
 {
-  ceph_assert(m_image_ctx.object_map_lock.is_locked());
+  RWLock::RLocker locker(m_lock);
   ceph_assert(object_no < m_object_map.size());
   return m_object_map[object_no];
 }
@@ -103,7 +96,6 @@ bool ObjectMap<I>::object_may_exist(uint64_t object_no) const
     return true;
   }
 
-  RWLock::RLocker l(m_image_ctx.object_map_lock);
   uint8_t state = (*this)[object_no];
   bool exists = (state != OBJECT_NONEXISTENT);
   ldout(m_image_ctx.cct, 20) << "object_no=" << object_no << " r=" << exists
@@ -130,7 +122,6 @@ bool ObjectMap<I>::object_may_not_exist(uint64_t object_no) const
     return true;
   }
 
-  RWLock::RLocker l(m_image_ctx.object_map_lock);
   uint8_t state = (*this)[object_no];
   bool nonexistent = (state != OBJECT_EXISTS && state != OBJECT_EXISTS_CLEAN);
   ldout(m_image_ctx.cct, 20) << "object_no=" << object_no << " r="
@@ -141,7 +132,7 @@ bool ObjectMap<I>::object_may_not_exist(uint64_t object_no) const
 template <typename I>
 bool ObjectMap<I>::update_required(const ceph::BitVector<2>::Iterator& it,
                                    uint8_t new_state) {
-  ceph_assert(m_image_ctx.object_map_lock.is_wlocked());
+  ceph_assert(m_lock.is_locked());
   uint8_t state = *it;
   if ((state == new_state) ||
       (new_state == OBJECT_PENDING && state == OBJECT_NONEXISTENT) ||
@@ -154,7 +145,7 @@ bool ObjectMap<I>::update_required(const ceph::BitVector<2>::Iterator& it,
 template <typename I>
 void ObjectMap<I>::open(Context *on_finish) {
   auto req = object_map::RefreshRequest<I>::create(
-    m_image_ctx, &m_object_map, m_snap_id, on_finish);
+    m_image_ctx, &m_lock, &m_object_map, m_snap_id, on_finish);
   req->send();
 }
 
@@ -175,7 +166,7 @@ bool ObjectMap<I>::set_object_map(ceph::BitVector<2> &target_object_map) {
   ceph_assert(m_image_ctx.image_lock.is_locked());
   ceph_assert(m_image_ctx.test_features(RBD_FEATURE_OBJECT_MAP,
                                         m_image_ctx.image_lock));
-  RWLock::RLocker object_map_locker(m_image_ctx.object_map_lock);
+  RWLock::WLocker locker(m_lock);
   m_object_map = target_object_map;
   return true;
 }
@@ -183,8 +174,8 @@ bool ObjectMap<I>::set_object_map(ceph::BitVector<2> &target_object_map) {
 template <typename I>
 void ObjectMap<I>::rollback(uint64_t snap_id, Context *on_finish) {
   ceph_assert(m_image_ctx.image_lock.is_locked());
-  ceph_assert(m_image_ctx.object_map_lock.is_wlocked());
 
+  RWLock::WLocker locker(m_lock);
   object_map::SnapshotRollbackRequest *req =
     new object_map::SnapshotRollbackRequest(m_image_ctx, snap_id, on_finish);
   req->send();
@@ -197,8 +188,8 @@ void ObjectMap<I>::snapshot_add(uint64_t snap_id, Context *on_finish) {
   ceph_assert(snap_id != CEPH_NOSNAP);
 
   object_map::SnapshotCreateRequest *req =
-    new object_map::SnapshotCreateRequest(m_image_ctx, &m_object_map, snap_id,
-                                          on_finish);
+    new object_map::SnapshotCreateRequest(m_image_ctx, &m_lock, &m_object_map,
+                                          snap_id, on_finish);
   req->send();
 }
 
@@ -209,8 +200,8 @@ void ObjectMap<I>::snapshot_remove(uint64_t snap_id, Context *on_finish) {
   ceph_assert(snap_id != CEPH_NOSNAP);
 
   object_map::SnapshotRemoveRequest *req =
-    new object_map::SnapshotRemoveRequest(m_image_ctx, &m_object_map, snap_id,
-                                          on_finish);
+    new object_map::SnapshotRemoveRequest(m_image_ctx, &m_lock, &m_object_map,
+                                          snap_id, on_finish);
   req->send();
 }
 
@@ -220,7 +211,7 @@ void ObjectMap<I>::aio_save(Context *on_finish) {
   ceph_assert(m_image_ctx.image_lock.is_locked());
   ceph_assert(m_image_ctx.test_features(RBD_FEATURE_OBJECT_MAP,
                                         m_image_ctx.image_lock));
-  RWLock::RLocker object_map_locker(m_image_ctx.object_map_lock);
+  RWLock::RLocker locker(m_lock);
 
   librados::ObjectWriteOperation op;
   if (m_snap_id == CEPH_NOSNAP) {
@@ -248,8 +239,8 @@ void ObjectMap<I>::aio_resize(uint64_t new_size, uint8_t default_object_state,
               m_image_ctx.exclusive_lock->is_lock_owner());
 
   object_map::ResizeRequest *req = new object_map::ResizeRequest(
-    m_image_ctx, &m_object_map, m_snap_id, new_size, default_object_state,
-    on_finish);
+    m_image_ctx, &m_lock, &m_object_map, m_snap_id, new_size,
+    default_object_state, on_finish);
   req->send();
 }
 
@@ -259,7 +250,7 @@ void ObjectMap<I>::detained_aio_update(UpdateOperation &&op) {
   ldout(cct, 20) << dendl;
 
   ceph_assert(m_image_ctx.image_lock.is_locked());
-  ceph_assert(m_image_ctx.object_map_lock.is_wlocked());
+  ceph_assert(m_lock.is_wlocked());
 
   BlockGuardCell *cell;
   int r = m_update_guard->detain({op.start_object_no, op.end_object_no},
@@ -300,7 +291,7 @@ void ObjectMap<I>::handle_detained_aio_update(BlockGuardCell *cell, int r,
 
   {
     RWLock::RLocker image_locker(m_image_ctx.image_lock);
-    RWLock::WLocker object_map_locker(m_image_ctx.object_map_lock);
+    RWLock::WLocker locker(m_lock);
     for (auto &op : block_ops) {
       detained_aio_update(std::move(op));
     }
@@ -320,8 +311,6 @@ void ObjectMap<I>::aio_update(uint64_t snap_id, uint64_t start_object_no,
   ceph_assert(m_image_ctx.image_watcher != nullptr);
   ceph_assert(m_image_ctx.exclusive_lock == nullptr ||
               m_image_ctx.exclusive_lock->is_lock_owner());
-  ceph_assert(snap_id != CEPH_NOSNAP ||
-              m_image_ctx.object_map_lock.is_wlocked());
   ceph_assert(start_object_no < end_object_no);
 
   CephContext *cct = m_image_ctx.cct;
@@ -331,6 +320,7 @@ void ObjectMap<I>::aio_update(uint64_t snap_id, uint64_t start_object_no,
                        stringify(static_cast<uint32_t>(*current_state)) : "")
                 << "->" << static_cast<uint32_t>(new_state) << dendl;
   if (snap_id == CEPH_NOSNAP) {
+    ceph_assert(m_lock.is_wlocked());
     end_object_no = std::min(end_object_no, m_object_map.size());
     if (start_object_no >= end_object_no) {
       ldout(cct, 20) << "skipping update of invalid object map" << dendl;
@@ -353,8 +343,9 @@ void ObjectMap<I>::aio_update(uint64_t snap_id, uint64_t start_object_no,
   }
 
   auto req = object_map::UpdateRequest<I>::create(
-    m_image_ctx, &m_object_map, snap_id, start_object_no, end_object_no,
-    new_state, current_state, parent_trace, ignore_enoent, on_finish);
+    m_image_ctx, &m_lock, &m_object_map, snap_id, start_object_no,
+    end_object_no, new_state, current_state, parent_trace, ignore_enoent,
+    on_finish);
   req->send();
 }
 

diff --git a/src/librbd/ObjectMap.h b/src/librbd/ObjectMap.h
index c930a5b554a353dcf224c99f66999a177da5ae37..3f7cdd04e0152fdaff203216e9b2547cc06409ea 100644 (file)
--- a/src/librbd/ObjectMap.h
+++ b/src/librbd/ObjectMap.h
@@ -9,11 +9,11 @@
 #include "include/rados/librados_fwd.hpp"
 #include "include/rbd/object_map_types.h"
 #include "common/bit_vector.hpp"
+#include "common/RWLock.h"
 #include "librbd/Utils.h"
 #include <boost/optional.hpp>
 
 class Context;
-class RWLock;
 namespace ZTracer { struct Trace; }
 
 namespace librbd {
@@ -38,12 +38,22 @@ public:
 
   static bool is_compatible(const file_layout_t& layout, uint64_t size);
 
-  ceph::BitVector<2u>::Reference operator[](uint64_t object_no);
   uint8_t operator[](uint64_t object_no) const;
   inline uint64_t size() const {
+    RWLock::RLocker locker(m_lock);
     return m_object_map.size();
   }
 
+  inline void set_state(uint64_t object_no, uint8_t new_state,
+                        const boost::optional<uint8_t> &current_state) {
+    RWLock::WLocker locker(m_lock);
+    ceph_assert(object_no < m_object_map.size());
+    if (current_state && m_object_map[object_no] != *current_state) {
+      return;
+    }
+    m_object_map[object_no] = new_state;
+  }
+
   void open(Context *on_finish);
   void close(Context *on_finish);
   bool set_object_map(ceph::BitVector<2> &target_object_map);
@@ -71,6 +81,8 @@ public:
                   const ZTracer::Trace &parent_trace, bool ignore_enoent,
                   T *callback_object) {
     ceph_assert(start_object_no < end_object_no);
+    RWLock::WLocker locker(m_lock);
+
     if (snap_id == CEPH_NOSNAP) {
       end_object_no = std::min(end_object_no, m_object_map.size());
       if (start_object_no >= end_object_no) {
@@ -132,9 +144,11 @@ private:
   typedef BlockGuard<UpdateOperation> UpdateGuard;
 
   ImageCtxT &m_image_ctx;
-  ceph::BitVector<2> m_object_map;
   uint64_t m_snap_id;
 
+  RWLock m_lock;
+  ceph::BitVector<2> m_object_map;
+
   UpdateGuard *m_update_guard = nullptr;
 
   void detained_aio_update(UpdateOperation &&update_operation);

diff --git a/src/librbd/object_map/RefreshRequest.cc b/src/librbd/object_map/RefreshRequest.cc
index 98984008cfd4faca2fed3fef19716c3bf38f4ad3..e061312e93845c1c84fd272af2fbbf3cfb820824 100644 (file)
--- a/src/librbd/object_map/RefreshRequest.cc
+++ b/src/librbd/object_map/RefreshRequest.cc
@@ -25,11 +25,12 @@ using util::create_rados_callback;
 namespace object_map {
 
 template <typename I>
-RefreshRequest<I>::RefreshRequest(I &image_ctx, ceph::BitVector<2> *object_map,
+RefreshRequest<I>::RefreshRequest(I &image_ctx, RWLock* object_map_lock,
+                                  ceph::BitVector<2> *object_map,
                                   uint64_t snap_id, Context *on_finish)
-  : m_image_ctx(image_ctx), m_object_map(object_map), m_snap_id(snap_id),
-    m_on_finish(on_finish), m_object_count(0),
-    m_truncate_on_disk_object_map(false) {
+  : m_image_ctx(image_ctx), m_object_map_lock(object_map_lock),
+     m_object_map(object_map), m_snap_id(snap_id), m_on_finish(on_finish),
+    m_object_count(0), m_truncate_on_disk_object_map(false) {
 }
 
 template <typename I>
@@ -57,6 +58,7 @@ void RefreshRequest<I>::apply() {
   }
   ceph_assert(m_on_disk_object_map.size() >= num_objs);
 
+  RWLock::WLocker object_map_locker(*m_object_map_lock);
   *m_object_map = m_on_disk_object_map;
 }
 
@@ -293,6 +295,7 @@ Context *RefreshRequest<I>::handle_invalidate_and_close(int *ret_val) {
     *ret_val = -EFBIG;
   }
 
+  RWLock::WLocker object_map_locker(*m_object_map_lock);
   m_object_map->clear();
   return m_on_finish;
 }

diff --git a/src/librbd/object_map/RefreshRequest.h b/src/librbd/object_map/RefreshRequest.h
index 24a7998ae1bc7e5c18e2c548620345a25cb61196..3e83dd3eee943218e71d786e6ea12298b0f403e0 100644 (file)
--- a/src/librbd/object_map/RefreshRequest.h
+++ b/src/librbd/object_map/RefreshRequest.h
@@ -9,6 +9,7 @@
 #include "common/bit_vector.hpp"
 
 class Context;
+class RWLock;
 
 namespace librbd {
 
@@ -19,14 +20,16 @@ namespace object_map {
 template <typename ImageCtxT = ImageCtx>
 class RefreshRequest {
 public:
-  static RefreshRequest *create(ImageCtxT &image_ctx,
+  static RefreshRequest *create(ImageCtxT &image_ctx, RWLock* object_map_lock,
                                 ceph::BitVector<2> *object_map,
                                 uint64_t snap_id, Context *on_finish) {
-    return new RefreshRequest(image_ctx, object_map, snap_id, on_finish);
+    return new RefreshRequest(image_ctx, object_map_lock, object_map, snap_id,
+                              on_finish);
   }
 
-  RefreshRequest(ImageCtxT &image_ctx, ceph::BitVector<2> *object_map,
-                 uint64_t snap_id, Context *on_finish);
+  RefreshRequest(ImageCtxT &image_ctx, RWLock* object_map_lock,
+                 ceph::BitVector<2> *object_map, uint64_t snap_id,
+                 Context *on_finish);
 
   void send();
 
@@ -58,6 +61,7 @@ private:
    */
 
   ImageCtxT &m_image_ctx;
+  RWLock* m_object_map_lock;
   ceph::BitVector<2> *m_object_map;
   uint64_t m_snap_id;
   Context *m_on_finish;

diff --git a/src/librbd/object_map/ResizeRequest.cc b/src/librbd/object_map/ResizeRequest.cc
index 1ec0d99afad2efd0257b260c682462fa5ebd2aca..8f0f1da51c09d2b18659f568c7d67df2cba0cffc 100644 (file)
--- a/src/librbd/object_map/ResizeRequest.cc
+++ b/src/librbd/object_map/ResizeRequest.cc
@@ -32,7 +32,7 @@ void ResizeRequest::resize(ceph::BitVector<2> *object_map, uint64_t num_objs,
 void ResizeRequest::send() {
   CephContext *cct = m_image_ctx.cct;
 
-  RWLock::WLocker l(m_image_ctx.object_map_lock);
+  RWLock::WLocker l(*m_object_map_lock);
   m_num_objs = Striper::get_num_objects(m_image_ctx.layout, m_new_size);
 
   std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, m_snap_id));
@@ -53,12 +53,11 @@ void ResizeRequest::send() {
 }
 
 void ResizeRequest::finish_request() {
-  RWLock::WLocker object_map_locker(m_image_ctx.object_map_lock);
-
   CephContext *cct = m_image_ctx.cct;
   ldout(cct, 5) << this << " resizing in-memory object map: "
                << m_num_objs << dendl;
 
+  RWLock::WLocker object_map_locker(*m_object_map_lock);
   resize(m_object_map, m_num_objs, m_default_object_state);
 }
 

diff --git a/src/librbd/object_map/ResizeRequest.h b/src/librbd/object_map/ResizeRequest.h
index 5183940075a000447b13f9bc28a7ac91ccd940dc..eda8f2f097eb020b1012cd269f329a0ed77ddf93 100644 (file)
--- a/src/librbd/object_map/ResizeRequest.h
+++ b/src/librbd/object_map/ResizeRequest.h
@@ -9,6 +9,7 @@
 #include "common/bit_vector.hpp"
 
 class Context;
+class RWLock;
 
 namespace librbd {
 
@@ -18,10 +19,12 @@ namespace object_map {
 
 class ResizeRequest : public Request {
 public:
-  ResizeRequest(ImageCtx &image_ctx, ceph::BitVector<2> *object_map,
-                uint64_t snap_id, uint64_t new_size,
-         uint8_t default_object_state, Context *on_finish)
-    : Request(image_ctx, snap_id, on_finish), m_object_map(object_map),
+  ResizeRequest(ImageCtx &image_ctx, RWLock *object_map_lock,
+                ceph::BitVector<2> *object_map, uint64_t snap_id,
+                uint64_t new_size, uint8_t default_object_state,
+                Context *on_finish)
+    : Request(image_ctx, snap_id, on_finish),
+      m_object_map_lock(object_map_lock), m_object_map(object_map),
       m_num_objs(0), m_new_size(new_size),
       m_default_object_state(default_object_state)
   {
@@ -36,6 +39,7 @@ protected:
   void finish_request() override;
 
 private:
+  RWLock* m_object_map_lock;
   ceph::BitVector<2> *m_object_map;
   uint64_t m_num_objs;
   uint64_t m_new_size;

diff --git a/src/librbd/object_map/SnapshotCreateRequest.cc b/src/librbd/object_map/SnapshotCreateRequest.cc
index 35950e32e9f3a4e431d262dd242141f4aec0cea0..2421adf9cbe029d57cf0d2cc7a96be174fab2c2f 100644 (file)
--- a/src/librbd/object_map/SnapshotCreateRequest.cc
+++ b/src/librbd/object_map/SnapshotCreateRequest.cc
@@ -132,8 +132,7 @@ bool SnapshotCreateRequest::send_add_snapshot() {
 }
 
 void SnapshotCreateRequest::update_object_map() {
-  RWLock::WLocker image_locker(m_image_ctx.image_lock);
-  RWLock::WLocker object_map_locker(m_image_ctx.object_map_lock);
+  RWLock::WLocker object_map_locker(*m_object_map_lock);
 
   auto it = m_object_map.begin();
   auto end_it = m_object_map.end();

diff --git a/src/librbd/object_map/SnapshotCreateRequest.h b/src/librbd/object_map/SnapshotCreateRequest.h
index dd15abefa931f04a2db835938d507efdd9a729aa..757833acf068f2c09fc4942d609ae76c0f899be1 100644 (file)
--- a/src/librbd/object_map/SnapshotCreateRequest.h
+++ b/src/librbd/object_map/SnapshotCreateRequest.h
@@ -9,6 +9,7 @@
 #include "librbd/object_map/Request.h"
 
 class Context;
+class RWLock;
 
 namespace librbd {
 
@@ -44,10 +45,12 @@ public:
     STATE_ADD_SNAPSHOT
   };
 
-  SnapshotCreateRequest(ImageCtx &image_ctx, ceph::BitVector<2> *object_map,
-                        uint64_t snap_id, Context *on_finish)
+  SnapshotCreateRequest(ImageCtx &image_ctx, RWLock* object_map_lock,
+                        ceph::BitVector<2> *object_map, uint64_t snap_id,
+                        Context *on_finish)
     : Request(image_ctx, snap_id, on_finish),
-      m_object_map(*object_map), m_ret_val(0) {
+      m_object_map_lock(object_map_lock), m_object_map(*object_map),
+      m_ret_val(0) {
   }
 
   void send() override;
@@ -56,9 +59,10 @@ protected:
   bool should_complete(int r) override;
 
 private:
-  State m_state = STATE_READ_MAP;
+  RWLock* m_object_map_lock;
   ceph::BitVector<2> &m_object_map;
 
+  State m_state = STATE_READ_MAP;
   bufferlist m_read_bl;
   int m_ret_val;
 

diff --git a/src/librbd/object_map/SnapshotRemoveRequest.cc b/src/librbd/object_map/SnapshotRemoveRequest.cc
index eef3ec37414121be55d7322677267058c8b2293e..42d3ca7ab683e77c92ff5f5dc5393354611b58c9 100644 (file)
--- a/src/librbd/object_map/SnapshotRemoveRequest.cc
+++ b/src/librbd/object_map/SnapshotRemoveRequest.cc
@@ -200,7 +200,7 @@ void SnapshotRemoveRequest::compute_next_snap_id() {
 
 void SnapshotRemoveRequest::update_object_map() {
   assert(m_image_ctx.image_lock.is_locked());
-  RWLock::WLocker object_map_locker(m_image_ctx.object_map_lock);
+  RWLock::WLocker object_map_locker(*m_object_map_lock);
   if (m_next_snap_id == m_image_ctx.snap_id && m_next_snap_id == CEPH_NOSNAP) {
     CephContext *cct = m_image_ctx.cct;
     ldout(cct, 5) << dendl;

diff --git a/src/librbd/object_map/SnapshotRemoveRequest.h b/src/librbd/object_map/SnapshotRemoveRequest.h
index bd020d1acbd00c064cda4a0c634db7a76d2c2abd..2327fea841810f4fb637cd462dc87c3f59534618 100644 (file)
--- a/src/librbd/object_map/SnapshotRemoveRequest.h
+++ b/src/librbd/object_map/SnapshotRemoveRequest.h
@@ -9,6 +9,8 @@
 #include "common/bit_vector.hpp"
 #include "librbd/AsyncRequest.h"
 
+class RWLock;
+
 namespace librbd {
 namespace object_map {
 
@@ -40,9 +42,11 @@ public:
    * otherwise, the state machine proceeds to remove the object map.
    */
 
-  SnapshotRemoveRequest(ImageCtx &image_ctx, ceph::BitVector<2> *object_map,
-                        uint64_t snap_id, Context *on_finish)
-    : AsyncRequest(image_ctx, on_finish), m_object_map(*object_map),
+  SnapshotRemoveRequest(ImageCtx &image_ctx, RWLock* object_map_lock,
+                        ceph::BitVector<2> *object_map, uint64_t snap_id,
+                        Context *on_finish)
+    : AsyncRequest(image_ctx, on_finish),
+      m_object_map_lock(object_map_lock), m_object_map(*object_map),
       m_snap_id(snap_id), m_next_snap_id(CEPH_NOSNAP) {
   }
 
@@ -54,6 +58,7 @@ protected:
   }
 
 private:
+  RWLock* m_object_map_lock;
   ceph::BitVector<2> &m_object_map;
   uint64_t m_snap_id;
   uint64_t m_next_snap_id;

diff --git a/src/librbd/object_map/UpdateRequest.cc b/src/librbd/object_map/UpdateRequest.cc
index cb9a2251cc8d9e329f4b6982ecde90a3f581c21b..0275034135c1bdca82195acbaa602af6d92bcc38 100644 (file)
--- a/src/librbd/object_map/UpdateRequest.cc
+++ b/src/librbd/object_map/UpdateRequest.cc
@@ -34,7 +34,7 @@ void UpdateRequest<I>::send() {
 template <typename I>
 void UpdateRequest<I>::update_object_map() {
   ceph_assert(m_image_ctx.image_lock.is_locked());
-  ceph_assert(m_image_ctx.object_map_lock.is_locked());
+  ceph_assert(m_object_map_lock->is_locked());
   CephContext *cct = m_image_ctx.cct;
 
   // break very large requests into manageable batches
@@ -81,7 +81,7 @@ void UpdateRequest<I>::handle_update_object_map(int r) {
 
   {
     RWLock::RLocker image_locker(m_image_ctx.image_lock);
-    RWLock::WLocker object_map_locker(m_image_ctx.object_map_lock);
+    RWLock::WLocker object_map_locker(*m_object_map_lock);
     update_in_memory_object_map();
 
     if (m_update_end_object_no < m_end_object_no) {
@@ -98,7 +98,7 @@ void UpdateRequest<I>::handle_update_object_map(int r) {
 template <typename I>
 void UpdateRequest<I>::update_in_memory_object_map() {
   ceph_assert(m_image_ctx.image_lock.is_locked());
-  ceph_assert(m_image_ctx.object_map_lock.is_locked());
+  ceph_assert(m_object_map_lock->is_locked());
 
   // rebuilding the object map might update on-disk only
   if (m_snap_id == m_image_ctx.snap_id) {

diff --git a/src/librbd/object_map/UpdateRequest.h b/src/librbd/object_map/UpdateRequest.h
index f5dcce15310278f5fef8e85587c46925a087e92e..ffaa883da547fa6225a8560cf42f4645101fd860 100644 (file)
--- a/src/librbd/object_map/UpdateRequest.h
+++ b/src/librbd/object_map/UpdateRequest.h
@@ -12,6 +12,7 @@
 #include <boost/optional.hpp>
 
 class Context;
+class RWLock;
 
 namespace librbd {
 
@@ -23,24 +24,28 @@ template <typename ImageCtxT = librbd::ImageCtx>
 class UpdateRequest : public Request {
 public:
   static UpdateRequest *create(ImageCtx &image_ctx,
+                               RWLock* object_map_lock,
                                ceph::BitVector<2> *object_map,
                                uint64_t snap_id, uint64_t start_object_no,
                                uint64_t end_object_no, uint8_t new_state,
                                const boost::optional<uint8_t> &current_state,
                                const ZTracer::Trace &parent_trace,
                                bool ignore_enoent, Context *on_finish) {
-    return new UpdateRequest(image_ctx, object_map, snap_id, start_object_no,
-                             end_object_no, new_state, current_state,
-                             parent_trace, ignore_enoent, on_finish);
+    return new UpdateRequest(image_ctx, object_map_lock, object_map, snap_id,
+                             start_object_no, end_object_no, new_state,
+                             current_state, parent_trace, ignore_enoent,
+                             on_finish);
   }
 
-  UpdateRequest(ImageCtx &image_ctx, ceph::BitVector<2> *object_map,
-                uint64_t snap_id, uint64_t start_object_no,
-                uint64_t end_object_no, uint8_t new_state,
+  UpdateRequest(ImageCtx &image_ctx, RWLock* object_map_lock,
+                ceph::BitVector<2> *object_map, uint64_t snap_id,
+                uint64_t start_object_no, uint64_t end_object_no,
+                uint8_t new_state,
                 const boost::optional<uint8_t> &current_state,
                const ZTracer::Trace &parent_trace, bool ignore_enoent,
                 Context *on_finish)
-    : Request(image_ctx, snap_id, on_finish), m_object_map(*object_map),
+    : Request(image_ctx, snap_id, on_finish),
+      m_object_map_lock(object_map_lock), m_object_map(*object_map),
       m_start_object_no(start_object_no), m_end_object_no(end_object_no),
       m_update_start_object_no(start_object_no), m_new_state(new_state),
       m_current_state(current_state),
@@ -74,6 +79,7 @@ private:
    * @endverbatim
    */
 
+  RWLock* m_object_map_lock;
   ceph::BitVector<2> &m_object_map;
   uint64_t m_start_object_no;
   uint64_t m_end_object_no;

diff --git a/src/librbd/operation/RebuildObjectMapRequest.cc b/src/librbd/operation/RebuildObjectMapRequest.cc
index 31497d67bbc01ab310aba8547b3cee08a94563ec..2bed8f9ec9fac377cc16512416858ae5aa28dc65 100644 (file)
--- a/src/librbd/operation/RebuildObjectMapRequest.cc
+++ b/src/librbd/operation/RebuildObjectMapRequest.cc
@@ -154,19 +154,19 @@ bool update_object_map(I& image_ctx, uint64_t object_no, uint8_t current_state,
   CephContext *cct = image_ctx.cct;
   uint64_t snap_id = image_ctx.snap_id;
 
-  uint8_t state = (*image_ctx.object_map)[object_no];
-  if (state == OBJECT_EXISTS && new_state == OBJECT_NONEXISTENT &&
+  current_state = (*image_ctx.object_map)[object_no];
+  if (current_state == OBJECT_EXISTS && new_state == OBJECT_NONEXISTENT &&
       snap_id == CEPH_NOSNAP) {
     // might be writing object to OSD concurrently
-    new_state = state;
+    new_state = current_state;
   }
 
-  if (new_state != state) {
+  if (new_state != current_state) {
     ldout(cct, 15) << image_ctx.get_object_name(object_no)
-      << " rebuild updating object map "
-      << static_cast<uint32_t>(state) << "->"
-      << static_cast<uint32_t>(new_state) << dendl;
-    (*image_ctx.object_map)[object_no] = new_state;
+                   << " rebuild updating object map "
+                   << static_cast<uint32_t>(current_state) << "->"
+                   << static_cast<uint32_t>(new_state) << dendl;
+    image_ctx.object_map->set_state(object_no, new_state, current_state);
   }
   return false;
 }

diff --git a/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc b/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc
index 17320a31100e5331af168fa0b12e4beeb75cb35a..883f4b468335f2e917a2c799bd2d62c4669385f5 100644 (file)
--- a/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc
+++ b/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc
@@ -292,7 +292,6 @@ public:
       } else {
         expect.WillOnce(DoAll(WithArg<7>(Invoke([&mock_image_ctx, snap_id, state](Context *ctx) {
                                   ceph_assert(mock_image_ctx.image_ctx->image_lock.is_locked());
-                                  ceph_assert(mock_image_ctx.image_ctx->object_map_lock.is_wlocked());
                                   mock_image_ctx.image_ctx->object_map->aio_update<Context>(
                                     snap_id, 0, 1, state, boost::none, {}, false, ctx);
                                 })),

diff --git a/src/test/librbd/object_map/test_mock_RefreshRequest.cc b/src/test/librbd/object_map/test_mock_RefreshRequest.cc
index 60cc579a8cf53d7c2d93642fb3df4f9b5a9cb135..68997a93a83954b96f86ccb6ee35817e9d835f8e 100644 (file)
--- a/src/test/librbd/object_map/test_mock_RefreshRequest.cc
+++ b/src/test/librbd/object_map/test_mock_RefreshRequest.cc
@@ -156,10 +156,11 @@ TEST_F(TestMockObjectMapRefreshRequest, SuccessHead) {
   init_object_map(mock_image_ctx, &on_disk_object_map);
 
   C_SaferCond ctx;
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
   MockLockRequest mock_lock_request;
-  MockRefreshRequest *req = new MockRefreshRequest(mock_image_ctx, &object_map,
-                                                   CEPH_NOSNAP, &ctx);
+  MockRefreshRequest *req = new MockRefreshRequest(
+    mock_image_ctx, &object_map_lock, &object_map, CEPH_NOSNAP, &ctx);
 
   InSequence seq;
   expect_get_image_size(mock_image_ctx, CEPH_NOSNAP,
@@ -186,9 +187,10 @@ TEST_F(TestMockObjectMapRefreshRequest, SuccessSnapshot) {
   init_object_map(mock_image_ctx, &on_disk_object_map);
 
   C_SaferCond ctx;
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
-  MockRefreshRequest *req = new MockRefreshRequest(mock_image_ctx, &object_map,
-                                                   TEST_SNAP_ID, &ctx);
+  MockRefreshRequest *req = new MockRefreshRequest(
+    mock_image_ctx, &object_map_lock, &object_map, TEST_SNAP_ID, &ctx);
 
   InSequence seq;
   expect_get_image_size(mock_image_ctx, TEST_SNAP_ID,
@@ -214,9 +216,10 @@ TEST_F(TestMockObjectMapRefreshRequest, LoadError) {
   init_object_map(mock_image_ctx, &on_disk_object_map);
 
   C_SaferCond ctx;
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
-  MockRefreshRequest *req = new MockRefreshRequest(mock_image_ctx, &object_map,
-                                                   TEST_SNAP_ID, &ctx);
+  MockRefreshRequest *req = new MockRefreshRequest(
+    mock_image_ctx, &object_map_lock, &object_map, TEST_SNAP_ID, &ctx);
 
   InSequence seq;
   expect_get_image_size(mock_image_ctx, TEST_SNAP_ID,
@@ -244,9 +247,10 @@ TEST_F(TestMockObjectMapRefreshRequest, LoadInvalidateError) {
   init_object_map(mock_image_ctx, &on_disk_object_map);
 
   C_SaferCond ctx;
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
-  MockRefreshRequest *req = new MockRefreshRequest(mock_image_ctx, &object_map,
-                                                   TEST_SNAP_ID, &ctx);
+  MockRefreshRequest *req = new MockRefreshRequest(
+    mock_image_ctx, &object_map_lock, &object_map, TEST_SNAP_ID, &ctx);
 
   InSequence seq;
   expect_get_image_size(mock_image_ctx, TEST_SNAP_ID,
@@ -274,9 +278,10 @@ TEST_F(TestMockObjectMapRefreshRequest, LoadCorrupt) {
   init_object_map(mock_image_ctx, &on_disk_object_map);
 
   C_SaferCond ctx;
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
-  MockRefreshRequest *req = new MockRefreshRequest(mock_image_ctx, &object_map,
-                                                   TEST_SNAP_ID, &ctx);
+  MockRefreshRequest *req = new MockRefreshRequest(
+    mock_image_ctx, &object_map_lock, &object_map, TEST_SNAP_ID, &ctx);
 
   InSequence seq;
   expect_get_image_size(mock_image_ctx, TEST_SNAP_ID,
@@ -308,9 +313,10 @@ TEST_F(TestMockObjectMapRefreshRequest, TooSmall) {
   ceph::BitVector<2> small_object_map;
 
   C_SaferCond ctx;
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
-  MockRefreshRequest *req = new MockRefreshRequest(mock_image_ctx, &object_map,
-                                                   TEST_SNAP_ID, &ctx);
+  MockRefreshRequest *req = new MockRefreshRequest(
+    mock_image_ctx, &object_map_lock, &object_map, TEST_SNAP_ID, &ctx);
 
   InSequence seq;
   expect_get_image_size(mock_image_ctx, TEST_SNAP_ID,
@@ -341,9 +347,10 @@ TEST_F(TestMockObjectMapRefreshRequest, TooSmallInvalidateError) {
   ceph::BitVector<2> small_object_map;
 
   C_SaferCond ctx;
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
-  MockRefreshRequest *req = new MockRefreshRequest(mock_image_ctx, &object_map,
-                                                   TEST_SNAP_ID, &ctx);
+  MockRefreshRequest *req = new MockRefreshRequest(
+    mock_image_ctx, &object_map_lock, &object_map, TEST_SNAP_ID, &ctx);
 
   InSequence seq;
   expect_get_image_size(mock_image_ctx, TEST_SNAP_ID,
@@ -374,9 +381,10 @@ TEST_F(TestMockObjectMapRefreshRequest, TooLarge) {
   large_object_map.resize(on_disk_object_map.size() * 2);
 
   C_SaferCond ctx;
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
-  MockRefreshRequest *req = new MockRefreshRequest(mock_image_ctx, &object_map,
-                                                   TEST_SNAP_ID, &ctx);
+  MockRefreshRequest *req = new MockRefreshRequest(
+    mock_image_ctx, &object_map_lock, &object_map, TEST_SNAP_ID, &ctx);
 
   InSequence seq;
   expect_get_image_size(mock_image_ctx, TEST_SNAP_ID,
@@ -402,9 +410,10 @@ TEST_F(TestMockObjectMapRefreshRequest, ResizeError) {
   ceph::BitVector<2> small_object_map;
 
   C_SaferCond ctx;
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
-  MockRefreshRequest *req = new MockRefreshRequest(mock_image_ctx, &object_map,
-                                                   TEST_SNAP_ID, &ctx);
+  MockRefreshRequest *req = new MockRefreshRequest(
+    mock_image_ctx, &object_map_lock, &object_map, TEST_SNAP_ID, &ctx);
 
   InSequence seq;
   expect_get_image_size(mock_image_ctx, TEST_SNAP_ID,
@@ -433,9 +442,10 @@ TEST_F(TestMockObjectMapRefreshRequest, LargeImageError) {
   init_object_map(mock_image_ctx, &on_disk_object_map);
 
   C_SaferCond ctx;
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
-  MockRefreshRequest *req = new MockRefreshRequest(mock_image_ctx, &object_map,
-                                                   TEST_SNAP_ID, &ctx);
+  MockRefreshRequest *req = new MockRefreshRequest(
+    mock_image_ctx, &object_map_lock, &object_map, TEST_SNAP_ID, &ctx);
 
   InSequence seq;
   expect_get_image_size(mock_image_ctx, TEST_SNAP_ID,

diff --git a/src/test/librbd/object_map/test_mock_ResizeRequest.cc b/src/test/librbd/object_map/test_mock_ResizeRequest.cc
index 3cfe34cfd49b7638ee856053493c7d1f1c3ccf9a..dfa4f7f81236c3248a55b2fcc463053f5169509c 100644 (file)
--- a/src/test/librbd/object_map/test_mock_ResizeRequest.cc
+++ b/src/test/librbd/object_map/test_mock_ResizeRequest.cc
@@ -55,13 +55,14 @@ TEST_F(TestMockObjectMapResizeRequest, UpdateInMemory) {
   ASSERT_EQ(0, open_image(m_image_name, &ictx));
   ASSERT_EQ(0, acquire_exclusive_lock(*ictx));
 
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
   object_map.resize(1);
 
   C_SaferCond cond_ctx;
   AsyncRequest<> *req = new ResizeRequest(
-    *ictx, &object_map, CEPH_NOSNAP, object_map.size(), OBJECT_EXISTS,
-    &cond_ctx);
+    *ictx, &object_map_lock, &object_map, CEPH_NOSNAP, object_map.size(),
+    OBJECT_EXISTS, &cond_ctx);
   req->send();
   ASSERT_EQ(0, cond_ctx.wait());
 
@@ -80,13 +81,14 @@ TEST_F(TestMockObjectMapResizeRequest, UpdateHeadOnDisk) {
 
   expect_resize(ictx, CEPH_NOSNAP, 0);
 
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
   object_map.resize(1);
 
   C_SaferCond cond_ctx;
   AsyncRequest<> *req = new ResizeRequest(
-    *ictx, &object_map, CEPH_NOSNAP, object_map.size(), OBJECT_EXISTS,
-    &cond_ctx);
+    *ictx, &object_map_lock, &object_map, CEPH_NOSNAP, object_map.size(),
+    OBJECT_EXISTS, &cond_ctx);
   req->send();
   ASSERT_EQ(0, cond_ctx.wait());
 
@@ -106,13 +108,14 @@ TEST_F(TestMockObjectMapResizeRequest, UpdateSnapOnDisk) {
   uint64_t snap_id = ictx->snap_id;
   expect_resize(ictx, snap_id, 0);
 
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
   object_map.resize(1);
 
   C_SaferCond cond_ctx;
   AsyncRequest<> *req = new ResizeRequest(
-    *ictx, &object_map, snap_id, object_map.size(), OBJECT_EXISTS,
-    &cond_ctx);
+    *ictx, &object_map_lock, &object_map, snap_id, object_map.size(),
+    OBJECT_EXISTS, &cond_ctx);
   req->send();
   ASSERT_EQ(0, cond_ctx.wait());
 
@@ -129,13 +132,14 @@ TEST_F(TestMockObjectMapResizeRequest, UpdateOnDiskError) {
   expect_resize(ictx, CEPH_NOSNAP, -EINVAL);
   expect_invalidate(ictx);
 
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
   object_map.resize(1);
 
   C_SaferCond cond_ctx;
   AsyncRequest<> *req = new ResizeRequest(
-    *ictx, &object_map, CEPH_NOSNAP, object_map.size(), OBJECT_EXISTS,
-    &cond_ctx);
+    *ictx, &object_map_lock, &object_map, CEPH_NOSNAP, object_map.size(),
+    OBJECT_EXISTS, &cond_ctx);
   req->send();
   ASSERT_EQ(0, cond_ctx.wait());
 

diff --git a/src/test/librbd/object_map/test_mock_SnapshotCreateRequest.cc b/src/test/librbd/object_map/test_mock_SnapshotCreateRequest.cc
index 90b5e750b1e6ac7d2ffd392858bf650d08390adf..dfccde317333556d2173434fb14e015acbe1731d 100644 (file)
--- a/src/test/librbd/object_map/test_mock_SnapshotCreateRequest.cc
+++ b/src/test/librbd/object_map/test_mock_SnapshotCreateRequest.cc
@@ -86,6 +86,7 @@ TEST_F(TestMockObjectMapSnapshotCreateRequest, Success) {
   ASSERT_EQ(0, open_image(m_image_name, &ictx));
   ASSERT_EQ(0, acquire_exclusive_lock(*ictx));
 
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
 
   uint64_t snap_id = 1;
@@ -98,7 +99,7 @@ TEST_F(TestMockObjectMapSnapshotCreateRequest, Success) {
 
   C_SaferCond cond_ctx;
   AsyncRequest<> *request = new SnapshotCreateRequest(
-    *ictx, &object_map, snap_id, &cond_ctx);
+    *ictx, &object_map_lock, &object_map, snap_id, &cond_ctx);
   {
     RWLock::RLocker image_locker(ictx->image_lock);
     request->send();
@@ -115,6 +116,7 @@ TEST_F(TestMockObjectMapSnapshotCreateRequest, ReadMapError) {
   ASSERT_EQ(0, open_image(m_image_name, &ictx));
   ASSERT_EQ(0, acquire_exclusive_lock(*ictx));
 
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
 
   uint64_t snap_id = 1;
@@ -124,7 +126,7 @@ TEST_F(TestMockObjectMapSnapshotCreateRequest, ReadMapError) {
 
   C_SaferCond cond_ctx;
   AsyncRequest<> *request = new SnapshotCreateRequest(
-    *ictx, &object_map, snap_id, &cond_ctx);
+    *ictx, &object_map_lock, &object_map, snap_id, &cond_ctx);
   {
     RWLock::RLocker image_locker(ictx->image_lock);
     request->send();
@@ -141,6 +143,7 @@ TEST_F(TestMockObjectMapSnapshotCreateRequest, WriteMapError) {
   ASSERT_EQ(0, open_image(m_image_name, &ictx));
   ASSERT_EQ(0, acquire_exclusive_lock(*ictx));
 
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
 
   uint64_t snap_id = 1;
@@ -151,7 +154,7 @@ TEST_F(TestMockObjectMapSnapshotCreateRequest, WriteMapError) {
 
   C_SaferCond cond_ctx;
   AsyncRequest<> *request = new SnapshotCreateRequest(
-    *ictx, &object_map, snap_id, &cond_ctx);
+    *ictx, &object_map_lock, &object_map, snap_id, &cond_ctx);
   {
     RWLock::RLocker image_locker(ictx->image_lock);
     request->send();
@@ -168,6 +171,7 @@ TEST_F(TestMockObjectMapSnapshotCreateRequest, AddSnapshotError) {
   ASSERT_EQ(0, open_image(m_image_name, &ictx));
   ASSERT_EQ(0, acquire_exclusive_lock(*ictx));
 
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
 
   uint64_t snap_id = 1;
@@ -179,7 +183,7 @@ TEST_F(TestMockObjectMapSnapshotCreateRequest, AddSnapshotError) {
 
   C_SaferCond cond_ctx;
   AsyncRequest<> *request = new SnapshotCreateRequest(
-    *ictx, &object_map, snap_id, &cond_ctx);
+    *ictx, &object_map_lock, &object_map, snap_id, &cond_ctx);
   {
     RWLock::RLocker image_locker(ictx->image_lock);
     request->send();
@@ -196,6 +200,7 @@ TEST_F(TestMockObjectMapSnapshotCreateRequest, FlagCleanObjects) {
   ASSERT_EQ(0, open_image(m_image_name, &ictx));
   ASSERT_EQ(0, acquire_exclusive_lock(*ictx));
 
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
   object_map.resize(1024);
   for (uint64_t i = 0; i < object_map.size(); ++i) {
@@ -207,7 +212,7 @@ TEST_F(TestMockObjectMapSnapshotCreateRequest, FlagCleanObjects) {
 
   C_SaferCond cond_ctx;
   AsyncRequest<> *request = new SnapshotCreateRequest(
-    *ictx, &object_map, snap_id, &cond_ctx);
+    *ictx, &object_map_lock, &object_map, snap_id, &cond_ctx);
   {
     RWLock::RLocker image_locker(ictx->image_lock);
     request->send();

diff --git a/src/test/librbd/object_map/test_mock_SnapshotRemoveRequest.cc b/src/test/librbd/object_map/test_mock_SnapshotRemoveRequest.cc
index ee85500dad6d207b6ebbc6d5b666d317caedd17e..9e4233dfb3c69452f2b43d551d3f49cd5f8811dd 100644 (file)
--- a/src/test/librbd/object_map/test_mock_SnapshotRemoveRequest.cc
+++ b/src/test/librbd/object_map/test_mock_SnapshotRemoveRequest.cc
@@ -84,10 +84,11 @@ TEST_F(TestMockObjectMapSnapshotRemoveRequest, Success) {
   }
   expect_remove_map(ictx, snap_id, 0);
 
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
   C_SaferCond cond_ctx;
   AsyncRequest<> *request = new SnapshotRemoveRequest(
-    *ictx, &object_map, snap_id, &cond_ctx);
+    *ictx, &object_map_lock, &object_map, snap_id, &cond_ctx);
   {
     RWLock::RLocker owner_locker(ictx->owner_lock);
     RWLock::WLocker image_locker(ictx->image_lock);
@@ -113,10 +114,11 @@ TEST_F(TestMockObjectMapSnapshotRemoveRequest, LoadMapMissing) {
 
   expect_load_map(ictx, snap_id, -ENOENT);
 
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
   C_SaferCond cond_ctx;
   AsyncRequest<> *request = new SnapshotRemoveRequest(
-    *ictx, &object_map, snap_id, &cond_ctx);
+    *ictx, &object_map_lock, &object_map, snap_id, &cond_ctx);
   {
     RWLock::RLocker owner_locker(ictx->owner_lock);
     RWLock::WLocker image_locker(ictx->image_lock);
@@ -149,10 +151,11 @@ TEST_F(TestMockObjectMapSnapshotRemoveRequest, LoadMapError) {
   expect_invalidate(ictx);
   expect_remove_map(ictx, snap_id, 0);
 
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
   C_SaferCond cond_ctx;
   AsyncRequest<> *request = new SnapshotRemoveRequest(
-    *ictx, &object_map, snap_id, &cond_ctx);
+    *ictx, &object_map_lock, &object_map, snap_id, &cond_ctx);
   {
     RWLock::RLocker owner_locker(ictx->owner_lock);
     RWLock::WLocker image_locker(ictx->image_lock);
@@ -176,10 +179,11 @@ TEST_F(TestMockObjectMapSnapshotRemoveRequest, RemoveSnapshotMissing) {
   expect_remove_snapshot(ictx, -ENOENT);
   expect_remove_map(ictx, snap_id, 0);
 
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
   C_SaferCond cond_ctx;
   AsyncRequest<> *request = new SnapshotRemoveRequest(
-    *ictx, &object_map, snap_id, &cond_ctx);
+    *ictx, &object_map_lock, &object_map, snap_id, &cond_ctx);
   {
     RWLock::RLocker owner_locker(ictx->owner_lock);
     RWLock::WLocker image_locker(ictx->image_lock);
@@ -204,10 +208,11 @@ TEST_F(TestMockObjectMapSnapshotRemoveRequest, RemoveSnapshotError) {
   expect_invalidate(ictx);
   expect_remove_map(ictx, snap_id, 0);
 
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
   C_SaferCond cond_ctx;
   AsyncRequest<> *request = new SnapshotRemoveRequest(
-    *ictx, &object_map, snap_id, &cond_ctx);
+    *ictx, &object_map_lock, &object_map, snap_id, &cond_ctx);
   {
     RWLock::RLocker owner_locker(ictx->owner_lock);
     RWLock::WLocker image_locker(ictx->image_lock);
@@ -233,10 +238,11 @@ TEST_F(TestMockObjectMapSnapshotRemoveRequest, RemoveMapMissing) {
   }
   expect_remove_map(ictx, snap_id, -ENOENT);
 
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
   C_SaferCond cond_ctx;
   AsyncRequest<> *request = new SnapshotRemoveRequest(
-    *ictx, &object_map, snap_id, &cond_ctx);
+    *ictx, &object_map_lock, &object_map, snap_id, &cond_ctx);
   {
     RWLock::RLocker owner_locker(ictx->owner_lock);
     RWLock::WLocker image_locker(ictx->image_lock);
@@ -262,10 +268,11 @@ TEST_F(TestMockObjectMapSnapshotRemoveRequest, RemoveMapError) {
   }
   expect_remove_map(ictx, snap_id, -EINVAL);
 
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
   C_SaferCond cond_ctx;
   AsyncRequest<> *request = new SnapshotRemoveRequest(
-    *ictx, &object_map, snap_id, &cond_ctx);
+    *ictx, &object_map_lock, &object_map, snap_id, &cond_ctx);
   {
     RWLock::RLocker owner_locker(ictx->owner_lock);
     RWLock::WLocker image_locker(ictx->image_lock);
@@ -284,14 +291,15 @@ TEST_F(TestMockObjectMapSnapshotRemoveRequest, ScrubCleanObjects) {
   librbd::NoOpProgressContext prog_ctx;
   uint64_t size = 4294967296; // 4GB = 1024 * 4MB
   ASSERT_EQ(0, resize(ictx, size));
-  
-  // update image objectmap for snap inherit 
+
+  // update image objectmap for snap inherit
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
   object_map.resize(1024);
   for (uint64_t i = 512; i < object_map.size(); ++i) {
     object_map[i] = i % 2 == 0 ? OBJECT_EXISTS : OBJECT_NONEXISTENT;
   }
-  
+
   C_SaferCond cond_ctx1;
   {
     librbd::ObjectMap om(*ictx, ictx->snap_id);
@@ -312,7 +320,7 @@ TEST_F(TestMockObjectMapSnapshotRemoveRequest, ScrubCleanObjects) {
   C_SaferCond cond_ctx2;
   uint64_t snap_id = ictx->snap_info.rbegin()->first;
   AsyncRequest<> *request = new SnapshotRemoveRequest(
-    *ictx, &object_map, snap_id, &cond_ctx2);
+    *ictx, &object_map_lock, &object_map, snap_id, &cond_ctx2);
   {
     RWLock::RLocker owner_locker(ictx->owner_lock);
     RWLock::WLocker image_locker(ictx->image_lock);

diff --git a/src/test/librbd/object_map/test_mock_UpdateRequest.cc b/src/test/librbd/object_map/test_mock_UpdateRequest.cc
index c7221567ccf6e5dc95083e3ff4e51d152720ad32..60628c0b9f9170c06e8a7e1012c194274d88ecdd 100644 (file)
--- a/src/test/librbd/object_map/test_mock_UpdateRequest.cc
+++ b/src/test/librbd/object_map/test_mock_UpdateRequest.cc
@@ -72,6 +72,7 @@ TEST_F(TestMockObjectMapUpdateRequest, UpdateInMemory) {
   ASSERT_EQ(0, ictx->operations->resize(4 << ictx->order, true, no_progress));
   ASSERT_EQ(0, acquire_exclusive_lock(*ictx));
 
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
   object_map.resize(4);
   for (uint64_t i = 0; i < object_map.size(); ++i) {
@@ -80,11 +81,11 @@ TEST_F(TestMockObjectMapUpdateRequest, UpdateInMemory) {
 
   C_SaferCond cond_ctx;
   AsyncRequest<> *req = new UpdateRequest<>(
-    *ictx, &object_map, CEPH_NOSNAP, 0, object_map.size(), OBJECT_NONEXISTENT,
-    OBJECT_EXISTS, {}, false, &cond_ctx);
+    *ictx, &object_map_lock, &object_map, CEPH_NOSNAP, 0, object_map.size(),
+    OBJECT_NONEXISTENT, OBJECT_EXISTS, {}, false, &cond_ctx);
   {
     RWLock::RLocker image_locker(ictx->image_lock);
-    RWLock::WLocker object_map_locker(ictx->object_map_lock);
+    RWLock::WLocker object_map_locker(object_map_lock);
     req->send();
   }
   ASSERT_EQ(0, cond_ctx.wait());
@@ -107,16 +108,17 @@ TEST_F(TestMockObjectMapUpdateRequest, UpdateHeadOnDisk) {
 
   expect_update(ictx, CEPH_NOSNAP, 0, 1, OBJECT_NONEXISTENT, OBJECT_EXISTS, 0);
 
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
   object_map.resize(1);
 
   C_SaferCond cond_ctx;
   AsyncRequest<> *req = new UpdateRequest<>(
-    *ictx, &object_map, CEPH_NOSNAP, 0, object_map.size(), OBJECT_NONEXISTENT,
-    OBJECT_EXISTS, {}, false, &cond_ctx);
+    *ictx, &object_map_lock, &object_map, CEPH_NOSNAP, 0, object_map.size(),
+    OBJECT_NONEXISTENT, OBJECT_EXISTS, {}, false, &cond_ctx);
   {
     RWLock::RLocker image_locker(ictx->image_lock);
-    RWLock::WLocker object_map_locker(ictx->object_map_lock);
+    RWLock::WLocker object_map_locker(object_map_lock);
     req->send();
   }
   ASSERT_EQ(0, cond_ctx.wait());
@@ -137,16 +139,17 @@ TEST_F(TestMockObjectMapUpdateRequest, UpdateSnapOnDisk) {
   uint64_t snap_id = ictx->snap_id;
   expect_update(ictx, snap_id, 0, 1, OBJECT_NONEXISTENT, OBJECT_EXISTS, 0);
 
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
   object_map.resize(1);
 
   C_SaferCond cond_ctx;
   AsyncRequest<> *req = new UpdateRequest<>(
-    *ictx, &object_map, snap_id, 0, object_map.size(), OBJECT_NONEXISTENT,
-    OBJECT_EXISTS, {}, false, &cond_ctx);
+    *ictx, &object_map_lock, &object_map, snap_id, 0, object_map.size(),
+    OBJECT_NONEXISTENT, OBJECT_EXISTS, {}, false, &cond_ctx);
   {
     RWLock::RLocker image_locker(ictx->image_lock);
-    RWLock::WLocker object_map_locker(ictx->object_map_lock);
+    RWLock::WLocker object_map_locker(object_map_lock);
     req->send();
   }
   ASSERT_EQ(0, cond_ctx.wait());
@@ -165,16 +168,17 @@ TEST_F(TestMockObjectMapUpdateRequest, UpdateOnDiskError) {
                 -EINVAL);
   expect_invalidate(ictx);
 
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
   object_map.resize(1);
 
   C_SaferCond cond_ctx;
   AsyncRequest<> *req = new UpdateRequest<>(
-    *ictx, &object_map, CEPH_NOSNAP, 0, object_map.size(), OBJECT_NONEXISTENT,
-    OBJECT_EXISTS, {}, false, &cond_ctx);
+    *ictx, &object_map_lock, &object_map, CEPH_NOSNAP, 0, object_map.size(),
+    OBJECT_NONEXISTENT, OBJECT_EXISTS, {}, false, &cond_ctx);
   {
     RWLock::RLocker image_locker(ictx->image_lock);
-    RWLock::WLocker object_map_locker(ictx->object_map_lock);
+    RWLock::WLocker object_map_locker(object_map_lock);
     req->send();
   }
   ASSERT_EQ(0, cond_ctx.wait());
@@ -196,16 +200,17 @@ TEST_F(TestMockObjectMapUpdateRequest, RebuildSnapOnDisk) {
                 boost::optional<uint8_t>(), 0);
   expect_unlock_exclusive_lock(*ictx);
 
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
   object_map.resize(1);
 
   C_SaferCond cond_ctx;
   AsyncRequest<> *req = new UpdateRequest<>(
-    *ictx, &object_map, snap_id, 0, object_map.size(), OBJECT_EXISTS_CLEAN,
-    boost::optional<uint8_t>(), {}, false, &cond_ctx);
+    *ictx, &object_map_lock, &object_map, snap_id, 0, object_map.size(),
+    OBJECT_EXISTS_CLEAN, boost::optional<uint8_t>(), {}, false, &cond_ctx);
   {
     RWLock::RLocker image_locker(ictx->image_lock);
-    RWLock::WLocker object_map_locker(ictx->object_map_lock);
+    RWLock::WLocker object_map_locker(object_map_lock);
     req->send();
   }
   ASSERT_EQ(0, cond_ctx.wait());
@@ -234,16 +239,17 @@ TEST_F(TestMockObjectMapUpdateRequest, BatchUpdate) {
                 OBJECT_EXISTS, 0);
   expect_unlock_exclusive_lock(*ictx);
 
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
   object_map.resize(712312);
 
   C_SaferCond cond_ctx;
   AsyncRequest<> *req = new UpdateRequest<>(
-    *ictx, &object_map, CEPH_NOSNAP, 0, object_map.size(), OBJECT_NONEXISTENT,
-    OBJECT_EXISTS, {}, false, &cond_ctx);
+    *ictx, &object_map_lock, &object_map, CEPH_NOSNAP, 0, object_map.size(),
+    OBJECT_NONEXISTENT, OBJECT_EXISTS, {}, false, &cond_ctx);
   {
     RWLock::RLocker image_locker(ictx->image_lock);
-    RWLock::WLocker object_map_locker(ictx->object_map_lock);
+    RWLock::WLocker object_map_locker(object_map_lock);
     req->send();
   }
   ASSERT_EQ(0, cond_ctx.wait());
@@ -259,16 +265,17 @@ TEST_F(TestMockObjectMapUpdateRequest, IgnoreMissingObjectMap) {
   expect_update(ictx, CEPH_NOSNAP, 0, 1, OBJECT_NONEXISTENT, OBJECT_EXISTS,
                 -ENOENT);
 
+  RWLock object_map_lock("lock");
   ceph::BitVector<2> object_map;
   object_map.resize(1);
 
   C_SaferCond cond_ctx;
   AsyncRequest<> *req = new UpdateRequest<>(
-    *ictx, &object_map, CEPH_NOSNAP, 0, object_map.size(), OBJECT_NONEXISTENT,
-    OBJECT_EXISTS, {}, true, &cond_ctx);
+    *ictx, &object_map_lock, &object_map, CEPH_NOSNAP, 0, object_map.size(),
+    OBJECT_NONEXISTENT, OBJECT_EXISTS, {}, true, &cond_ctx);
   {
     RWLock::RLocker image_locker(ictx->image_lock);
-    RWLock::WLocker object_map_locker(ictx->object_map_lock);
+    RWLock::WLocker object_map_locker(object_map_lock);
     req->send();
   }
   ASSERT_EQ(0, cond_ctx.wait());

diff --git a/src/test/librbd/test_ObjectMap.cc b/src/test/librbd/test_ObjectMap.cc
index 3003d68b335f171992a2ee71b61c9bdee646f660..fe30920659275704748d645fe05e794a5a2737b1 100644 (file)
--- a/src/test/librbd/test_ObjectMap.cc
+++ b/src/test/librbd/test_ObjectMap.cc
@@ -200,7 +200,6 @@ TEST_F(TestObjectMap, DISABLED_StressTest) {
 
     RWLock::RLocker owner_locker(ictx->owner_lock);
     RWLock::RLocker image_locker(ictx->image_lock);
-    RWLock::WLocker object_map_locker(ictx->object_map_lock);
     ASSERT_TRUE(ictx->object_map != nullptr);
 
     if (!ictx->object_map->aio_update<

diff --git a/src/test/librbd/test_internal.cc b/src/test/librbd/test_internal.cc
index 3ca8297abebb9d6550c56091a2457b0f8c034d31..11db69f2e1ee76e83d01f96c7e51ba15f93734dd 100644 (file)
--- a/src/test/librbd/test_internal.cc
+++ b/src/test/librbd/test_internal.cc
@@ -659,8 +659,6 @@ TEST_F(TestInternal, SnapshotCopyup)
       C_SaferCond ctx;
       object_map.open(&ctx);
       ASSERT_EQ(0, ctx.wait());
-
-      RWLock::WLocker object_map_locker(ictx2->object_map_lock);
       ASSERT_EQ(state, object_map[0]);
     }
   }

diff --git a/src/test/librbd/test_mock_ObjectMap.cc b/src/test/librbd/test_mock_ObjectMap.cc
index 98c91173492162fec9d4fc90690539b0811976f7..88331a8257602a656397ac8a1f39c4dcf24c2ab8 100644 (file)
--- a/src/test/librbd/test_mock_ObjectMap.cc
+++ b/src/test/librbd/test_mock_ObjectMap.cc
@@ -27,7 +27,7 @@ struct RefreshRequest<MockTestImageCtx> {
   Context *on_finish = nullptr;
   ceph::BitVector<2u> *object_map = nullptr;
   static RefreshRequest *s_instance;
-  static RefreshRequest *create(MockTestImageCtx &image_ctx,
+  static RefreshRequest *create(MockTestImageCtx &image_ctx, RWLock*,
                                 ceph::BitVector<2u> *object_map,
                                 uint64_t snap_id, Context *on_finish) {
     ceph_assert(s_instance != nullptr);
@@ -64,7 +64,7 @@ template <>
 struct UpdateRequest<MockTestImageCtx> {
   Context *on_finish = nullptr;
   static UpdateRequest *s_instance;
-  static UpdateRequest *create(MockTestImageCtx &image_ctx,
+  static UpdateRequest *create(MockTestImageCtx &image_ctx, RWLock*,
                                ceph::BitVector<2u> *object_map,
                                uint64_t snap_id,
                                uint64_t start_object_no, uint64_t end_object_no,
@@ -181,7 +181,6 @@ TEST_F(TestMockObjectMap, NonDetainedUpdate) {
   C_SaferCond update_ctx2;
   {
     RWLock::RLocker image_locker(mock_image_ctx.image_lock);
-    RWLock::WLocker object_map_locker(mock_image_ctx.object_map_lock);
     mock_object_map.aio_update(CEPH_NOSNAP, 0, 1, {}, {}, false, &update_ctx1);
     mock_object_map.aio_update(CEPH_NOSNAP, 1, 1, {}, {}, false, &update_ctx2);
   }
@@ -239,7 +238,6 @@ TEST_F(TestMockObjectMap, DetainedUpdate) {
   C_SaferCond update_ctx4;
   {
     RWLock::RLocker image_locker(mock_image_ctx.image_lock);
-    RWLock::WLocker object_map_locker(mock_image_ctx.object_map_lock);
     mock_object_map.aio_update(CEPH_NOSNAP, 1, 4, 1, {}, {}, false,
                                &update_ctx1);
     mock_object_map.aio_update(CEPH_NOSNAP, 1, 3, 1, {}, {}, false,