librbd: optimize away unnecessary object map updates

Fixes: http://tracker.ceph.com/issues/16689 Signed-off-by: Jason Dillaman <dillaman@redhat.com>
ceph · Jul 19, 2016 · e5b4188 · e5b4188
1 parent b7a4db2
commit e5b4188
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 15 deletions.
diff --git 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
@@ -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
@@ -39,6 +39,7 @@ class ObjectMap {
   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,