From: Jason Dillaman <dillaman@redhat.com>
Date: Wed, 17 Jun 2020 19:20:35 +0000 (-0400)
Subject: librbd: switch all queued contexts in IO path to use asio post
X-Git-Tag: v16.1.0~1685^2~9
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=bb14993873821307780ee0d267b6229a27bf3a40;p=ceph.git

librbd: switch all queued contexts in IO path to use asio post

The post operation is similar to the asio::ContextWQ::queue wrapper
but execution is not limited to a single thread strand.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
---

diff --git a/src/librbd/AsioEngine.cc b/src/librbd/AsioEngine.cc
index 3e62988283be..535ad57b5ac0 100644
--- a/src/librbd/AsioEngine.cc
+++ b/src/librbd/AsioEngine.cc
@@ -2,6 +2,7 @@
 // vim: ts=8 sw=2 smarttab
 
 #include "librbd/AsioEngine.h"
+#include "include/Context.h"
 #include "include/stringify.h"
 #include "include/neorados/RADOS.hpp"
 #include "include/rados/librados.hpp"
@@ -41,4 +42,12 @@ AsioEngine::~AsioEngine() {
   ldout(m_cct, 20) << dendl;
 }
 
+void AsioEngine::dispatch(Context* ctx, int r) {
+  dispatch([ctx, r]() { ctx->complete(r); });
+}
+
+void AsioEngine::post(Context* ctx, int r) {
+  post([ctx, r]() { ctx->complete(r); });
+}
+
 } // namespace librbd
diff --git a/src/librbd/AsioEngine.h b/src/librbd/AsioEngine.h
index d33843137249..00cb7f7f8a10 100644
--- a/src/librbd/AsioEngine.h
+++ b/src/librbd/AsioEngine.h
@@ -7,9 +7,12 @@
 #include "include/common_fwd.h"
 #include "include/rados/librados_fwd.hpp"
 #include <memory>
+#include <boost/asio/dispatch.hpp>
 #include <boost/asio/io_context.hpp>
 #include <boost/asio/io_context_strand.hpp>
+#include <boost/asio/post.hpp>
 
+struct Context;
 namespace neorados { struct RADOS; }
 
 namespace librbd {
@@ -49,6 +52,18 @@ public:
     return m_context_wq.get();
   }
 
+  template <typename T>
+  void dispatch(T&& t) {
+    boost::asio::dispatch(m_io_context, std::forward<T>(t));
+  }
+  void dispatch(Context* ctx, int r);
+
+  template <typename T>
+  void post(T&& t) {
+    boost::asio::post(m_io_context, std::forward<T>(t));
+  }
+  void post(Context* ctx, int r);
+
 private:
   std::shared_ptr<neorados::RADOS> m_rados_api;
   CephContext* m_cct;
