[PATCH] block: Make blk_mq_delay_kick_requeue_list() rerun the queue at a quiet time

[PATCH] block: Make blk_mq_delay_kick_requeue_list() rerun the queue at a quiet time

[Bart Van Assche]

The blk_mq_delay_kick_requeue_list() function is used by the device
mapper and only by the device mapper to rerun the queue and requeue
list after a delay. This function is called once per request that
gets requeued. Modify this function such that the queue is run once
per path change event instead of once per request that is requeued.

Fixes: commit 2849450ad39d ("blk-mq: introduce blk_mq_delay_kick_requeue_list()")
Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: Laurence Oberman <loberman@redhat.com>
Cc: <stable@vger.kernel.org>
---
 block/blk-mq.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 041f7b7fa0d6..8bfea36e92f9 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -679,8 +679,8 @@ EXPORT_SYMBOL(blk_mq_kick_requeue_list);
 void blk_mq_delay_kick_requeue_list(struct request_queue *q,
 				    unsigned long msecs)
 {
-	kblockd_schedule_delayed_work(&q->requeue_work,
-				      msecs_to_jiffies(msecs));
+	kblockd_mod_delayed_work_on(WORK_CPU_UNBOUND, &q->requeue_work,
+				    msecs_to_jiffies(msecs));
 }
 EXPORT_SYMBOL(blk_mq_delay_kick_requeue_list);
 
-- 
2.13.3

[PATCH] block: Make blk_mq_delay_kick_requeue_list() rerun the queue at a quiet time

[Bart Van Assche]

The blk_mq_delay_kick_requeue_list() function is used by the device
mapper and only by the device mapper to rerun the queue and requeue
list after a delay. This function is called once per request that
gets requeued. Modify this function such that the queue is run once
per path change event instead of once per request that is requeued.

Fixes: commit 2849450ad39d ("blk-mq: introduce blk_mq_delay_kick_requeue_list()")
Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: Laurence Oberman <loberman@redhat.com>
Cc: <stable@vger.kernel.org>
---
 block/blk-mq.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 041f7b7fa0d6..8bfea36e92f9 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -679,8 +679,8 @@ EXPORT_SYMBOL(blk_mq_kick_requeue_list);
 void blk_mq_delay_kick_requeue_list(struct request_queue *q,
 				    unsigned long msecs)
 {
-	kblockd_schedule_delayed_work(&q->requeue_work,
-				      msecs_to_jiffies(msecs));
+	kblockd_mod_delayed_work_on(WORK_CPU_UNBOUND, &q->requeue_work,
+				    msecs_to_jiffies(msecs));
 }
 EXPORT_SYMBOL(blk_mq_delay_kick_requeue_list);
 
-- 
2.13.3

Re: [PATCH] block: Make blk_mq_delay_kick_requeue_list() rerun the queue at a quiet time

[Jens Axboe]

On 08/09/2017 12:28 PM, Bart Van Assche wrote:
> The blk_mq_delay_kick_requeue_list() function is used by the device
> mapper and only by the device mapper to rerun the queue and requeue
> list after a delay. This function is called once per request that
> gets requeued. Modify this function such that the queue is run once
> per path change event instead of once per request that is requeued.

Looks good to me, will add for this series. Thanks Bart.

-- 
Jens Axboe

Re: [PATCH] block: Make blk_mq_delay_kick_requeue_list() rerun the queue at a quiet time

[Christoph Hellwig]

On Wed, Aug 09, 2017 at 11:28:06AM -0700, Bart Van Assche wrote:
> The blk_mq_delay_kick_requeue_list() function is used by the device
> mapper and only by the device mapper to rerun the queue and requeue
> list after a delay. This function is called once per request that
> gets requeued. Modify this function such that the queue is run once
> per path change event instead of once per request that is requeued.

Is there a chance we could look into killing this function and moving
dm to run it every time we requeue a request like all the other drivers?

Re: [PATCH] block: Make blk_mq_delay_kick_requeue_list() rerun the queue at a quiet time

[Bart Van Assche]

On Thu, 2017-08-10 at 09:14 +0200, Christoph Hellwig wrote:
> On Wed, Aug 09, 2017 at 11:28:06AM -0700, Bart Van Assche wrote:
> > The blk_mq_delay_kick_requeue_list() function is used by the device
> > mapper and only by the device mapper to rerun the queue and requeue
> > list after a delay. This function is called once per request that
> > gets requeued. Modify this function such that the queue is run once
> > per path change event instead of once per request that is requeued.
> 
> Is there a chance we could look into killing this function and moving
> dm to run it every time we requeue a request like all the other drivers?

Hello Christoph,

That's an interesting question. I think it's not possible to remove this
function without adding more infrastructure to the block layer. As an
example, if the .lld_busy_fn() request queue callback function reports that
a request queue is busy the only way for the dm-mpath driver to detect that
an underlying request queue is no longer busy is by calling that function
again after a certain time (see also multipath_busy() in dm-mpath.c). In
other words, if we want to get rid of blk_mq_delay_kick_requeue_list() then
notification mechanisms will have to be added for every type of state
information that can cause the state of a multipath from "not ready" into
"ready".

