blk-mq: avoid extending delays of active hctx from blk_mq_delay_run_hw_queues
PATCH 1/1 blk-mq: avoid extending delays of active hctx from blk_mq_delay_run_hw_queues
When blk_mq_delay_run_hw_queues sets an hctx to run in the future, it can reset the delay length for an already pending delayed work run_work. This creates a scenario where multiple hctx may have their queues set to run, but if one runs first and finds nothing to do, it can reset the delay of another hctx and stall the other hctx's ability to run requests.
To avoid this I/O stall when an hctx's run_work is already pending, leave it untouched to run at its current designated time rather than extending its delay. The work will still run which keeps closed the race calling blk_mq_delay_run_hw_queues is needed for while also avoiding the I/O stall.
--- block/blk-mq.c | 8 ++++++++ 1 file changed, 8 insertions(+)
block/blk-mq.c +8 -0
@@ -2177,6 +2177,14 @@ void blk_mq_delay_run_hw_queues(struct request_queue *q, unsigned long msecs)queue_for_each_hw_ctx (q, hctx, i) {if (blk_mq_hctx_stopped (hctx))continue ; +/* +* If there is already a run_work pending, leave the +* pending delay untouched. Otherwise, a hctx can stall +* if another hctx is re-delaying the other's work +* before the work executes. +*/ +if (delayed_work_pending (&hctx->run_work)) +continue ;/* * Dispatch from this hctx either if there's no hctx preferred * by IO scheduler or if it has requests that bypass the
Quoted 43 lines · David Jeffery
When
blk_mq_delay_run_hw_queuessets an hctx to run in the future, it can
reset the delay length for an already pending delayed workrun_work. This
creates a scenario where multiple hctx may have their queues set to run,
but if one runs first and finds nothing to do, it can reset the delay of
another hctx and stall the other hctx's ability to run requests.To avoid this I/O stall when an hctx's
run_workis already pending, leave it untouched to run at its current designated time rather than extending its delay. The work will still run which keeps closed the race
callingblk_mq_delay_run_hw_queuesis needed for while also avoiding the
I/O stall.Signed-off-by David Jeffery <[email protected]>--- block/blk-mq.c | 8 ++++++++ 1 file changed, 8 insertions(+)block/blk-mq.c +8
-0@@ -2177,6 +2177,14 @@ void blk_mq_delay_run_hw_queues(struct request_queue *q, unsigned long msecs) queue_for_each_hw_ctx(q, hctx, i) { if (blk_mq_hctx_stopped(hctx)) continue; +/* +* If there is already a run_work pending, leave the +* pending delay untouched. Otherwise, a hctx can stall +* if another hctx is re-delaying the other's work +* before the work executes. +*/ +if (delayed_work_pending (&hctx->run_work)) +continue ;/* * Dispatch from this hctx either if there's no hctx
Quoted 4 lines
preferred
* by IO scheduler or if it has requests that bypass
the
Ming is aware of this patch and had asked David to submit it. David already explained his reasoning internally. It's for an already reported issue by a customer.
Reviewed-by:
Laurence Oberman <[email protected]>
Quoted 25 lines · David Jeffery
When
blk_mq_delay_run_hw_queuessets an hctx to run in the future, it can reset the delay length for an already pending delayed workrun_work. This creates a scenario where multiple hctx may have their queues set to run, but if one runs first and finds nothing to do, it can reset the delay of another hctx and stall the other hctx's ability to run requests.To avoid this I/O stall when an hctx's
run_workis already pending, leave it untouched to run at its current designated time rather than extending its delay. The work will still run which keeps closed the race callingblk_mq_delay_run_hw_queuesis needed for while also avoiding the I/O stall.Signed-off-by David Jeffery <[email protected]>--- block/blk-mq.c | 8 ++++++++ 1 file changed, 8 insertions(+)block/blk-mq.c +0
-0@@ -2177,6 +2177,14 @@ void blk_mq_delay_run_hw_queues(struct request_queue *q, unsigned long msecs)queue_for_each_hw_ctx (q, hctx, i) {
block/blk-mq.c void blk_mq_delay_run_hw_queues(struct request_queue *q, unsigned long msecs) · line 2187
Quoted 10 lines
block/blk-mq.c +8
-0if (blk_mq_hctx_stopped (hctx))continue ; +/* +* If there is already a run_work pending, leave the +* pending delay untouched. Otherwise, a hctx can stall +* if another hctx is re-delaying the other's work +* before the work executes. +*/ +if (delayed_work_pending (&hctx->run_work)) +continue ;
The issue is triggered on BFQ, since BFQ's has_work() may return true, however its ->dispatch_request() may return NULL, so blk_mq_delay_run_hw_queues()
is run for delay schedule.
In case of multiple hw queue, the described issue may be triggered, and cause io stall for long time. And there are only 3 in-tree callers of blk_mq_delay_run_hw_queues(),
David's fix works well for the 3 users, so this patch looks fine:
Thanks,
This patch has now been tested in the customer environment and results were good (fixed the hangs).
Quoted 52 lines · Ming Lei
Quoted 25 lines · David Jeffery
When
blk_mq_delay_run_hw_queuessets an hctx to run in the future, it can reset the delay length for an already pending delayed workrun_work. This creates a scenario where multiple hctx may have their queues set to run, but if one runs first and finds nothing to do, it can reset the delay of another hctx and stall the other hctx's ability to run requests.To avoid this I/O stall when an hctx's
run_workis already pending, leave it untouched to run at its current designated time rather than extending its delay. The work will still run which keeps closed the race callingblk_mq_delay_run_hw_queuesis needed for while also avoiding the I/O stall.Signed-off-by David Jeffery <[email protected]>--- block/blk-mq.c | 8 ++++++++ 1 file changed, 8 insertions(+)block/blk-mq.c +0
-0@@ -2177,6 +2177,14 @@ void blk_mq_delay_run_hw_queues(struct request_queue *q, unsigned long msecs)queue_for_each_hw_ctx (q, hctx, i) {block/blk-mq.c void blk_mq_delay_run_hw_queues(struct request_queue *q, unsigned long msecs) · line 2187
Quoted 10 lines
block/blk-mq.c +8
-0if (blk_mq_hctx_stopped (hctx))continue ; +/* +* If there is already a run_work pending, leave the +* pending delay untouched. Otherwise, a hctx can stall +* if another hctx is re-delaying the other's work +* before the work executes. +*/ +if (delayed_work_pending (&hctx->run_work)) +continue ;The issue is triggered on BFQ, since BFQ's
has_work()may return true, however its ->dispatch_request() may return NULL, soblk_mq_delay_run_hw_queues()
is run for delay schedule.In case of multiple hw queue, the described issue may be triggered, and cause io stall for long time. And there are only 3 in-tree callers of
blk_mq_delay_run_hw_queues(),
David's fix works well for the 3 users, so this patch looks fine:Reviewed-by Ming Lei <[email protected]>Thanks,
Quoted 4 lines · John Pittman
This patch has now been tested in the customer environment and results
were good (fixed the hangs).
Quoted 25 lines
Quoted 24 lines · Ming Lei
Quoted 21 lines · David Jeffery
When
blk_mq_delay_run_hw_queuessets an hctx to run in the future, it can
reset the delay length for an already pending delayed workrun_work. This
creates a scenario where multiple hctx may have their queues set to run,
but if one runs first and finds nothing to do, it can reset the delay of
another hctx and stall the other hctx's ability to run requests.To avoid this I/O stall when an hctx's
run_workis already pending,
leave it untouched to run at its current designated time rather than
extending its delay. The work will still run which keeps closed the race
callingblk_mq_delay_run_hw_queuesis needed for while also avoiding the
I/O stall.
Hello
Quoted 39 lines
Quoted 39 lines
Quoted 22 lines
Signed-off-by David Jeffery <[email protected]>--- block/blk-mq.c | 8 ++++++++ 1 file changed, 8 insertions(+)block/blk-mq.c +2
-0@@ -2177,6 +2177,14 @@ void blk_mq_delay_run_hw_queues(struct request_queue *q, unsigned long msecs) queue_for_each_hw_ctx(q, hctx, i) { if (blk_mq_hctx_stopped(hctx)) continue; +/* +* If there is already a run_work pending, leave the
+ * pending delay untouched. Otherwise, a hctx can
stall
+ * if another hctx is re-delaying the other's
workblock/blk-mq.c void blk_mq_delay_run_hw_queues(struct request_queue *q, unsigned long msecs) · line 2187
Quoted 4 lines
block/blk-mq.c +4
-0+* before the work executes. +*/ +if (delayed_work_pending (&hctx->run_work)) +continue ;The issue is triggered on BFQ, since BFQ's
has_work()may return true,
however its ->dispatch_request() may return NULL, soblk_mq_delay_run_hw_queues()
is run for delay schedule.In case of multiple hw queue, the described issue may be triggered, and cause io
stall for long time. And there are only 3 in-tree callers ofblk_mq_delay_run_hw_queues(),
David's fix works well for the 3 users, so this patch looks fine:
Quoted 6 lines
Quoted 4 lines
Reviewed-by Ming Lei <[email protected]>Thanks,
Hello
Jens, gentle ping, can we get this in please
Sincerely
Laurence and the RH team
Quoted 6 lines · David Jeffery
When
blk_mq_delay_run_hw_queuessets an hctx to run in the future, it can reset the delay length for an already pending delayed workrun_work. This creates a scenario where multiple hctx may have their queues set to run, but if one runs first and finds nothing to do, it can reset the delay of another hctx and stall the other hctx's ability to run requests.
Quoted 7 lines
To avoid this I/O stall when an hctx's
run_workis already pending, leave it untouched to run at its current designated time rather than extending its delay. The work will still run which keeps closed the race callingblk_mq_delay_run_hw_queuesis needed for while also avoiding the I/O stall.[...]
Applied, thanks!
[1/1] blk-mq: avoid extending delays of active hctx from blk_mq_delay_run_hw_queues
commit: 8f5fea65b06de1cc51d4fc23fb4d378d1abd6ed7
Best regards,
Signature · 1 line
Jens Axboe