From: Jason Dillaman <dillaman@redhat.com>
Date: Tue, 6 Mar 2018 19:23:47 +0000 (-0500)
Subject: librbd: switch to lock-free queue for event poll IO interface
X-Git-Tag: v15.1.0~2767^2~6
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=ef9d74720f2e8ba3146774e75f60fed815a3c9de;p=ceph.git

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>
---

diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc
index b39ef9302a54..a20069c6e163 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 25a1300ce8ed..1b7cf5a8d5ee 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 a40649ae4c1b..dadf699c29d7 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 fce9529d5f61..e151d622433d 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 0e6874126e28..46f2bd0b0216 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;
     }
   }