From a30276eadbb29d36baf5532327786da720856db8 Mon Sep 17 00:00:00 2001 From: Mykola Golub Date: Thu, 14 Mar 2019 18:46:23 +0000 Subject: [PATCH] librbd: make flush be queued by QOS throttler So when it is eventually processed we sure there is no pending requests in the throttler queue it needs to wait to complete. Fixes: https://tracker.ceph.com/issues/38706 Signed-off-by: Mykola Golub --- src/librbd/io/ImageDispatchSpec.cc | 32 ++++++++++--------- src/librbd/io/ImageDispatchSpec.h | 2 +- src/librbd/io/ImageRequestWQ.cc | 5 ++- .../librbd/io/test_mock_ImageRequestWQ.cc | 15 ++++++--- 4 files changed, 30 insertions(+), 24 deletions(-) diff --git a/src/librbd/io/ImageDispatchSpec.cc b/src/librbd/io/ImageDispatchSpec.cc index 33020403b5a..2c405d74dc1 100644 --- a/src/librbd/io/ImageDispatchSpec.cc +++ b/src/librbd/io/ImageDispatchSpec.cc @@ -76,36 +76,37 @@ struct ImageDispatchSpec::TokenRequestedVisitor : public boost::static_visitor { ImageDispatchSpec* spec; uint64_t flag; + uint64_t *tokens; - TokenRequestedVisitor(ImageDispatchSpec* spec, uint64_t _flag) - : spec(spec), flag(_flag) { + TokenRequestedVisitor(ImageDispatchSpec* spec, uint64_t _flag, + uint64_t *tokens) + : spec(spec), flag(_flag), tokens(tokens) { } uint64_t operator()(const Read&) const { if (flag & RBD_QOS_WRITE_MASK) { - return 0; + *tokens = 0; + return false; } - if (flag & RBD_QOS_BPS_MASK) { - return spec->extents_length(); - } - return 1; + *tokens = (flag & RBD_QOS_BPS_MASK) ? spec->extents_length() : 1; + return true; } uint64_t operator()(const Flush&) const { - return 0; + *tokens = 0; + return true; } template uint64_t operator()(const T&) const { if (flag & RBD_QOS_READ_MASK) { - return 0; + *tokens = 0; + return false; } - if (flag & RBD_QOS_BPS_MASK) { - return spec->extents_length(); - } - return 1; + *tokens = (flag & RBD_QOS_BPS_MASK) ? spec->extents_length() : 1; + return true; } }; @@ -137,8 +138,9 @@ bool ImageDispatchSpec::is_write_op() const { } template -uint64_t ImageDispatchSpec::tokens_requested(uint64_t flag) { - return boost::apply_visitor(TokenRequestedVisitor{this, flag}, m_request); +bool ImageDispatchSpec::tokens_requested(uint64_t flag, uint64_t *tokens) { + return boost::apply_visitor(TokenRequestedVisitor{this, flag, tokens}, + m_request); } template diff --git a/src/librbd/io/ImageDispatchSpec.h b/src/librbd/io/ImageDispatchSpec.h index dc6a709f05e..93c53a0fe8b 100644 --- a/src/librbd/io/ImageDispatchSpec.h +++ b/src/librbd/io/ImageDispatchSpec.h @@ -129,7 +129,7 @@ public: void start_op(); - uint64_t tokens_requested(uint64_t flag); + bool tokens_requested(uint64_t flag, uint64_t *tokens); bool was_throttled(uint64_t flag) { return m_throttled_flag & flag; diff --git a/src/librbd/io/ImageRequestWQ.cc b/src/librbd/io/ImageRequestWQ.cc index 2b38bf3b8d4..752ef82c72f 100644 --- a/src/librbd/io/ImageRequestWQ.cc +++ b/src/librbd/io/ImageRequestWQ.cc @@ -685,9 +685,8 @@ bool ImageRequestWQ::needs_throttle(ImageDispatchSpec *item) { } throttle = t.second; - tokens = item->tokens_requested(flag); - - if (throttle->get, ImageDispatchSpec, + if (item->tokens_requested(flag, &tokens) && + throttle->get, ImageDispatchSpec, &ImageRequestWQ::handle_throttle_ready>( tokens, this, item, flag)) { blocked = true; diff --git a/src/test/librbd/io/test_mock_ImageRequestWQ.cc b/src/test/librbd/io/test_mock_ImageRequestWQ.cc index dd4f153a5fb..2dd72bff76c 100644 --- a/src/test/librbd/io/test_mock_ImageRequestWQ.cc +++ b/src/test/librbd/io/test_mock_ImageRequestWQ.cc @@ -65,7 +65,7 @@ struct ImageDispatchSpec { MOCK_CONST_METHOD1(was_throttled, bool(uint64_t)); MOCK_CONST_METHOD0(were_all_throttled, bool()); MOCK_CONST_METHOD1(set_throttled, void(uint64_t)); - MOCK_CONST_METHOD1(tokens_requested, uint64_t(uint64_t)); + MOCK_CONST_METHOD2(tokens_requested, bool(uint64_t, uint64_t *)); ImageDispatchSpec() { s_instance = this; @@ -236,8 +236,13 @@ struct TestMockIoImageRequestWQ : public TestMockFixture { EXPECT_CALL(mock_image_request, was_throttled(_)).Times(6).WillRepeatedly(Return(value)); } - void expect_tokens_requested(MockImageDispatchSpec &mock_image_request, uint64_t value) { - EXPECT_CALL(mock_image_request, tokens_requested(_)).WillOnce(Return(value)); + void expect_tokens_requested(MockImageDispatchSpec &mock_image_request, + uint64_t tokens, bool r) { + EXPECT_CALL(mock_image_request, tokens_requested(_, _)) + .WillOnce(WithArg<1>(Invoke([tokens, r](uint64_t *t) { + *t = tokens; + return r; + }))); } void expect_all_throttled(MockImageDispatchSpec &mock_image_request, bool value) { @@ -417,7 +422,7 @@ TEST_F(TestMockIoImageRequestWQ, BPSQosNoBurst) { mock_image_request_wq.apply_qos_limit(RBD_QOS_BPS_THROTTLE, 1, 0); expect_front(mock_image_request_wq, &mock_queued_image_request); - expect_tokens_requested(mock_queued_image_request, 2); + expect_tokens_requested(mock_queued_image_request, 2, true); expect_dequeue(mock_image_request_wq, &mock_queued_image_request); expect_all_throttled(mock_queued_image_request, true); expect_requeue(mock_image_request_wq); @@ -441,7 +446,7 @@ TEST_F(TestMockIoImageRequestWQ, BPSQosWithBurst) { mock_image_request_wq.apply_qos_limit(RBD_QOS_BPS_THROTTLE, 1, 1); expect_front(mock_image_request_wq, &mock_queued_image_request); - expect_tokens_requested(mock_queued_image_request, 2); + expect_tokens_requested(mock_queued_image_request, 2, true); expect_dequeue(mock_image_request_wq, &mock_queued_image_request); expect_all_throttled(mock_queued_image_request, true); expect_requeue(mock_image_request_wq); -- 2.39.5