From b4861a5cfead7439bd92454e94b9850c2b5ec099 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Tue, 28 Feb 2017 11:26:35 -0500 Subject: [PATCH] rbd-mirror: eliminate context switch from close image state machine Signed-off-by: Jason Dillaman --- .../test_mock_BootstrapRequest.cc | 4 +- .../test_mock_CreateImageRequest.cc | 4 +- .../rbd_mirror/test_mock_ImageReplayer.cc | 1 - src/tools/rbd_mirror/ImageReplayer.cc | 2 +- .../image_replayer/BootstrapRequest.cc | 6 +-- .../image_replayer/CloseImageRequest.cc | 31 +--------------- .../image_replayer/CloseImageRequest.h | 20 ++-------- .../image_replayer/CreateImageRequest.cc | 10 ++--- .../image_replayer/OpenImageRequest.cc | 37 +++---------------- .../image_replayer/OpenImageRequest.h | 19 ++++------ .../image_replayer/OpenLocalImageRequest.cc | 18 +++++---- .../image_replayer/OpenLocalImageRequest.h | 2 +- 12 files changed, 40 insertions(+), 114 deletions(-) diff --git a/src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc b/src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc index c6dee1859325..cc1bfc55dc37 100644 --- a/src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc +++ b/src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc @@ -101,7 +101,6 @@ struct CloseImageRequest { Context *on_finish = nullptr; static CloseImageRequest* create(librbd::MockTestImageCtx **image_ctx, - ContextWQ *work_queue, bool destroy_only, Context *on_finish) { assert(s_instance != nullptr); s_instance->image_ctx = image_ctx; @@ -184,8 +183,7 @@ struct OpenImageRequest { static OpenImageRequest* create(librados::IoCtx &io_ctx, librbd::MockTestImageCtx **image_ctx, const std::string &image_id, - bool read_only, ContextWQ *work_queue, - Context *on_finish) { + bool read_only, Context *on_finish) { assert(s_instance != nullptr); s_instance->image_ctx = image_ctx; s_instance->on_finish = on_finish; diff --git a/src/test/rbd_mirror/image_replayer/test_mock_CreateImageRequest.cc b/src/test/rbd_mirror/image_replayer/test_mock_CreateImageRequest.cc index 297fd6bf7adf..455362d30d13 100644 --- a/src/test/rbd_mirror/image_replayer/test_mock_CreateImageRequest.cc +++ b/src/test/rbd_mirror/image_replayer/test_mock_CreateImageRequest.cc @@ -118,7 +118,6 @@ struct CloseImageRequest { Context *on_finish = nullptr; static CloseImageRequest* create(librbd::MockTestImageCtx **image_ctx, - ContextWQ *work_queue, bool destroy_only, Context *on_finish) { assert(s_instance != nullptr); s_instance->construct(*image_ctx); @@ -147,8 +146,7 @@ struct OpenImageRequest { static OpenImageRequest* create(librados::IoCtx &io_ctx, librbd::MockTestImageCtx **image_ctx, const std::string &image_id, - bool read_only, ContextWQ *work_queue, - Context *on_finish) { + bool read_only, Context *on_finish) { assert(s_instance != nullptr); s_instance->image_ctx = image_ctx; s_instance->on_finish = on_finish; diff --git a/src/test/rbd_mirror/test_mock_ImageReplayer.cc b/src/test/rbd_mirror/test_mock_ImageReplayer.cc index ee3e466a6d3c..ead5d102efa0 100644 --- a/src/test/rbd_mirror/test_mock_ImageReplayer.cc +++ b/src/test/rbd_mirror/test_mock_ImageReplayer.cc @@ -147,7 +147,6 @@ struct CloseImageRequest { Context *on_finish = nullptr; static CloseImageRequest* create(librbd::MockTestImageCtx **image_ctx, - ContextWQ *work_queue, bool destroy_only, Context *on_finish) { assert(s_instance != nullptr); s_instance->image_ctx = image_ctx; diff --git a/src/tools/rbd_mirror/ImageReplayer.cc b/src/tools/rbd_mirror/ImageReplayer.cc index 0ef8560d7f97..bd5036b53cec 100644 --- a/src/tools/rbd_mirror/ImageReplayer.cc +++ b/src/tools/rbd_mirror/ImageReplayer.cc @@ -1423,7 +1423,7 @@ void ImageReplayer::shut_down(int r) { if (m_local_image_ctx) { ctx = new FunctionContext([this, ctx](int r) { CloseImageRequest *request = CloseImageRequest::create( - &m_local_image_ctx, m_threads->work_queue, false, ctx); + &m_local_image_ctx, ctx); request->send(); }); } diff --git a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc index bd820c208f96..c1d805af4c26 100644 --- a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc +++ b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc @@ -253,7 +253,7 @@ void BootstrapRequest::open_remote_image() { this); OpenImageRequest *request = OpenImageRequest::create( m_remote_io_ctx, &m_remote_image_ctx, m_remote_image_id, false, - m_work_queue, ctx); + ctx); request->send(); } @@ -729,7 +729,7 @@ void BootstrapRequest::close_local_image() { BootstrapRequest, &BootstrapRequest::handle_close_local_image>( this); CloseImageRequest *request = CloseImageRequest::create( - m_local_image_ctx, m_work_queue, false, ctx); + m_local_image_ctx, ctx); request->send(); } @@ -755,7 +755,7 @@ void BootstrapRequest::close_remote_image() { BootstrapRequest, &BootstrapRequest::handle_close_remote_image>( this); CloseImageRequest *request = CloseImageRequest::create( - &m_remote_image_ctx, m_work_queue, false, ctx); + &m_remote_image_ctx, ctx); request->send(); } diff --git a/src/tools/rbd_mirror/image_replayer/CloseImageRequest.cc b/src/tools/rbd_mirror/image_replayer/CloseImageRequest.cc index d7ea6085edbe..234fded69ee8 100644 --- a/src/tools/rbd_mirror/image_replayer/CloseImageRequest.cc +++ b/src/tools/rbd_mirror/image_replayer/CloseImageRequest.cc @@ -22,10 +22,8 @@ namespace image_replayer { using librbd::util::create_context_callback; template -CloseImageRequest::CloseImageRequest(I **image_ctx, ContextWQ *work_queue, - bool destroy_only, Context *on_finish) - : m_image_ctx(image_ctx), m_work_queue(work_queue), - m_destroy_only(destroy_only), m_on_finish(on_finish) { +CloseImageRequest::CloseImageRequest(I **image_ctx, Context *on_finish) + : m_image_ctx(image_ctx), m_on_finish(on_finish) { } template @@ -35,11 +33,6 @@ void CloseImageRequest::send() { template void CloseImageRequest::close_image() { - if (m_destroy_only) { - switch_thread_context(); - return; - } - dout(20) << dendl; Context *ctx = create_context_callback< @@ -56,26 +49,6 @@ void CloseImageRequest::handle_close_image(int r) { << dendl; } - switch_thread_context(); -} - -template -void CloseImageRequest::switch_thread_context() { - dout(20) << dendl; - - // swap the librbd thread context for the rbd-mirror thread context - Context *ctx = create_context_callback< - CloseImageRequest, &CloseImageRequest::handle_switch_thread_context>( - this); - m_work_queue->queue(ctx, 0); -} - -template -void CloseImageRequest::handle_switch_thread_context(int r) { - dout(20) << dendl; - - assert(r == 0); - delete *m_image_ctx; *m_image_ctx = nullptr; diff --git a/src/tools/rbd_mirror/image_replayer/CloseImageRequest.h b/src/tools/rbd_mirror/image_replayer/CloseImageRequest.h index dddad47220e2..02481369d000 100644 --- a/src/tools/rbd_mirror/image_replayer/CloseImageRequest.h +++ b/src/tools/rbd_mirror/image_replayer/CloseImageRequest.h @@ -9,7 +9,6 @@ #include class Context; -class ContextWQ; namespace librbd { class ImageCtx; } namespace rbd { @@ -19,14 +18,11 @@ namespace image_replayer { template class CloseImageRequest { public: - static CloseImageRequest* create(ImageCtxT **image_ctx, ContextWQ *work_queue, - bool destroy_only, Context *on_finish) { - return new CloseImageRequest(image_ctx, work_queue, destroy_only, - on_finish); + static CloseImageRequest* create(ImageCtxT **image_ctx, Context *on_finish) { + return new CloseImageRequest(image_ctx, on_finish); } - CloseImageRequest(ImageCtxT **image_ctx, ContextWQ *work_queue, - bool destroy_only, Context *on_finish); + CloseImageRequest(ImageCtxT **image_ctx, Context *on_finish); void send(); @@ -37,10 +33,7 @@ private: * * | * v - * CLOSE_IMAGE (skip if not needed) - * | - * v - * SWITCH_CONTEXT + * CLOSE_IMAGE * | * v * @@ -48,15 +41,10 @@ private: * @endverbatim */ ImageCtxT **m_image_ctx; - ContextWQ *m_work_queue; - bool m_destroy_only; Context *m_on_finish; void close_image(); void handle_close_image(int r); - - void switch_thread_context(); - void handle_switch_thread_context(int r); }; } // namespace image_replayer diff --git a/src/tools/rbd_mirror/image_replayer/CreateImageRequest.cc b/src/tools/rbd_mirror/image_replayer/CreateImageRequest.cc index 88d856c467f7..6097388c82d4 100644 --- a/src/tools/rbd_mirror/image_replayer/CreateImageRequest.cc +++ b/src/tools/rbd_mirror/image_replayer/CreateImageRequest.cc @@ -193,7 +193,7 @@ void CreateImageRequest::open_remote_parent_image() { &CreateImageRequest::handle_open_remote_parent_image>(this); OpenImageRequest *request = OpenImageRequest::create( m_remote_parent_io_ctx, &m_remote_parent_image_ctx, - m_remote_parent_spec.image_id, true, m_work_queue, ctx); + m_remote_parent_spec.image_id, true, ctx); request->send(); } @@ -218,8 +218,8 @@ void CreateImageRequest::open_local_parent_image() { CreateImageRequest, &CreateImageRequest::handle_open_local_parent_image>(this); OpenImageRequest *request = OpenImageRequest::create( - m_local_parent_io_ctx, &m_local_parent_image_ctx, m_local_parent_spec.image_id, - true, m_work_queue, ctx); + m_local_parent_io_ctx, &m_local_parent_image_ctx, + m_local_parent_spec.image_id, true, ctx); request->send(); } @@ -320,7 +320,7 @@ void CreateImageRequest::close_local_parent_image() { CreateImageRequest, &CreateImageRequest::handle_close_local_parent_image>(this); CloseImageRequest *request = CloseImageRequest::create( - &m_local_parent_image_ctx, m_work_queue, false, ctx); + &m_local_parent_image_ctx, ctx); request->send(); } @@ -342,7 +342,7 @@ void CreateImageRequest::close_remote_parent_image() { CreateImageRequest, &CreateImageRequest::handle_close_remote_parent_image>(this); CloseImageRequest *request = CloseImageRequest::create( - &m_remote_parent_image_ctx, m_work_queue, false, ctx); + &m_remote_parent_image_ctx, ctx); request->send(); } diff --git a/src/tools/rbd_mirror/image_replayer/OpenImageRequest.cc b/src/tools/rbd_mirror/image_replayer/OpenImageRequest.cc index 3ffdaad6ea6b..efba012731b0 100644 --- a/src/tools/rbd_mirror/image_replayer/OpenImageRequest.cc +++ b/src/tools/rbd_mirror/image_replayer/OpenImageRequest.cc @@ -2,9 +2,7 @@ // vim: ts=8 sw=2 smarttab #include "OpenImageRequest.h" -#include "CloseImageRequest.h" #include "common/errno.h" -#include "common/WorkQueue.h" #include "librbd/ImageCtx.h" #include "librbd/ImageState.h" #include "librbd/Utils.h" @@ -25,10 +23,9 @@ using librbd::util::create_context_callback; template OpenImageRequest::OpenImageRequest(librados::IoCtx &io_ctx, I **image_ctx, const std::string &image_id, - bool read_only, ContextWQ *work_queue, - Context *on_finish) + bool read_only, Context *on_finish) : m_io_ctx(io_ctx), m_image_ctx(image_ctx), m_image_id(image_id), - m_read_only(read_only), m_work_queue(work_queue), m_on_finish(on_finish) { + m_read_only(read_only), m_on_finish(on_finish) { } template @@ -55,35 +52,11 @@ void OpenImageRequest::handle_open_image(int r) { if (r < 0) { derr << ": failed to open image '" << m_image_id << "': " << cpp_strerror(r) << dendl; - send_close_image(r); - return; + (*m_image_ctx)->destroy(); + *m_image_ctx = nullptr; } - finish(0); -} - -template -void OpenImageRequest::send_close_image(int r) { - dout(20) << dendl; - - if (m_ret_val == 0 && r < 0) { - m_ret_val = r; - } - - Context *ctx = create_context_callback< - OpenImageRequest, &OpenImageRequest::handle_close_image>( - this); - CloseImageRequest *request = CloseImageRequest::create( - m_image_ctx, m_work_queue, true, ctx); - request->send(); -} - -template -void OpenImageRequest::handle_close_image(int r) { - dout(20) << dendl; - - assert(r == 0); - finish(m_ret_val); + finish(r); } template diff --git a/src/tools/rbd_mirror/image_replayer/OpenImageRequest.h b/src/tools/rbd_mirror/image_replayer/OpenImageRequest.h index d9407c052b16..01ab31171950 100644 --- a/src/tools/rbd_mirror/image_replayer/OpenImageRequest.h +++ b/src/tools/rbd_mirror/image_replayer/OpenImageRequest.h @@ -9,7 +9,6 @@ #include class Context; -class ContextWQ; namespace librbd { class ImageCtx; } namespace rbd { @@ -22,15 +21,14 @@ public: static OpenImageRequest* create(librados::IoCtx &io_ctx, ImageCtxT **image_ctx, const std::string &image_id, - bool read_only, ContextWQ *work_queue, - Context *on_finish) { + bool read_only, Context *on_finish) { return new OpenImageRequest(io_ctx, image_ctx, image_id, read_only, - work_queue, on_finish); + on_finish); } OpenImageRequest(librados::IoCtx &io_ctx, ImageCtxT **image_ctx, const std::string &image_id, bool read_only, - ContextWQ *m_work_queue, Context *on_finish); + Context *on_finish); void send(); @@ -41,10 +39,10 @@ private: * * | * v - * OPEN_IMAGE * * * * * * * * - * | * - * v v - * <---------- CLOSE_IMAGE + * OPEN_IMAGE + * | + * v + * * * @endverbatim */ @@ -52,11 +50,8 @@ private: ImageCtxT **m_image_ctx; std::string m_image_id; bool m_read_only; - ContextWQ *m_work_queue; Context *m_on_finish; - int m_ret_val = 0; - void send_open_image(); void handle_open_image(int r); diff --git a/src/tools/rbd_mirror/image_replayer/OpenLocalImageRequest.cc b/src/tools/rbd_mirror/image_replayer/OpenLocalImageRequest.cc index cf37048aa5db..5f67c41cfa43 100644 --- a/src/tools/rbd_mirror/image_replayer/OpenLocalImageRequest.cc +++ b/src/tools/rbd_mirror/image_replayer/OpenLocalImageRequest.cc @@ -112,7 +112,9 @@ void OpenLocalImageRequest::handle_open_image(int r) { if (r < 0) { derr << ": failed to open image '" << m_local_image_id << "': " << cpp_strerror(r) << dendl; - send_close_image(true, r); + (*m_local_image_ctx)->destroy(); + *m_local_image_ctx = nullptr; + finish(r); return; } @@ -138,7 +140,7 @@ void OpenLocalImageRequest::handle_is_primary(int r) { if (r < 0) { derr << ": error querying local image primary status: " << cpp_strerror(r) << dendl; - send_close_image(false, r); + send_close_image(r); return; } @@ -146,7 +148,7 @@ void OpenLocalImageRequest::handle_is_primary(int r) { // we aren't going to mirror peer data into this image anyway if (m_primary) { dout(10) << ": local image is primary -- skipping image replay" << dendl; - send_close_image(false, -EREMOTEIO); + send_close_image(-EREMOTEIO); return; } @@ -160,7 +162,7 @@ void OpenLocalImageRequest::send_lock_image() { RWLock::RLocker owner_locker((*m_local_image_ctx)->owner_lock); if ((*m_local_image_ctx)->exclusive_lock == nullptr) { derr << ": image does not support exclusive lock" << dendl; - send_close_image(false, -EINVAL); + send_close_image(-EINVAL); return; } @@ -181,7 +183,7 @@ void OpenLocalImageRequest::handle_lock_image(int r) { if (r < 0) { derr << ": failed to lock image '" << m_local_image_id << "': " << cpp_strerror(r) << dendl; - send_close_image(false, r); + send_close_image(r); return; } @@ -190,7 +192,7 @@ void OpenLocalImageRequest::handle_lock_image(int r) { if ((*m_local_image_ctx)->exclusive_lock == nullptr || !(*m_local_image_ctx)->exclusive_lock->is_lock_owner()) { derr << ": image is not locked" << dendl; - send_close_image(false, -EBUSY); + send_close_image(-EBUSY); return; } } @@ -199,7 +201,7 @@ void OpenLocalImageRequest::handle_lock_image(int r) { } template -void OpenLocalImageRequest::send_close_image(bool destroy_only, int r) { +void OpenLocalImageRequest::send_close_image(int r) { dout(20) << dendl; if (m_ret_val == 0 && r < 0) { @@ -210,7 +212,7 @@ void OpenLocalImageRequest::send_close_image(bool destroy_only, int r) { OpenLocalImageRequest, &OpenLocalImageRequest::handle_close_image>( this); CloseImageRequest *request = CloseImageRequest::create( - m_local_image_ctx, m_work_queue, destroy_only, ctx); + m_local_image_ctx, ctx); request->send(); } diff --git a/src/tools/rbd_mirror/image_replayer/OpenLocalImageRequest.h b/src/tools/rbd_mirror/image_replayer/OpenLocalImageRequest.h index 2a1bbb2a8e2e..570728dd762b 100644 --- a/src/tools/rbd_mirror/image_replayer/OpenLocalImageRequest.h +++ b/src/tools/rbd_mirror/image_replayer/OpenLocalImageRequest.h @@ -78,7 +78,7 @@ private: void send_lock_image(); void handle_lock_image(int r); - void send_close_image(bool destroy_only, int r); + void send_close_image(int r); void handle_close_image(int r); void finish(int r); -- 2.47.3