From 7fec876843f76ebcc71c7ee0ab84c7792baf3a8c 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 (cherry picked from commit a30276eadbb29d36baf5532327786da720856db8) --- 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 33020403b5a17..2c405d74dc1eb 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 dc6a709f05ec3..93c53a0fe8b2c 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 2b38bf3b8d40c..752ef82c72fbc 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 dd4f153a5fbc0..2dd72bff76cc2 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