[PATCH 0/2] blk-mq: kyber: fix domain token leak during requeue

[PATCH 0/2] blk-mq: kyber: fix domain token leak during requeue

[Ming Lei]

Hi,

This two patch fixes domain token leak on kyber scheduler, and actually
fixes one IO hang issue.

Thanks,

Ming Lei (2):
  blk-mq: don't call io sched's .requeue_request when requeueing rq to
    ->dispatch
  block: kyber: fix domain token leak during requeue

 block/blk-mq.c        | 4 +++-
 block/kyber-iosched.c | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

-- 
2.9.5

[PATCH 1/2] blk-mq: don't call io sched's .requeue_request when requeueing rq to ->dispatch

[Ming Lei]

__blk_mq_requeue_request() covers two cases:

- one is that the requeued request is added to hctx->dispatch, such as
blk_mq_dispatch_rq_list()

- another case is that the request is requeued to io scheduler, such as
blk_mq_requeue_request().

We should call io sched's .requeue_request callback only for the 2nd
case.

Cc: Paolo Valente <paolo.valente@linaro.org>
Cc: Omar Sandoval <osandov@fb.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-mq.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 357492712b0e..16e83e6df404 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -712,7 +712,6 @@ static void __blk_mq_requeue_request(struct request *rq)
 
 	trace_block_rq_requeue(q, rq);
 	wbt_requeue(q->rq_wb, &rq->issue_stat);
