librbd: optimize away unnecessary object map updates

Fixes: http://tracker.ceph.com/issues/16689
Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/librbd/AioObjectRequest.cc b/src/librbd/AioObjectRequest.cc
index 152a55fa55febf417005b6284e758efbc6012d2b..dd55c8e6503d10ef4594b93cbbca19458dd852c2 100644 (file)
--- a/src/librbd/AioObjectRequest.cc
+++ b/src/librbd/AioObjectRequest.cc
@@ -402,22 +402,21 @@ namespace librbd {
       } else {
         // should have been flushed prior to releasing lock
         assert(m_ictx->exclusive_lock->is_lock_owner());
-
         m_object_exist = m_ictx->object_map->object_may_exist(m_object_no);
 
-        ldout(m_ictx->cct, 20) << "send_pre " << this << " " << m_oid << " "
-                              << m_object_off << "~" << m_object_len << dendl;
-        m_state = LIBRBD_AIO_WRITE_PRE;
-
         uint8_t new_state;
-        boost::optional<uint8_t> current_state;
         pre_object_map_update(&new_state);
 
         RWLock::WLocker object_map_locker(m_ictx->object_map_lock);
-        if ((*m_ictx->object_map)[m_object_no] != new_state) {
+        if (m_ictx->object_map->update_required(m_object_no, new_state)) {
+          ldout(m_ictx->cct, 20) << "send_pre " << this << " " << m_oid << " "
+                                 << m_object_off << "~" << m_object_len
+                                 << dendl;
+          m_state = LIBRBD_AIO_WRITE_PRE;
+
           Context *ctx = util::create_context_callback<AioObjectRequest>(this);
           bool updated = m_ictx->object_map->aio_update(m_object_no, new_state,
-                                                        current_state, ctx);
+                                                        {}, ctx);
           assert(updated);
         } else {
           write = true;
@@ -442,17 +441,15 @@ namespace librbd {
     // should have been flushed prior to releasing lock
     assert(m_ictx->exclusive_lock->is_lock_owner());
 
-    ldout(m_ictx->cct, 20) << "send_post " << this << " " << m_oid << " "
-                          << m_object_off << "~" << m_object_len << dendl;
-    m_state = LIBRBD_AIO_WRITE_POST;
-
     RWLock::WLocker object_map_locker(m_ictx->object_map_lock);
-    uint8_t current_state = (*m_ictx->object_map)[m_object_no];
-    if (current_state != OBJECT_PENDING ||
-        current_state == OBJECT_NONEXISTENT) {
+    if (!m_ictx->object_map->update_required(m_object_no, OBJECT_NONEXISTENT)) {
       return true;
     }
 
+    ldout(m_ictx->cct, 20) << "send_post " << this << " " << m_oid << " "
+                          << m_object_off << "~" << m_object_len << dendl;
+    m_state = LIBRBD_AIO_WRITE_POST;
+
     Context *ctx = util::create_context_callback<AioObjectRequest>(this);
     bool updated = m_ictx->object_map->aio_update(m_object_no,
                                                   OBJECT_NONEXISTENT,

diff --git a/src/librbd/ObjectMap.cc b/src/librbd/ObjectMap.cc
index 8b567861bcf3c801f37d6f4c95e3f82b127e3cbd..b5d659ef8a2e8aa9ba2880171dcf0a3f07fc13d7 100644 (file)
--- a/src/librbd/ObjectMap.cc
+++ b/src/librbd/ObjectMap.cc
@@ -90,6 +90,18 @@ bool ObjectMap::object_may_exist(uint64_t object_no) const
   return exists;
 }
 
+bool ObjectMap::update_required(uint64_t object_no, uint8_t new_state) {
+  assert(m_image_ctx.object_map_lock.is_wlocked());
+  uint8_t state = (*this)[object_no];
+
+  if ((state == new_state) ||
+      (new_state == OBJECT_PENDING && state == OBJECT_NONEXISTENT) ||
+      (new_state == OBJECT_NONEXISTENT && state != OBJECT_PENDING)) {
+    return false;
+  }
+  return true;
+}
+
 void ObjectMap::open(Context *on_finish) {
   object_map::RefreshRequest<> *req = new object_map::RefreshRequest<>(
     m_image_ctx, &m_object_map, m_snap_id, on_finish);

diff --git a/src/librbd/ObjectMap.h b/src/librbd/ObjectMap.h
index a13166066ff1633b17a05716e23e99394e22c717..5d99180e771a3e930baa02a2c5075af2c3049b5e 100644 (file)
--- a/src/librbd/ObjectMap.h
+++ b/src/librbd/ObjectMap.h
@@ -39,6 +39,7 @@ public:
   void close(Context *on_finish);
 
   bool object_may_exist(uint64_t object_no) const;
+  bool update_required(uint64_t object_no, uint8_t new_state);
 
   void aio_save(Context *on_finish);
   void aio_resize(uint64_t new_size, uint8_t default_object_state,