From: Jason Dillaman Date: Thu, 17 Sep 2020 18:52:30 +0000 (-0400) Subject: librbd: skip flush from exclusive-lock dispatch layer on init/shutdown X-Git-Tag: v16.1.0~1032^2~11 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=437354ffcd884a94cefdd626c6dcd5f1b7488710;p=ceph.git librbd: skip flush from exclusive-lock dispatch layer on init/shutdown If the exclusive-lock layer is being initialized/shut down at image open/close, there is no IO flowing so there is no need to flush. Signed-off-by: Jason Dillaman --- diff --git a/src/librbd/ExclusiveLock.cc b/src/librbd/ExclusiveLock.cc index a475d1c53e3b..92ddc665b7f6 100644 --- a/src/librbd/ExclusiveLock.cc +++ b/src/librbd/ExclusiveLock.cc @@ -75,9 +75,10 @@ bool ExclusiveLock::accept_ops(const ceph::mutex &lock) const { } template -void ExclusiveLock::set_require_lock(io::Direction direction, +void ExclusiveLock::set_require_lock(bool init_shutdown, + io::Direction direction, Context* on_finish) { - m_image_dispatch->set_require_lock(direction, on_finish); + m_image_dispatch->set_require_lock(init_shutdown, direction, on_finish); } template @@ -209,9 +210,9 @@ void ExclusiveLock::handle_init_complete(int r, uint64_t features, if (m_image_ctx.clone_copy_on_read || (features & RBD_FEATURE_JOURNALING) != 0 || rwl_enabled) { - m_image_dispatch->set_require_lock(io::DIRECTION_BOTH, on_finish); + m_image_dispatch->set_require_lock(true, io::DIRECTION_BOTH, on_finish); } else { - m_image_dispatch->set_require_lock(io::DIRECTION_WRITE, on_finish); + m_image_dispatch->set_require_lock(true, io::DIRECTION_WRITE, on_finish); } } diff --git a/src/librbd/ExclusiveLock.h b/src/librbd/ExclusiveLock.h index b77c961e2f0c..4a3215ce98c3 100644 --- a/src/librbd/ExclusiveLock.h +++ b/src/librbd/ExclusiveLock.h @@ -30,7 +30,8 @@ public: int *ret_val) const; bool accept_ops() const; - void set_require_lock(io::Direction direction, Context* on_finish); + void set_require_lock(bool init_shutdown, io::Direction direction, + Context* on_finish); void unset_require_lock(io::Direction direction); void block_requests(int r); diff --git a/src/librbd/exclusive_lock/ImageDispatch.cc b/src/librbd/exclusive_lock/ImageDispatch.cc index fe3ab14bace2..e734cecf22a6 100644 --- a/src/librbd/exclusive_lock/ImageDispatch.cc +++ b/src/librbd/exclusive_lock/ImageDispatch.cc @@ -57,7 +57,8 @@ void ImageDispatch::shut_down(Context* on_finish) { } template -void ImageDispatch::set_require_lock(io::Direction direction, +void ImageDispatch::set_require_lock(bool init_shutdown, + io::Direction direction, Context* on_finish) { // pause any matching IO from proceeding past this layer set_require_lock(direction, true); @@ -72,7 +73,9 @@ void ImageDispatch::set_require_lock(io::Direction direction, on_finish, util::get_image_ctx(m_image_ctx), io::AIO_TYPE_FLUSH); auto req = io::ImageDispatchSpec::create_flush( *m_image_ctx, io::IMAGE_DISPATCH_LAYER_EXCLUSIVE_LOCK, aio_comp, - io::FLUSH_SOURCE_EXCLUSIVE_LOCK, {}); + (init_shutdown ? + io::FLUSH_SOURCE_EXCLUSIVE_LOCK_SKIP_REFRESH : + io::FLUSH_SOURCE_EXCLUSIVE_LOCK), {}); req->send(); } diff --git a/src/librbd/exclusive_lock/ImageDispatch.h b/src/librbd/exclusive_lock/ImageDispatch.h index 546163e87b21..2a20407f3169 100644 --- a/src/librbd/exclusive_lock/ImageDispatch.h +++ b/src/librbd/exclusive_lock/ImageDispatch.h @@ -45,7 +45,8 @@ public: return io::IMAGE_DISPATCH_LAYER_EXCLUSIVE_LOCK; } - void set_require_lock(io::Direction direction, Context* on_finish); + void set_require_lock(bool init_shutdown, + io::Direction direction, Context* on_finish); void unset_require_lock(io::Direction direction); void shut_down(Context* on_finish) override; diff --git a/src/librbd/exclusive_lock/PreReleaseRequest.cc b/src/librbd/exclusive_lock/PreReleaseRequest.cc index 5eae45dab3a7..644aa57415e6 100644 --- a/src/librbd/exclusive_lock/PreReleaseRequest.cc +++ b/src/librbd/exclusive_lock/PreReleaseRequest.cc @@ -116,9 +116,11 @@ void PreReleaseRequest::send_block_writes() { // queue to re-request the lock if any IO is queued if (m_image_ctx.clone_copy_on_read || m_image_ctx.test_features(RBD_FEATURE_JOURNALING)) { - m_image_dispatch->set_require_lock(io::DIRECTION_BOTH, ctx); + m_image_dispatch->set_require_lock(m_shutting_down, + io::DIRECTION_BOTH, ctx); } else { - m_image_dispatch->set_require_lock(io::DIRECTION_WRITE, ctx); + m_image_dispatch->set_require_lock(m_shutting_down, + io::DIRECTION_WRITE, ctx); } } diff --git a/src/librbd/image/RefreshRequest.cc b/src/librbd/image/RefreshRequest.cc index b66f375315f1..ce125e2ce398 100644 --- a/src/librbd/image/RefreshRequest.cc +++ b/src/librbd/image/RefreshRequest.cc @@ -922,7 +922,7 @@ void RefreshRequest::send_v2_open_journal() { send_v2_block_writes(); }); m_image_ctx.exclusive_lock->set_require_lock( - librbd::io::DIRECTION_BOTH, ctx); + true, librbd::io::DIRECTION_BOTH, ctx); return; } diff --git a/src/test/librbd/exclusive_lock/test_mock_PreReleaseRequest.cc b/src/test/librbd/exclusive_lock/test_mock_PreReleaseRequest.cc index ac43ddf4f321..a1429c619f1a 100644 --- a/src/test/librbd/exclusive_lock/test_mock_PreReleaseRequest.cc +++ b/src/test/librbd/exclusive_lock/test_mock_PreReleaseRequest.cc @@ -33,7 +33,8 @@ namespace exclusive_lock { template <> struct ImageDispatch { - MOCK_METHOD2(set_require_lock, void(io::Direction, Context*)); + MOCK_METHOD3(set_require_lock, void(bool init_shutdown, io::Direction, + Context*)); MOCK_METHOD1(unset_require_lock, void(io::Direction)); }; @@ -106,22 +107,25 @@ public: } void expect_set_require_lock(MockImageDispatch &mock_image_dispatch, + bool init_shutdown, librbd::io::Direction direction, int r) { - EXPECT_CALL(mock_image_dispatch, set_require_lock(direction, _)) - .WillOnce(WithArg<1>(Invoke([r](Context* ctx) { ctx->complete(r); }))); + EXPECT_CALL(mock_image_dispatch, set_require_lock(init_shutdown, + direction, _)) + .WillOnce(WithArg<2>(Invoke([r](Context* ctx) { ctx->complete(r); }))); } void expect_set_require_lock(MockTestImageCtx &mock_image_ctx, - MockImageDispatch &mock_image_dispatch, int r) { + MockImageDispatch &mock_image_dispatch, + bool init_shutdown, int r) { expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING, ((mock_image_ctx.features & RBD_FEATURE_JOURNALING) != 0)); if (mock_image_ctx.clone_copy_on_read || (mock_image_ctx.features & RBD_FEATURE_JOURNALING) != 0) { - expect_set_require_lock(mock_image_dispatch, librbd::io::DIRECTION_BOTH, - r); + expect_set_require_lock(mock_image_dispatch, init_shutdown, + librbd::io::DIRECTION_BOTH, r); } else { - expect_set_require_lock(mock_image_dispatch, librbd::io::DIRECTION_WRITE, - r); + expect_set_require_lock(mock_image_dispatch, init_shutdown, + librbd::io::DIRECTION_WRITE, r); } } @@ -192,7 +196,7 @@ TEST_F(TestMockExclusiveLockPreReleaseRequest, Success) { expect_prepare_lock(mock_image_ctx); expect_cancel_op_requests(mock_image_ctx, 0); MockImageDispatch mock_image_dispatch; - expect_set_require_lock(mock_image_ctx, mock_image_dispatch, 0); + expect_set_require_lock(mock_image_ctx, mock_image_dispatch, false, 0); cache::MockImageCache mock_image_cache; mock_image_ctx.image_cache = &mock_image_cache; @@ -229,7 +233,7 @@ TEST_F(TestMockExclusiveLockPreReleaseRequest, SuccessJournalDisabled) { MockTestImageCtx mock_image_ctx(*ictx); MockImageDispatch mock_image_dispatch; - expect_set_require_lock(mock_image_ctx, mock_image_dispatch, 0); + expect_set_require_lock(mock_image_ctx, mock_image_dispatch, false, 0); expect_op_work_queue(mock_image_ctx); InSequence seq; @@ -267,7 +271,7 @@ TEST_F(TestMockExclusiveLockPreReleaseRequest, SuccessObjectMapDisabled) { MockTestImageCtx mock_image_ctx(*ictx); MockImageDispatch mock_image_dispatch; - expect_set_require_lock(mock_image_ctx, mock_image_dispatch, 0); + expect_set_require_lock(mock_image_ctx, mock_image_dispatch, true, 0); expect_op_work_queue(mock_image_ctx); InSequence seq; @@ -303,7 +307,8 @@ TEST_F(TestMockExclusiveLockPreReleaseRequest, Blocklisted) { expect_prepare_lock(mock_image_ctx); expect_cancel_op_requests(mock_image_ctx, 0); MockImageDispatch mock_image_dispatch; - expect_set_require_lock(mock_image_ctx, mock_image_dispatch, -EBLOCKLISTED); + expect_set_require_lock(mock_image_ctx, mock_image_dispatch, false, + -EBLOCKLISTED); cache::MockImageCache mock_image_cache; mock_image_ctx.image_cache = &mock_image_cache; @@ -344,7 +349,7 @@ TEST_F(TestMockExclusiveLockPreReleaseRequest, BlockWritesError) { InSequence seq; expect_cancel_op_requests(mock_image_ctx, 0); MockImageDispatch mock_image_dispatch; - expect_set_require_lock(mock_image_ctx, mock_image_dispatch, -EINVAL); + expect_set_require_lock(mock_image_ctx, mock_image_dispatch, true, -EINVAL); expect_unset_require_lock(mock_image_dispatch); C_SaferCond ctx; @@ -354,30 +359,5 @@ TEST_F(TestMockExclusiveLockPreReleaseRequest, BlockWritesError) { ASSERT_EQ(-EINVAL, ctx.wait()); } -TEST_F(TestMockExclusiveLockPreReleaseRequest, UnlockError) { - REQUIRE_FEATURE(RBD_FEATURE_EXCLUSIVE_LOCK); - - librbd::ImageCtx *ictx; - ASSERT_EQ(0, open_image(m_image_name, &ictx)); - - MockTestImageCtx mock_image_ctx(*ictx); - - expect_op_work_queue(mock_image_ctx); - - InSequence seq; - expect_cancel_op_requests(mock_image_ctx, 0); - MockImageDispatch mock_image_dispatch; - expect_set_require_lock(mock_image_ctx, mock_image_dispatch, 0); - expect_invalidate_cache(mock_image_ctx, 0); - - expect_flush_notifies(mock_image_ctx); - - C_SaferCond ctx; - MockPreReleaseRequest *req = MockPreReleaseRequest::create( - mock_image_ctx, &mock_image_dispatch, true, m_async_op_tracker, &ctx); - req->send(); - ASSERT_EQ(0, ctx.wait()); -} - } // namespace exclusive_lock } // namespace librbd diff --git a/src/test/librbd/image/test_mock_RefreshRequest.cc b/src/test/librbd/image/test_mock_RefreshRequest.cc index c9bcf4e014c9..2912237c1662 100644 --- a/src/test/librbd/image/test_mock_RefreshRequest.cc +++ b/src/test/librbd/image/test_mock_RefreshRequest.cc @@ -157,8 +157,8 @@ public: void expect_set_require_lock(MockExclusiveLock &mock_exclusive_lock, librbd::io::Direction direction) { - EXPECT_CALL(mock_exclusive_lock, set_require_lock(direction, _)) - .WillOnce(WithArg<1>(Invoke([](Context* ctx) { ctx->complete(0); }))); + EXPECT_CALL(mock_exclusive_lock, set_require_lock(true, direction, _)) + .WillOnce(WithArg<2>(Invoke([](Context* ctx) { ctx->complete(0); }))); } void expect_unset_require_lock(MockExclusiveLock &mock_exclusive_lock, diff --git a/src/test/librbd/mock/MockExclusiveLock.h b/src/test/librbd/mock/MockExclusiveLock.h index 6df6dbb1e2d3..db675abf0ef4 100644 --- a/src/test/librbd/mock/MockExclusiveLock.h +++ b/src/test/librbd/mock/MockExclusiveLock.h @@ -35,7 +35,8 @@ struct MockExclusiveLock { MOCK_METHOD0(accept_ops, bool()); MOCK_METHOD0(get_unlocked_op_error, int()); - MOCK_METHOD2(set_require_lock, void(io::Direction, Context*)); + MOCK_METHOD3(set_require_lock, void(bool init_shutdown, io::Direction, + Context*)); MOCK_METHOD1(unset_require_lock, void(io::Direction)); MOCK_METHOD1(start_op, Context*(int*)); diff --git a/src/test/librbd/test_mock_ExclusiveLock.cc b/src/test/librbd/test_mock_ExclusiveLock.cc index 3fb512e8fbc7..33f7d975cd39 100644 --- a/src/test/librbd/test_mock_ExclusiveLock.cc +++ b/src/test/librbd/test_mock_ExclusiveLock.cc @@ -131,7 +131,7 @@ struct ImageDispatch return io::IMAGE_DISPATCH_LAYER_EXCLUSIVE_LOCK; } - MOCK_METHOD2(set_require_lock, void(io::Direction, Context*)); + MOCK_METHOD3(set_require_lock, void(bool, io::Direction, Context*)); MOCK_METHOD1(unset_require_lock, void(io::Direction)); }; @@ -254,19 +254,23 @@ public: } void expect_set_require_lock(MockImageDispatch &mock_image_dispatch, - io::Direction direction) { - EXPECT_CALL(mock_image_dispatch, set_require_lock(direction, _)) - .WillOnce(WithArg<1>(Invoke([](Context* ctx) { ctx->complete(0); }))); + bool init_shutdown, io::Direction direction) { + EXPECT_CALL(mock_image_dispatch, set_require_lock(init_shutdown, + direction, _)) + .WillOnce(WithArg<2>(Invoke([](Context* ctx) { ctx->complete(0); }))); } void expect_set_require_lock(MockExclusiveLockImageCtx &mock_image_ctx, - MockImageDispatch &mock_image_dispatch) { + MockImageDispatch &mock_image_dispatch, + bool init_shutdown) { if (mock_image_ctx.clone_copy_on_read || (mock_image_ctx.features & RBD_FEATURE_JOURNALING) != 0 || is_rbd_rwl_enabled(mock_image_ctx.cct)) { - expect_set_require_lock(mock_image_dispatch, io::DIRECTION_BOTH); + expect_set_require_lock(mock_image_dispatch, init_shutdown, + io::DIRECTION_BOTH); } else { - expect_set_require_lock(mock_image_dispatch, io::DIRECTION_WRITE); + expect_set_require_lock(mock_image_dispatch, init_shutdown, + io::DIRECTION_WRITE); } } @@ -433,7 +437,7 @@ TEST_F(TestMockExclusiveLock, StateTransitions) { MockImageDispatch mock_image_dispatch; expect_block_writes(mock_image_ctx, mock_image_dispatch); expect_register_dispatch(mock_image_ctx); - expect_set_require_lock(mock_image_ctx, mock_image_dispatch); + expect_set_require_lock(mock_image_ctx, mock_image_dispatch, true); expect_unblock_writes(mock_image_ctx); expect_set_state_unlocked(exclusive_lock); ASSERT_EQ(0, when_init(mock_image_ctx, exclusive_lock)); @@ -504,7 +508,7 @@ TEST_F(TestMockExclusiveLock, TryLockAlreadyLocked) { MockImageDispatch mock_image_dispatch; expect_block_writes(mock_image_ctx, mock_image_dispatch); expect_register_dispatch(mock_image_ctx); - expect_set_require_lock(mock_image_ctx, mock_image_dispatch); + expect_set_require_lock(mock_image_ctx, mock_image_dispatch, true); expect_unblock_writes(mock_image_ctx); expect_set_state_unlocked(exclusive_lock); ASSERT_EQ(0, when_init(mock_image_ctx, exclusive_lock)); @@ -539,7 +543,7 @@ TEST_F(TestMockExclusiveLock, TryLockError) { MockImageDispatch mock_image_dispatch; expect_block_writes(mock_image_ctx, mock_image_dispatch); expect_register_dispatch(mock_image_ctx); - expect_set_require_lock(mock_image_ctx, mock_image_dispatch); + expect_set_require_lock(mock_image_ctx, mock_image_dispatch, true); expect_unblock_writes(mock_image_ctx); expect_set_state_unlocked(exclusive_lock); ASSERT_EQ(0, when_init(mock_image_ctx, exclusive_lock)); @@ -574,7 +578,7 @@ TEST_F(TestMockExclusiveLock, AcquireLockAlreadyLocked) { MockImageDispatch mock_image_dispatch; expect_block_writes(mock_image_ctx, mock_image_dispatch); expect_register_dispatch(mock_image_ctx); - expect_set_require_lock(mock_image_ctx, mock_image_dispatch); + expect_set_require_lock(mock_image_ctx, mock_image_dispatch, true); expect_unblock_writes(mock_image_ctx); expect_set_state_unlocked(exclusive_lock); ASSERT_EQ(0, when_init(mock_image_ctx, exclusive_lock)); @@ -612,7 +616,7 @@ TEST_F(TestMockExclusiveLock, AcquireLockBusy) { MockImageDispatch mock_image_dispatch; expect_block_writes(mock_image_ctx, mock_image_dispatch); expect_register_dispatch(mock_image_ctx); - expect_set_require_lock(mock_image_ctx, mock_image_dispatch); + expect_set_require_lock(mock_image_ctx, mock_image_dispatch, true); expect_unblock_writes(mock_image_ctx); expect_set_state_unlocked(exclusive_lock); ASSERT_EQ(0, when_init(mock_image_ctx, exclusive_lock)); @@ -650,7 +654,7 @@ TEST_F(TestMockExclusiveLock, AcquireLockError) { MockImageDispatch mock_image_dispatch; expect_block_writes(mock_image_ctx, mock_image_dispatch); expect_register_dispatch(mock_image_ctx); - expect_set_require_lock(mock_image_ctx, mock_image_dispatch); + expect_set_require_lock(mock_image_ctx, mock_image_dispatch, true); expect_unblock_writes(mock_image_ctx); expect_set_state_unlocked(exclusive_lock); ASSERT_EQ(0, when_init(mock_image_ctx, exclusive_lock)); @@ -686,7 +690,7 @@ TEST_F(TestMockExclusiveLock, PostAcquireLockError) { MockImageDispatch mock_image_dispatch; expect_block_writes(mock_image_ctx, mock_image_dispatch); expect_register_dispatch(mock_image_ctx); - expect_set_require_lock(mock_image_ctx, mock_image_dispatch); + expect_set_require_lock(mock_image_ctx, mock_image_dispatch, true); expect_unblock_writes(mock_image_ctx); expect_set_state_unlocked(exclusive_lock); ASSERT_EQ(0, when_init(mock_image_ctx, exclusive_lock)); @@ -722,7 +726,7 @@ TEST_F(TestMockExclusiveLock, PreReleaseLockError) { MockImageDispatch mock_image_dispatch; expect_block_writes(mock_image_ctx, mock_image_dispatch); expect_register_dispatch(mock_image_ctx); - expect_set_require_lock(mock_image_ctx, mock_image_dispatch); + expect_set_require_lock(mock_image_ctx, mock_image_dispatch, true); expect_unblock_writes(mock_image_ctx); expect_set_state_unlocked(exclusive_lock); ASSERT_EQ(0, when_init(mock_image_ctx, exclusive_lock)); @@ -756,7 +760,7 @@ TEST_F(TestMockExclusiveLock, ReacquireLock) { MockImageDispatch mock_image_dispatch; expect_block_writes(mock_image_ctx, mock_image_dispatch); expect_register_dispatch(mock_image_ctx); - expect_set_require_lock(mock_image_ctx, mock_image_dispatch); + expect_set_require_lock(mock_image_ctx, mock_image_dispatch, true); expect_unblock_writes(mock_image_ctx); expect_set_state_unlocked(exclusive_lock); ASSERT_EQ(0, when_init(mock_image_ctx, exclusive_lock)); @@ -813,7 +817,7 @@ TEST_F(TestMockExclusiveLock, BlockRequests) { MockImageDispatch mock_image_dispatch; expect_block_writes(mock_image_ctx, mock_image_dispatch); expect_register_dispatch(mock_image_ctx); - expect_set_require_lock(mock_image_ctx, mock_image_dispatch); + expect_set_require_lock(mock_image_ctx, mock_image_dispatch, true); expect_unblock_writes(mock_image_ctx); expect_set_state_unlocked(exclusive_lock); ASSERT_EQ(0, when_init(mock_image_ctx, exclusive_lock));