-	blk_mq_sched_requeue_request(rq);
 
 	if (blk_mq_rq_state(rq) != MQ_RQ_IDLE) {
 		blk_mq_rq_update_state(rq, MQ_RQ_IDLE);
@@ -725,6 +724,9 @@ void blk_mq_requeue_request(struct request *rq, bool kick_requeue_list)
 {
 	__blk_mq_requeue_request(rq);
 
+	/* this request will be re-inserted to io scheduler queue */
+	blk_mq_sched_requeue_request(rq);
+
 	BUG_ON(blk_queued_rq(rq));
 	blk_mq_add_to_requeue_list(rq, true, kick_requeue_list);
 }
-- 
2.9.5

Re: [PATCH 1/2] blk-mq: don't call io sched's .requeue_request when requeueing rq to ->dispatch

[Bart Van Assche]

T24gRnJpLCAyMDE4LTAyLTIzIGF0IDIzOjM2ICswODAwLCBNaW5nIExlaSB3cm90ZToNCj4gX19i
bGtfbXFfcmVxdWV1ZV9yZXF1ZXN0KCkgY292ZXJzIHR3byBjYXNlczoNCj4gDQo+IC0gb25lIGlz
IHRoYXQgdGhlIHJlcXVldWVkIHJlcXVlc3QgaXMgYWRkZWQgdG8gaGN0eC0+ZGlzcGF0Y2gsIHN1
Y2ggYXMNCj4gYmxrX21xX2Rpc3BhdGNoX3JxX2xpc3QoKQ0KPiANCj4gLSBhbm90aGVyIGNhc2Ug
aXMgdGhhdCB0aGUgcmVxdWVzdCBpcyByZXF1ZXVlZCB0byBpbyBzY2hlZHVsZXIsIHN1Y2ggYXMN
Cj4gYmxrX21xX3JlcXVldWVfcmVxdWVzdCgpLg0KPiANCj4gV2Ugc2hvdWxkIGNhbGwgaW8gc2No
ZWQncyAucmVxdWV1ZV9yZXF1ZXN0IGNhbGxiYWNrIG9ubHkgZm9yIHRoZSAybmQNCj4gY2FzZS4N
Cg0KSSB0aGluayBpdCB3b3VsZCBoYXZlIGJlZW4gbW9yZSBjbGVhciBpZiBpdCB3b3VsZCBoYXZl
IGJlZW4gbWVudGlvbmVkIHRoYXQNCmJsa19tcV9zY2hlZF9yZXF1ZXVlX3JlcXVlc3QoKSBtdXN0
IG9ubHkgYmUgY2FsbGVkIGZvciByZXF1ZXN0cyB0aGF0IGhhdmUNCmJlZW4gc3RhcnRlZC4gQW55
d2F5LCBpZiB5b3UgYWRkIHRoZSBmb2xsb3dpbmcgdG8gdGhpcyBwYXRjaDoNCg0KRml4ZXM6IGJk
MTY2ZWYxODNjMiAoImJsay1tcS1zY2hlZDogYWRkIGZyYW1ld29yayBmb3IgTVEgY2FwYWJsZSBJ
TyBzY2hlZHVsZXJzIikNCkNjOiBzdGFibGVAdmdlci5rZXJuZWwub3JnDQoNCnRoZW4geW91IGNh
biBhbHNvIGFkZDoNCg0KUmV2aWV3ZWQtYnk6IEJhcnQgVmFuIEFzc2NoZSA8YmFydC52YW5hc3Nj
aGVAd2RjLmNvbT4NCg0KDQoNCg==

Re: [PATCH 1/2] blk-mq: don't call io sched's .requeue_request when requeueing rq to ->dispatch

[Paolo Valente]

> Il giorno 23 feb 2018, alle ore 16:36, Ming Lei <ming.lei@redhat.com> =
ha scritto:
>=20
> __blk_mq_requeue_request() covers two cases:
>=20
> - one is that the requeued request is added to hctx->dispatch, such as
> blk_mq_dispatch_rq_list()
>=20
> - another case is that the request is requeued to io scheduler, such =
as
> blk_mq_requeue_request().
>=20
> We should call io sched's .requeue_request callback only for the 2nd
> case.
>=20

Acked-by: Paolo Valente <paolo.valente@linaro.org>

> Cc: Paolo Valente <paolo.valente@linaro.org>
> Cc: Omar Sandoval <osandov@fb.com>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
> block/blk-mq.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>=20
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 357492712b0e..16e83e6df404 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -712,7 +712,6 @@ static void __blk_mq_requeue_request(struct =
request *rq)
>=20
> 	trace_block_rq_requeue(q, rq);
> 	wbt_requeue(q->rq_wb, &rq->issue_stat);
> -	blk_mq_sched_requeue_request(rq);
>=20
> 	if (blk_mq_rq_state(rq) !=3D MQ_RQ_IDLE) {
> 		blk_mq_rq_update_state(rq, MQ_RQ_IDLE);
> @@ -725,6 +724,9 @@ void blk_mq_requeue_request(struct request *rq, =
bool kick_requeue_list)
> {
> 	__blk_mq_requeue_request(rq);
>=20
> +	/* this request will be re-inserted to io scheduler queue */
> +	blk_mq_sched_requeue_request(rq);
> +
> 	BUG_ON(blk_queued_rq(rq));
> 	blk_mq_add_to_requeue_list(rq, true, kick_requeue_list);
> }
> --=20
> 2.9.5
>=20

[PATCH 2/2] block: kyber: fix domain token leak during requeue

[Ming Lei]

When requeuing request, the domain token should have been freed
before re-inserting the request to io scheduler. Otherwise, the
assigned domain token will be leaked, and IO hang can be caused.

Cc: Paolo Valente <paolo.valente@linaro.org>
Cc: Omar Sandoval <osandov@fb.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/kyber-iosched.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/kyber-iosched.c b/block/kyber-iosched.c
index f95c60774ce8..0d6d25e32e1f 100644
--- a/block/kyber-iosched.c
+++ b/block/kyber-iosched.c
@@ -833,6 +833,7 @@ static struct elevator_type kyber_sched = {
 		.limit_depth = kyber_limit_depth,
 		.prepare_request = kyber_prepare_request,
 		.finish_request = kyber_finish_request,
+		.requeue_request = kyber_finish_request,
 		.completed_request = kyber_completed_request,
 		.dispatch_request = kyber_dispatch_request,
 		.has_work = kyber_has_work,
-- 
2.9.5

Re: [PATCH 2/2] block: kyber: fix domain token leak during requeue

[Bart Van Assche]

T24gRnJpLCAyMDE4LTAyLTIzIGF0IDIzOjM2ICswODAwLCBNaW5nIExlaSB3cm90ZToNCj4gV2hl
biByZXF1ZXVpbmcgcmVxdWVzdCwgdGhlIGRvbWFpbiB0b2tlbiBzaG91bGQgaGF2ZSBiZWVuIGZy
ZWVkDQo+IGJlZm9yZSByZS1pbnNlcnRpbmcgdGhlIHJlcXVlc3QgdG8gaW8gc2NoZWR1bGVyLiBP
dGhlcndpc2UsIHRoZQ0KPiBhc3NpZ25lZCBkb21haW4gdG9rZW4gd2lsbCBiZSBsZWFrZWQsIGFu
ZCBJTyBoYW5nIGNhbiBiZSBjYXVzZWQuDQoNClBsZWFzZSBjb25zaWRlciB0byBhZGQgYSAiQ2M6
IHN0YWJsZSIgdGFnIHRvIHRoaXMgcGF0Y2guDQoNCkFueXdheToNCg0KUmV2aWV3ZWQtYnk6IEJh
cnQgVmFuIEFzc2NoZSA8YmFydC52YW5hc3NjaGVAd2RjLmNvbT4NCg0KDQo=

Re: [PATCH 0/2] blk-mq: kyber: fix domain token leak during requeue

[Jens Axboe]

On Fri, Feb 23 2018, Ming Lei wrote:
> Hi,
> 
> This two patch fixes domain token leak on kyber scheduler, and actually
> fixes one IO hang issue.
> 
> Thanks,
> 
> Ming Lei (2):
>   blk-mq: don't call io sched's .requeue_request when requeueing rq to
>     ->dispatch
>   block: kyber: fix domain token leak during requeue

Applied, thanks.

-- 
Jens Axboe

Re: [PATCH 0/2] blk-mq: kyber: fix domain token leak during requeue

[Omar Sandoval]

On Fri, Feb 23, 2018 at 11:36:55PM +0800, Ming Lei wrote:
> Hi,
> 
> This two patch fixes domain token leak on kyber scheduler, and actually
> fixes one IO hang issue.
> 
> Thanks,
> 
> Ming Lei (2):
>   blk-mq: don't call io sched's .requeue_request when requeueing rq to
>     ->dispatch
>   block: kyber: fix domain token leak during requeue
> 
>  block/blk-mq.c        | 4 +++-
>  block/kyber-iosched.c | 1 +
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> -- 
> 2.9.5
> 

I didn't get to add my reviewed-by before Jens queued this up, but
thanks for fixing this, Ming.