* [PATCH] blk-mq: avoid extending delays of active hctx from blk_mq_delay_run_hw_queues
@ 2022-01-31 20:33 David Jeffery
2022-02-01 13:39 ` Laurence Oberman
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: David Jeffery @ 2022-01-31 20:33 UTC (permalink / raw)
To: linux-block
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.
Signed-off-by: David Jeffery <djeffery@redhat.com>
---
block/blk-mq.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index f3bf3358a3bb..ae46eb4bf547 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] blk-mq: avoid extending delays of active hctx from blk_mq_delay_run_hw_queues
2022-01-31 20:33 [PATCH] blk-mq: avoid extending delays of active hctx from blk_mq_delay_run_hw_queues David Jeffery
@ 2022-02-01 13:39 ` Laurence Oberman
2022-02-08 2:45 ` Ming Lei
2022-02-17 2:48 ` Jens Axboe
2 siblings, 0 replies; 6+ messages in thread
From: Laurence Oberman @ 2022-02-01 13:39 UTC (permalink / raw)
To: David Jeffery, linux-block
On Mon, 2022-01-31 at 15:33 -0500, David Jeffery wrote:
> 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.
>
> Signed-off-by: David Jeffery <djeffery@redhat.com>
> ---
> block/blk-mq.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
>
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index f3bf3358a3bb..ae46eb4bf547 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -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
>
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 <loberman@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] blk-mq: avoid extending delays of active hctx from blk_mq_delay_run_hw_queues
2022-01-31 20:33 [PATCH] blk-mq: avoid extending delays of active hctx from blk_mq_delay_run_hw_queues David Jeffery
2022-02-01 13:39 ` Laurence Oberman
@ 2022-02-08 2:45 ` Ming Lei
2022-02-14 14:50 ` John Pittman
2022-02-17 2:48 ` Jens Axboe
2 siblings, 1 reply; 6+ messages in thread
From: Ming Lei @ 2022-02-08 2:45 UTC (permalink / raw)
To: David Jeffery; +Cc: linux-block
On Tue, Feb 1, 2022 at 4:34 AM David Jeffery <djeffery@redhat.com> wrote:
>
> 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.
>
> Signed-off-by: David Jeffery <djeffery@redhat.com>
> ---
> block/blk-mq.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
>
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index f3bf3358a3bb..ae46eb4bf547 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -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;
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:
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Thanks,
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] blk-mq: avoid extending delays of active hctx from blk_mq_delay_run_hw_queues
2022-02-08 2:45 ` Ming Lei
@ 2022-02-14 14:50 ` John Pittman
2022-02-22 14:31 ` Laurence Oberman
0 siblings, 1 reply; 6+ messages in thread
From: John Pittman @ 2022-02-14 14:50 UTC (permalink / raw)
To: Ming Lei; +Cc: David Jeffery, linux-block, Laurence Oberman
This patch has now been tested in the customer environment and results
were good (fixed the hangs).
On Mon, Feb 7, 2022 at 9:45 PM Ming Lei <ming.lei@redhat.com> wrote:
>
> On Tue, Feb 1, 2022 at 4:34 AM David Jeffery <djeffery@redhat.com> wrote:
> >
> > 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.
> >
> > Signed-off-by: David Jeffery <djeffery@redhat.com>
> > ---
> > block/blk-mq.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> >
> > diff --git a/block/blk-mq.c b/block/blk-mq.c
> > index f3bf3358a3bb..ae46eb4bf547 100644
> > --- a/block/blk-mq.c
> > +++ b/block/blk-mq.c
> > @@ -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;
>
> 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:
>
> Reviewed-by: Ming Lei <ming.lei@redhat.com>
>
> Thanks,
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] blk-mq: avoid extending delays of active hctx from blk_mq_delay_run_hw_queues
2022-01-31 20:33 [PATCH] blk-mq: avoid extending delays of active hctx from blk_mq_delay_run_hw_queues David Jeffery
2022-02-01 13:39 ` Laurence Oberman
2022-02-08 2:45 ` Ming Lei
@ 2022-02-17 2:48 ` Jens Axboe
2 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2022-02-17 2:48 UTC (permalink / raw)
To: David Jeffery, linux-block
On Mon, 31 Jan 2022 15:33:37 -0500, David Jeffery wrote:
> 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.
>
> [...]
Applied, thanks!
[1/1] blk-mq: avoid extending delays of active hctx from blk_mq_delay_run_hw_queues
commit: 8f5fea65b06de1cc51d4fc23fb4d378d1abd6ed7
Best regards,
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] blk-mq: avoid extending delays of active hctx from blk_mq_delay_run_hw_queues
2022-02-14 14:50 ` John Pittman
@ 2022-02-22 14:31 ` Laurence Oberman
0 siblings, 0 replies; 6+ messages in thread
From: Laurence Oberman @ 2022-02-22 14:31 UTC (permalink / raw)
To: John Pittman, Ming Lei; +Cc: David Jeffery, linux-block
On Mon, 2022-02-14 at 09:50 -0500, John Pittman wrote:
> This patch has now been tested in the customer environment and
> results
> were good (fixed the hangs).
>
> On Mon, Feb 7, 2022 at 9:45 PM Ming Lei <ming.lei@redhat.com> wrote:
> >
> > On Tue, Feb 1, 2022 at 4:34 AM David Jeffery <djeffery@redhat.com>
> > wrote:
> > >
> > > 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.
> > >
Hello
> > > Signed-off-by: David Jeffery <djeffery@redhat.com>
> > > ---
> > > block/blk-mq.c | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > >
> > >
> > > diff --git a/block/blk-mq.c b/block/blk-mq.c
> > > index f3bf3358a3bb..ae46eb4bf547 100644
> > > --- a/block/blk-mq.c
> > > +++ b/block/blk-mq.c
> > > @@ -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;
> >
> > 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:
> >
> > Reviewed-by: Ming Lei <ming.lei@redhat.com>
> >
> > Thanks,
> >
>
>
Hello
Jens, gentle ping, can we get this in please
Sincerely
Laurence and the RH team
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-02-22 14:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-31 20:33 [PATCH] blk-mq: avoid extending delays of active hctx from blk_mq_delay_run_hw_queues David Jeffery
2022-02-01 13:39 ` Laurence Oberman
2022-02-08 2:45 ` Ming Lei
2022-02-14 14:50 ` John Pittman
2022-02-22 14:31 ` Laurence Oberman
2022-02-17 2:48 ` Jens Axboe
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.