From a8882932e7be741cf7de807a5a67154c94abf42e Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Tue, 23 Apr 2019 09:48:52 -0400 Subject: [PATCH] librbd: protect object map with a private member lock Signed-off-by: Jason Dillaman --- src/librbd/ObjectMap.cc | 47 +++++++---------- src/librbd/ObjectMap.h | 20 ++++++-- src/librbd/object_map/RefreshRequest.cc | 11 ++-- src/librbd/object_map/RefreshRequest.h | 12 +++-- src/librbd/object_map/ResizeRequest.cc | 5 +- src/librbd/object_map/ResizeRequest.h | 12 +++-- .../object_map/SnapshotCreateRequest.cc | 3 +- src/librbd/object_map/SnapshotCreateRequest.h | 12 +++-- .../object_map/SnapshotRemoveRequest.cc | 2 +- src/librbd/object_map/SnapshotRemoveRequest.h | 11 ++-- src/librbd/object_map/UpdateRequest.cc | 6 +-- src/librbd/object_map/UpdateRequest.h | 20 +++++--- .../operation/RebuildObjectMapRequest.cc | 16 +++--- .../deep_copy/test_mock_ObjectCopyRequest.cc | 1 - .../object_map/test_mock_RefreshRequest.cc | 50 +++++++++++-------- .../object_map/test_mock_ResizeRequest.cc | 20 +++++--- .../test_mock_SnapshotCreateRequest.cc | 15 ++++-- .../test_mock_SnapshotRemoveRequest.cc | 30 +++++++---- .../object_map/test_mock_UpdateRequest.cc | 49 ++++++++++-------- src/test/librbd/test_ObjectMap.cc | 1 - src/test/librbd/test_internal.cc | 2 - src/test/librbd/test_mock_ObjectMap.cc | 6 +-- 22 files changed, 204 insertions(+), 147 deletions(-) diff --git a/src/librbd/ObjectMap.cc b/src/librbd/ObjectMap.cc index 4eedf80bba1..ad86a8c7b16 100644 --- a/src/librbd/ObjectMap.cc +++ b/src/librbd/ObjectMap.cc @@ -35,6 +35,7 @@ namespace librbd { template ObjectMap::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::is_compatible(const file_layout_t& layout, uint64_t size) { return (object_count <= cls::rbd::MAX_OBJECT_MAP_OBJECT_COUNT); } -template -ceph::BitVector<2u>::Reference ObjectMap::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 uint8_t ObjectMap::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::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::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::object_may_not_exist(uint64_t object_no) const template bool ObjectMap::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::update_required(const ceph::BitVector<2>::Iterator& it, template void ObjectMap::open(Context *on_finish) { auto req = object_map::RefreshRequest::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::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::set_object_map(ceph::BitVector<2> &target_object_map) { template void ObjectMap::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::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::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::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::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::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::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::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::aio_update(uint64_t snap_id, uint64_t start_object_no, stringify(static_cast(*current_state)) : "") << "->" << static_cast(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::aio_update(uint64_t snap_id, uint64_t start_object_no, } auto req = object_map::UpdateRequest::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 c930a5b554a..3f7cdd04e01 100644 --- 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 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 ¤t_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 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 98984008cfd..e061312e938 100644 --- 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 -RefreshRequest::RefreshRequest(I &image_ctx, ceph::BitVector<2> *object_map, +RefreshRequest::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 @@ -57,6 +58,7 @@ void RefreshRequest::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::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 24a7998ae1b..3e83dd3eee9 100644 --- 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 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 1ec0d99afad..8f0f1da51c0 100644 --- 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 5183940075a..eda8f2f097e 100644 --- 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 35950e32e9f..2421adf9cbe 100644 --- 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 dd15abefa93..757833acf06 100644 --- 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 eef3ec37414..42d3ca7ab68 100644 --- 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 bd020d1acbd..2327fea8418 100644 --- 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 cb9a2251cc8..0275034135c 100644 --- a/src/librbd/object_map/UpdateRequest.cc +++ b/src/librbd/object_map/UpdateRequest.cc @@ -34,7 +34,7 @@ void UpdateRequest::send() { template void UpdateRequest::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::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::handle_update_object_map(int r) { template void UpdateRequest::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 f5dcce15310..ffaa883da54 100644 --- a/src/librbd/object_map/UpdateRequest.h +++ b/src/librbd/object_map/UpdateRequest.h @@ -12,6 +12,7 @@ #include class Context; +class RWLock; namespace librbd { @@ -23,24 +24,28 @@ template 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 ¤t_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 ¤t_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 31497d67bbc..2bed8f9ec9f 100644 --- 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(state) << "->" - << static_cast(new_state) << dendl; - (*image_ctx.object_map)[object_no] = new_state; + << " rebuild updating object map " + << static_cast(current_state) << "->" + << static_cast(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 17320a31100..883f4b46833 100644 --- 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( 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 60cc579a8cf..68997a93a83 100644 --- 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 3cfe34cfd49..dfa4f7f8123 100644 --- 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 90b5e750b1e..dfccde31733 100644 --- 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 ee85500dad6..9e4233dfb3c 100644 --- 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 c7221567ccf..60628c0b9f9 100644 --- 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(), 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(), {}, false, &cond_ctx); + *ictx, &object_map_lock, &object_map, snap_id, 0, object_map.size(), + OBJECT_EXISTS_CLEAN, boost::optional(), {}, 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 3003d68b335..fe309206592 100644 --- 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 3ca8297abeb..11db69f2e1e 100644 --- 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 98c91173492..88331a82576 100644 --- a/src/test/librbd/test_mock_ObjectMap.cc +++ b/src/test/librbd/test_mock_ObjectMap.cc @@ -27,7 +27,7 @@ struct RefreshRequest { 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 { 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, -- 2.39.5