Bart.

Re: [PATCH] block: Make blk_mq_delay_kick_requeue_list() rerun the queue at a quiet time

[Bart Van Assche]

T24gVGh1LCAyMDE3LTA4LTEwIGF0IDA5OjE0ICswMjAwLCBDaHJpc3RvcGggSGVsbHdpZyB3cm90
ZToNCj4gT24gV2VkLCBBdWcgMDksIDIwMTcgYXQgMTE6Mjg6MDZBTSAtMDcwMCwgQmFydCBWYW4g
QXNzY2hlIHdyb3RlOg0KPiA+IFRoZSBibGtfbXFfZGVsYXlfa2lja19yZXF1ZXVlX2xpc3QoKSBm
dW5jdGlvbiBpcyB1c2VkIGJ5IHRoZSBkZXZpY2UNCj4gPiBtYXBwZXIgYW5kIG9ubHkgYnkgdGhl
IGRldmljZSBtYXBwZXIgdG8gcmVydW4gdGhlIHF1ZXVlIGFuZCByZXF1ZXVlDQo+ID4gbGlzdCBh
ZnRlciBhIGRlbGF5LiBUaGlzIGZ1bmN0aW9uIGlzIGNhbGxlZCBvbmNlIHBlciByZXF1ZXN0IHRo
YXQNCj4gPiBnZXRzIHJlcXVldWVkLiBNb2RpZnkgdGhpcyBmdW5jdGlvbiBzdWNoIHRoYXQgdGhl
IHF1ZXVlIGlzIHJ1biBvbmNlDQo+ID4gcGVyIHBhdGggY2hhbmdlIGV2ZW50IGluc3RlYWQgb2Yg
b25jZSBwZXIgcmVxdWVzdCB0aGF0IGlzIHJlcXVldWVkLg0KPiANCj4gSXMgdGhlcmUgYSBjaGFu
Y2Ugd2UgY291bGQgbG9vayBpbnRvIGtpbGxpbmcgdGhpcyBmdW5jdGlvbiBhbmQgbW92aW5nDQo+
IGRtIHRvIHJ1biBpdCBldmVyeSB0aW1lIHdlIHJlcXVldWUgYSByZXF1ZXN0IGxpa2UgYWxsIHRo
ZSBvdGhlciBkcml2ZXJzPw0KDQpIZWxsbyBDaHJpc3RvcGgsDQoNClRoYXQncyBhbiBpbnRlcmVz
dGluZyBxdWVzdGlvbi4gSSB0aGluayBpdCdzIG5vdCBwb3NzaWJsZSB0byByZW1vdmUgdGhpcw0K
ZnVuY3Rpb24gd2l0aG91dCBhZGRpbmcgbW9yZSBpbmZyYXN0cnVjdHVyZSB0byB0aGUgYmxvY2sg
bGF5ZXIuIEFzIGFuDQpleGFtcGxlLCBpZiB0aGUgLmxsZF9idXN5X2ZuKCkgcmVxdWVzdCBxdWV1
ZSBjYWxsYmFjayBmdW5jdGlvbiByZXBvcnRzIHRoYXQNCmEgcmVxdWVzdCBxdWV1ZSBpcyBidXN5
IHRoZSBvbmx5IHdheSBmb3IgdGhlIGRtLW1wYXRoIGRyaXZlciB0byBkZXRlY3QgdGhhdA0KYW4g
dW5kZXJseWluZyByZXF1ZXN0IHF1ZXVlIGlzIG5vIGxvbmdlciBidXN5IGlzIGJ5IGNhbGxpbmcg
dGhhdCBmdW5jdGlvbg0KYWdhaW4gYWZ0ZXIgYSBjZXJ0YWluIHRpbWUgKHNlZSBhbHNvIG11bHRp
cGF0aF9idXN5KCkgaW4gZG0tbXBhdGguYykuIEluDQpvdGhlciB3b3JkcywgaWYgd2Ugd2FudCB0
byBnZXQgcmlkIG9mIGJsa19tcV9kZWxheV9raWNrX3JlcXVldWVfbGlzdCgpIHRoZW4NCm5vdGlm
aWNhdGlvbiBtZWNoYW5pc21zIHdpbGwgaGF2ZSB0byBiZSBhZGRlZCBmb3IgZXZlcnkgdHlwZSBv
ZiBzdGF0ZQ0KaW5mb3JtYXRpb24gdGhhdCBjYW4gY2F1c2UgdGhlIHN0YXRlIG9mIGEgbXVsdGlw
YXRoIGZyb20gIm5vdCByZWFkeSIgaW50bw0KInJlYWR5Ii4NCg0KQmFydC4=