diff --git a/src/librbd/io/AioCompletion.cc b/src/librbd/io/AioCompletion.cc
index 24e7afa4c75b..a50677215818 100644
--- a/src/librbd/io/AioCompletion.cc
+++ b/src/librbd/io/AioCompletion.cc
@@ -157,8 +157,8 @@ void AioCompletion::queue_complete() {
   add_request();
 
   // ensure completion fires in clean lock context
-  boost::asio::post(ictx->asio_engine, boost::asio::bind_executor(
-    ictx->asio_engine.get_api_strand(), [this]() {
+  boost::asio::post(*ictx->asio_engine, boost::asio::bind_executor(
+    ictx->asio_engine->get_api_strand(), [this]() {
       complete_request(0);
     }));
 }
@@ -261,8 +261,8 @@ void AioCompletion::complete_external_callback() {
 
   // ensure librbd external users never experience concurrent callbacks
   // from multiple librbd-internal threads.
-  boost::asio::dispatch(ictx->asio_engine, boost::asio::bind_executor(
-    ictx->asio_engine.get_api_strand(), [this]() {
+  boost::asio::dispatch(*ictx->asio_engine, boost::asio::bind_executor(
+    ictx->asio_engine->get_api_strand(), [this]() {
       complete_cb(rbd_comp, complete_arg);
       complete_event_socket();
       put();
diff --git a/src/librbd/io/AsyncOperation.cc b/src/librbd/io/AsyncOperation.cc
index 4c4d508e7d25..18db2410e4a5 100644
--- a/src/librbd/io/AsyncOperation.cc
+++ b/src/librbd/io/AsyncOperation.cc
@@ -2,10 +2,10 @@
 // vim: ts=8 sw=2 smarttab
 
 #include "librbd/io/AsyncOperation.h"
-#include "librbd/ImageCtx.h"
-#include "librbd/asio/ContextWQ.h"
-#include "common/dout.h"
 #include "include/ceph_assert.h"
+#include "common/dout.h"
+#include "librbd/AsioEngine.h"
+#include "librbd/ImageCtx.h"
 
 #define dout_subsys ceph_subsys_rbd
 #undef dout_prefix
@@ -70,7 +70,7 @@ void AsyncOperation::finish_op() {
   if (!m_flush_contexts.empty()) {
     C_CompleteFlushes *ctx = new C_CompleteFlushes(m_image_ctx,
                                                    std::move(m_flush_contexts));
-    m_image_ctx->op_work_queue->queue(ctx);
+    m_image_ctx->asio_engine->post(ctx, 0);
   }
 }
 
@@ -87,7 +87,7 @@ void AsyncOperation::flush(Context* on_finish) {
     }
   }
 
-  m_image_ctx->op_work_queue->queue(on_finish);
+  m_image_ctx->asio_engine->post(on_finish, 0);
 }
 
 } // namespace io
diff --git a/src/librbd/io/CopyupRequest.cc b/src/librbd/io/CopyupRequest.cc
index d41573d69311..27d396a0eaf1 100644
--- a/src/librbd/io/CopyupRequest.cc
+++ b/src/librbd/io/CopyupRequest.cc
@@ -6,6 +6,7 @@
 #include "common/ceph_mutex.h"
 #include "common/dout.h"
 #include "common/errno.h"
+#include "librbd/AsioEngine.h"
 #include "librbd/AsyncObjectThrottle.h"
 #include "librbd/ExclusiveLock.h"
 #include "librbd/ImageCtx.h"
@@ -154,10 +155,8 @@ void CopyupRequest<I>::read_from_parent() {
   if (m_image_ctx->parent == nullptr) {
     ldout(cct, 5) << "parent detached" << dendl;
 
-    m_image_ctx->op_work_queue->queue(
-      util::create_context_callback<
-        CopyupRequest<I>, &CopyupRequest<I>::handle_read_from_parent>(this),
-      -ENOENT);
+    m_image_ctx->asio_engine->post(
+      [this]() { handle_read_from_parent(-ENOENT); });
     return;
   } else if (is_deep_copy()) {
     deep_copy();
@@ -470,21 +469,28 @@ template <typename I>
 void CopyupRequest<I>::handle_copyup(int r) {
   auto cct = m_image_ctx->cct;
   unsigned pending_copyups;
+  int copyup_ret_val = r;
   {
     std::lock_guard locker{m_lock};
     ceph_assert(m_pending_copyups > 0);
     pending_copyups = --m_pending_copyups;
+    if (m_copyup_ret_val < 0) {
+      copyup_ret_val = m_copyup_ret_val;
+    } else if (r < 0) {
+      m_copyup_ret_val = r;
+    }
   }
 
   ldout(cct, 20) << "r=" << r << ", "
                  << "pending=" << pending_copyups << dendl;
 
-  if (r < 0 && r != -ENOENT) {
-    lderr(cct) << "failed to copyup object: " << cpp_strerror(r) << dendl;
-    complete_requests(false, r);
-  }
-
   if (pending_copyups == 0) {
+    if (copyup_ret_val < 0 && copyup_ret_val != -ENOENT) {
+      lderr(cct) << "failed to copyup object: " << cpp_strerror(copyup_ret_val)
+                 << dendl;
+      complete_requests(false, copyup_ret_val);
+    }
+
     finish(0);
   }
 }
@@ -603,6 +609,7 @@ void CopyupRequest<I>::compute_deep_copy_snap_ids() {
       deep_copied.insert(it.second.front());
     }
   }
+  ldout(m_image_ctx->cct, 15) << "deep_copied=" << deep_copied << dendl;
 
   std::copy_if(m_image_ctx->snaps.rbegin(), m_image_ctx->snaps.rend(),
                std::back_inserter(m_snap_ids),
diff --git a/src/librbd/io/CopyupRequest.h b/src/librbd/io/CopyupRequest.h
index 01d679f6213b..1dc9d1ca4931 100644
--- a/src/librbd/io/CopyupRequest.h
+++ b/src/librbd/io/CopyupRequest.h
@@ -99,6 +99,7 @@ private:
   ceph::mutex m_lock = ceph::make_mutex("CopyupRequest", false);
   WriteRequests m_pending_requests;
   unsigned m_pending_copyups = 0;
+  int m_copyup_ret_val = 0;
 
   WriteRequests m_restart_requests;
   bool m_append_request_permitted = true;
diff --git a/src/librbd/io/ObjectDispatch.cc b/src/librbd/io/ObjectDispatch.cc
index ecf9ae23d078..3e202f1b0795 100644
--- a/src/librbd/io/ObjectDispatch.cc
+++ b/src/librbd/io/ObjectDispatch.cc
@@ -3,9 +3,9 @@
 
 #include "librbd/io/ObjectDispatch.h"
 #include "common/dout.h"
+#include "librbd/AsioEngine.h"
 #include "librbd/ImageCtx.h"
 #include "librbd/Utils.h"
-#include "librbd/asio/ContextWQ.h"
 #include "librbd/io/ObjectRequest.h"
 
 #define dout_subsys ceph_subsys_rbd
@@ -28,7 +28,7 @@ void ObjectDispatch<I>::shut_down(Context* on_finish) {
   auto cct = m_image_ctx->cct;
   ldout(cct, 5) << dendl;
 
-  m_image_ctx->op_work_queue->queue(on_finish, 0);
+  m_image_ctx->asio_engine->post(on_finish, 0);
 }
 
 template <typename I>
diff --git a/src/librbd/io/ObjectRequest.cc b/src/librbd/io/ObjectRequest.cc
index 51efbae21efb..407bc1d33304 100644
--- a/src/librbd/io/ObjectRequest.cc
+++ b/src/librbd/io/ObjectRequest.cc
@@ -10,11 +10,11 @@
 #include "include/err.h"
 #include "osd/osd_types.h"
 
+#include "librbd/AsioEngine.h"
 #include "librbd/ExclusiveLock.h"
 #include "librbd/ImageCtx.h"
 #include "librbd/ObjectMap.h"
 #include "librbd/Utils.h"
-#include "librbd/asio/ContextWQ.h"
 #include "librbd/io/AioCompletion.h"
 #include "librbd/io/CopyupRequest.h"
 #include "librbd/io/ImageRequest.h"
@@ -165,8 +165,7 @@ bool ObjectRequest<I>::compute_parent_extents(Extents *parent_extents,
 template <typename I>
 void ObjectRequest<I>::async_finish(int r) {
   ldout(m_ictx->cct, 20) << "r=" << r << dendl;
-  m_ictx->op_work_queue->queue(util::create_context_callback<
-    ObjectRequest<I>, &ObjectRequest<I>::finish>(this), r);
+  m_ictx->asio_engine->post([this, r]() { finish(r); });
 }
 
 template <typename I>
@@ -203,9 +202,7 @@ void ObjectReadRequest<I>::read_object() {
     std::shared_lock image_locker{image_ctx->image_lock};
     if (image_ctx->object_map != nullptr &&
         !image_ctx->object_map->object_may_exist(this->m_object_no)) {
-      image_ctx->op_work_queue->queue(new LambdaContext([this](int r) {
-          read_parent();
-        }), 0);
+      image_ctx->asio_engine->post([this]() { read_parent(); });
       return;
     }
   }
diff --git a/src/librbd/io/QosImageDispatch.cc b/src/librbd/io/QosImageDispatch.cc
index f19471f8d73f..8badb58b1a4a 100644
--- a/src/librbd/io/QosImageDispatch.cc
+++ b/src/librbd/io/QosImageDispatch.cc
@@ -3,8 +3,8 @@
 
 #include "librbd/io/QosImageDispatch.h"
 #include "common/dout.h"
+#include "librbd/AsioEngine.h"
 #include "librbd/ImageCtx.h"
-#include "librbd/asio/ContextWQ.h"
 #include "librbd/io/FlushTracker.h"
 #include <map>
 
@@ -282,7 +282,7 @@ void QosImageDispatch<I>::handle_throttle_ready(Tag&& tag, uint64_t flag) {
 
   if (set_throttle_flag(tag.image_dispatch_flags, flag)) {
     // timer_lock is held -- so dispatch from outside the timer thread
-    m_image_ctx->op_work_queue->queue(tag.on_dispatched, 0);
+    m_image_ctx->asio_engine->post(tag.on_dispatched, 0);
   }
 }
 
diff --git a/src/librbd/io/QueueImageDispatch.cc b/src/librbd/io/QueueImageDispatch.cc
index d901d2ebbd0b..7cb5dfc51484 100644
--- a/src/librbd/io/QueueImageDispatch.cc
+++ b/src/librbd/io/QueueImageDispatch.cc
@@ -4,9 +4,9 @@
 #include "librbd/io/QueueImageDispatch.h"
 #include "common/dout.h"
 #include "common/Cond.h"
+#include "librbd/AsioEngine.h"
 #include "librbd/ImageCtx.h"
 #include "librbd/Utils.h"
-#include "librbd/asio/ContextWQ.h"
 #include "librbd/io/AioCompletion.h"
 #include "librbd/io/ImageDispatchSpec.h"
 
@@ -115,7 +115,7 @@ bool QueueImageDispatch<I>::enqueue(
   }
 
   *dispatch_result = DISPATCH_RESULT_CONTINUE;
-  m_image_ctx->op_work_queue->queue(on_dispatched, 0);
+  m_image_ctx->asio_engine->post(on_dispatched, 0);
   return true;
 }
 
diff --git a/src/librbd/io/SimpleSchedulerObjectDispatch.cc b/src/librbd/io/SimpleSchedulerObjectDispatch.cc
index ec9c3b434084..f6781b6646e6 100644
--- a/src/librbd/io/SimpleSchedulerObjectDispatch.cc
+++ b/src/librbd/io/SimpleSchedulerObjectDispatch.cc
@@ -4,9 +4,9 @@
 #include "librbd/io/SimpleSchedulerObjectDispatch.h"
 #include "common/Timer.h"
 #include "common/errno.h"
+#include "librbd/AsioEngine.h"
 #include "librbd/ImageCtx.h"
 #include "librbd/Utils.h"
-#include "librbd/asio/ContextWQ.h"
 #include "librbd/io/ObjectDispatchSpec.h"
 #include "librbd/io/ObjectDispatcher.h"
 #include "librbd/io/Utils.h"
@@ -505,12 +505,11 @@ void SimpleSchedulerObjectDispatch<I>::schedule_dispatch_delayed_requests() {
       ldout(cct, 20) << "running timer task " << m_timer_task << dendl;
 
       m_timer_task = nullptr;
-      m_image_ctx->op_work_queue->queue(
-          new LambdaContext(
-            [this, object_no](int r) {
-	      std::lock_guard locker{m_lock};
-              dispatch_delayed_requests(object_no);
-            }), 0);
+      m_image_ctx->asio_engine->post(
+        [this, object_no]() {
+          std::lock_guard locker{m_lock};
+          dispatch_delayed_requests(object_no);
+        });
     });
 
   ldout(cct, 20) << "scheduling task " << m_timer_task << " at "