From ef9d74720f2e8ba3146774e75f60fed815a3c9de Mon Sep 17 00:00:00 2001
From: Jason Dillaman <dillaman@redhat.com>
Date: Tue, 6 Mar 2018 14:23:47 -0500
Subject: [PATCH] librbd: switch to lock-free queue for event poll IO interface

'perf' shows several percent of CPU being wasted on lock contention
in the event poll interface. The 'fio' RBD engine uses this poll
IO interface by default when available.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
---
 src/librbd/ImageCtx.cc         |  9 +--------
 src/librbd/ImageCtx.h          | 11 +++++------
 src/librbd/internal.cc         |  8 +++-----
 src/librbd/io/AioCompletion.cc |  4 +---
 src/librbd/io/AioCompletion.h  |  8 +-------
 5 files changed, 11 insertions(+), 29 deletions(-)

diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc
index b39ef9302a5..a20069c6e16 100644
--- a/src/librbd/ImageCtx.cc
+++ b/src/librbd/ImageCtx.cc
@@ -110,7 +110,6 @@ public:
       timestamp_lock(util::unique_lock_name("librbd::ImageCtx::timestamp_lock", this)),
       async_ops_lock(util::unique_lock_name("librbd::ImageCtx::async_ops_lock", this)),
       copyup_list_lock(util::unique_lock_name("librbd::ImageCtx::copyup_list_lock", this)),
-      completed_reqs_lock(util::unique_lock_name("librbd::ImageCtx::completed_reqs_lock", this)),
       extra_read_flags(0),
       old_format(false),
       order(0), size(0), features(0),
@@ -123,6 +122,7 @@ public:
       operations(new Operations<>(*this)),
       exclusive_lock(nullptr), object_map(nullptr),
       io_work_queue(nullptr), op_work_queue(nullptr),
+      completed_reqs(32),
       asok_hook(nullptr),
       trace_endpoint("librbd")
   {
@@ -712,13 +712,6 @@ public:
     on_finish->complete(0);
   }
 
-  void ImageCtx::clear_pending_completions() {
-    Mutex::Locker l(completed_reqs_lock);
-    ldout(cct, 10) << "clear pending AioCompletion: count="
-                   << completed_reqs.size() << dendl;
-    completed_reqs.clear();
-  }
-
   void ImageCtx::apply_metadata(const std::map<std::string, bufferlist> &meta,
                                 bool thread_safe) {
     ldout(cct, 20) << __func__ << dendl;
diff --git a/src/librbd/ImageCtx.h b/src/librbd/ImageCtx.h
index 25a1300ce8e..1b7cf5a8d5e 100644
--- a/src/librbd/ImageCtx.h
+++ b/src/librbd/ImageCtx.h
@@ -31,6 +31,8 @@
 #include "librbd/AsyncRequest.h"
 #include "librbd/Types.h"
 
+#include <boost/lockfree/queue.hpp>
+
 class CephContext;
 class ContextWQ;
 class Finisher;
@@ -118,7 +120,6 @@ namespace librbd {
     RWLock timestamp_lock; // protects (create/access/modify)_timestamp
     Mutex async_ops_lock; // protects async_ops and async_requests
     Mutex copyup_list_lock; // protects copyup_waiting_list
-    Mutex completed_reqs_lock; // protects completed_reqs
 
     unsigned extra_read_flags;
 
@@ -167,11 +168,11 @@ namespace librbd {
     io::ImageRequestWQ<ImageCtx> *io_work_queue;
     io::ObjectDispatcher<ImageCtx> *io_object_dispatcher = nullptr;
 
-    xlist<io::AioCompletion*> completed_reqs;
-    EventSocket event_socket;
-
     ContextWQ *op_work_queue;
 
+    boost::lockfree::queue<io::AioCompletion*> completed_reqs;
+    EventSocket event_socket;
+
     bool ignore_migrating = false;
 
     /// Cached latency-sensitive configuration settings
@@ -301,8 +302,6 @@ namespace librbd {
     ObjectMap<ImageCtx> *create_object_map(uint64_t snap_id);
     Journal<ImageCtx> *create_journal();
 
-    void clear_pending_completions();
-
     void set_image_name(const std::string &name);
 
     void notify_update();
diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index a40649ae4c1..dadf699c29d 100644
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -1839,12 +1839,10 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
     ldout(cct, 20) << __func__ << " " << ictx << " numcomp = " << numcomp
                    << dendl;
     int i = 0;
-    Mutex::Locker l(ictx->completed_reqs_lock);
-    numcomp = std::min(numcomp, (int)ictx->completed_reqs.size());
-    while (i < numcomp) {
-      comps[i++] = ictx->completed_reqs.front();
-      ictx->completed_reqs.pop_front();
+    while (i < numcomp && ictx->completed_reqs.pop(comps[i])) {
+      ++i;
     }
+
     return i;
   }
 
diff --git a/src/librbd/io/AioCompletion.cc b/src/librbd/io/AioCompletion.cc
index fce9529d5f6..e151d622433 100644
--- a/src/librbd/io/AioCompletion.cc
+++ b/src/librbd/io/AioCompletion.cc
@@ -103,9 +103,7 @@ void AioCompletion::complete() {
   }
 
   if (ictx != nullptr && event_notify && ictx->event_socket.is_valid()) {
-    ictx->completed_reqs_lock.Lock();
-    ictx->completed_reqs.push_back(&m_xlist_item);
-    ictx->completed_reqs_lock.Unlock();
+    ictx->completed_reqs.push(this);
     ictx->event_socket.notify();
   }
   state = AIO_STATE_COMPLETE;
diff --git a/src/librbd/io/AioCompletion.h b/src/librbd/io/AioCompletion.h
index 0e6874126e2..46f2bd0b021 100644
--- a/src/librbd/io/AioCompletion.h
+++ b/src/librbd/io/AioCompletion.h
@@ -68,7 +68,6 @@ struct AioCompletion {
 
   AsyncOperation async_op;
 
-  xlist<AioCompletion*>::item m_xlist_item;
   bool event_notify = false;
 
   template <typename T, void (T::*MF)(int)>
@@ -104,7 +103,7 @@ struct AioCompletion {
     return comp;
   }
 
-  AioCompletion() : m_xlist_item(this) {
+  AioCompletion() {
   }
 
   ~AioCompletion() {
@@ -159,11 +158,6 @@ struct AioCompletion {
     ceph_assert(previous_ref > 0);
 
     if (previous_ref == 1) {
-      if (ictx != nullptr && event_notify) {
-        ictx->completed_reqs_lock.Lock();
-        m_xlist_item.remove_myself();
-        ictx->completed_reqs_lock.Unlock();
-      }
       delete this;
     }
   }
-- 
2.39.5