2017-04-06 00:17:51

by Long Li

[permalink] [raw]
Subject: [PATCH] block-mq: set both block queue and hardware queue restart bit for restart

From: Long Li <[email protected]>

Under heavy I/O, one hardware queue may be unable to dispatch any I/O to the
device layer. This poses a problem with restarting this hardware queue on I/O
finish in blk_mq_sched_restart_queues(), becaue there is nothing pending that
will finish in future on this hardware qeueu. This will result in deadlock.

With this patch, we check for all possible stalled hardware queues when I/O
finishes on any hardware queues. This prevents this deadlock.

Signed-off-by: Long Li <[email protected]>
---
block/blk-mq-sched.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index 09af8ff..f7f3d44 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -202,7 +202,7 @@ void blk_mq_sched_dispatch_requests(struct blk_mq_hw_ctx *hctx)
* needing a restart in that case.
*/
if (!list_empty(&rq_list)) {
- blk_mq_sched_mark_restart_hctx(hctx);
+ blk_mq_sched_mark_restart_queue(hctx);
did_work = blk_mq_dispatch_rq_list(hctx, &rq_list);
} else if (!has_sched_dispatch) {
blk_mq_flush_busy_ctxs(hctx, &rq_list);
--
2.7.4


2017-04-06 00:32:28

by Bart Van Assche

[permalink] [raw]
Subject: Re: [PATCH] block-mq: set both block queue and hardware queue restart bit for restart

On Wed, 2017-04-05 at 17:16 -0700, Long Li wrote:
> Under heavy I/O, one hardware queue may be unable to dispatch any I/O to the
> device layer. This poses a problem with restarting this hardware queue on I/O
> finish in blk_mq_sched_restart_queues(), becaue there is nothing pending that
> will finish in future on this hardware qeueu. This will result in deadlock.
>
> With this patch, we check for all possible stalled hardware queues when I/O
> finishes on any hardware queues. This prevents this deadlock.
>
> Signed-off-by: Long Li <[email protected]>
> ---
> block/blk-mq-sched.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
> index 09af8ff..f7f3d44 100644
> --- a/block/blk-mq-sched.c
> +++ b/block/blk-mq-sched.c
> @@ -202,7 +202,7 @@ void blk_mq_sched_dispatch_requests(struct blk_mq_hw_ctx *hctx)
> * needing a restart in that case.
> */
> if (!list_empty(&rq_list)) {
> - blk_mq_sched_mark_restart_hctx(hctx);
> + blk_mq_sched_mark_restart_queue(hctx);
> did_work = blk_mq_dispatch_rq_list(hctx, &rq_list);
> } else if (!has_sched_dispatch) {
> blk_mq_flush_busy_ctxs(hctx, &rq_list);

Please drop this patch. I'm working on a better solution.

Thanks,

Bart.

2017-04-06 03:38:37

by Long Li

[permalink] [raw]
Subject: RE: [PATCH] block-mq: set both block queue and hardware queue restart bit for restart



> -----Original Message-----
> From: Bart Van Assche [mailto:[email protected]]
> Sent: Wednesday, April 5, 2017 5:32 PM
> To: [email protected]; [email protected]; Long Li
> <[email protected]>; [email protected]
> Cc: Stephen Hemminger <[email protected]>; KY Srinivasan
> <[email protected]>; Long Li <[email protected]>
> Subject: Re: [PATCH] block-mq: set both block queue and hardware queue
> restart bit for restart
>
> On Wed, 2017-04-05 at 17:16 -0700, Long Li wrote:
> > Under heavy I/O, one hardware queue may be unable to dispatch any I/O
> > to the device layer. This poses a problem with restarting this
> > hardware queue on I/O finish in blk_mq_sched_restart_queues(), becaue
> > there is nothing pending that will finish in future on this hardware qeueu.
> This will result in deadlock.
> >
> > With this patch, we check for all possible stalled hardware queues
> > when I/O finishes on any hardware queues. This prevents this deadlock.
> >
> > Signed-off-by: Long Li <[email protected]>
> > ---
> > block/blk-mq-sched.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c index
> > 09af8ff..f7f3d44 100644
> > --- a/block/blk-mq-sched.c
> > +++ b/block/blk-mq-sched.c
> > @@ -202,7 +202,7 @@ void blk_mq_sched_dispatch_requests(struct
> blk_mq_hw_ctx *hctx)
> > * needing a restart in that case.
> > */
> > if (!list_empty(&rq_list)) {
> > - blk_mq_sched_mark_restart_hctx(hctx);
> > + blk_mq_sched_mark_restart_queue(hctx);
> > did_work = blk_mq_dispatch_rq_list(hctx, &rq_list);
> > } else if (!has_sched_dispatch) {
> > blk_mq_flush_busy_ctxs(hctx, &rq_list);
>
> Please drop this patch. I'm working on a better solution.

Thank you. Looking forward to your patch.

>
> Thanks,
>
> Bart.

2017-04-06 03:45:58

by Bart Van Assche

[permalink] [raw]
Subject: Re: [PATCH] block-mq: set both block queue and hardware queue restart bit for restart

On Thu, 2017-04-06 at 03:38 +0000, Long Li wrote:
> > -----Original Message-----
> > From: Bart Van Assche [mailto:[email protected]]
> >
> > Please drop this patch. I'm working on a better solution.
>
> Thank you. Looking forward to your patch.

Hello Long,

It would help if you could share the name of the block or SCSI driver with
which you ran into that lockup and also if you could share the name of the
I/O scheduler used in your test.

Thanks,

Bart.

2017-04-06 04:21:14

by KY Srinivasan

[permalink] [raw]
Subject: RE: [PATCH] block-mq: set both block queue and hardware queue restart bit for restart



> -----Original Message-----
> From: Bart Van Assche [mailto:[email protected]]
> Sent: Wednesday, April 5, 2017 8:46 PM
> To: [email protected]; [email protected]; Long Li
> <[email protected]>; [email protected]
> Cc: Stephen Hemminger <[email protected]>; KY Srinivasan
> <[email protected]>
> Subject: Re: [PATCH] block-mq: set both block queue and hardware queue
> restart bit for restart
>
> On Thu, 2017-04-06 at 03:38 +0000, Long Li wrote:
> > > -----Original Message-----
> > > From: Bart Van Assche [mailto:[email protected]]
> > >
> > > Please drop this patch. I'm working on a better solution.
> >
> > Thank you. Looking forward to your patch.
>
> Hello Long,
>
> It would help if you could share the name of the block or SCSI driver with
> which you ran into that lockup and also if you could share the name of the
> I/O scheduler used in your test.

The tests that indicated the issue were run Hyper-V. The driver is storvsc_drv.c
The I/O scheduler was I think noop.

K. Y
>
> Thanks,
>
> Bart.

2017-04-06 05:38:21

by Long Li

[permalink] [raw]
Subject: RE: [PATCH] block-mq: set both block queue and hardware queue restart bit for restart



> -----Original Message-----
> From: KY Srinivasan
> Sent: Wednesday, April 5, 2017 9:21 PM
> To: Bart Van Assche <[email protected]>; linux-
> [email protected]; [email protected]; Long Li
> <[email protected]>; [email protected]
> Cc: Stephen Hemminger <[email protected]>
> Subject: RE: [PATCH] block-mq: set both block queue and hardware queue
> restart bit for restart
>
>
>
> > -----Original Message-----
> > From: Bart Van Assche [mailto:[email protected]]
> > Sent: Wednesday, April 5, 2017 8:46 PM
> > To: [email protected]; [email protected]; Long Li
> > <[email protected]>; [email protected]
> > Cc: Stephen Hemminger <[email protected]>; KY Srinivasan
> > <[email protected]>
> > Subject: Re: [PATCH] block-mq: set both block queue and hardware queue
> > restart bit for restart
> >
> > On Thu, 2017-04-06 at 03:38 +0000, Long Li wrote:
> > > > -----Original Message-----
> > > > From: Bart Van Assche [mailto:[email protected]]
> > > >
> > > > Please drop this patch. I'm working on a better solution.
> > >
> > > Thank you. Looking forward to your patch.
> >
> > Hello Long,
> >
> > It would help if you could share the name of the block or SCSI driver
> > with which you ran into that lockup and also if you could share the
> > name of the I/O scheduler used in your test.
>
> The tests that indicated the issue were run Hyper-V. The driver is
> storvsc_drv.c The I/O scheduler was I think noop.

Yes, we see I/O hung on scheduler none. Also tried on mq-deadline, same hung with the same cause.

>
> K. Y
> >
> > Thanks,
> >
> > Bart.

2017-04-10 23:47:44

by Bart Van Assche

[permalink] [raw]
Subject: Re: [PATCH] block-mq: set both block queue and hardware queue restart bit for restart

On Thu, 2017-04-06 at 04:21 +0000, KY Srinivasan wrote:
> > -----Original Message-----
> > From: Bart Van Assche [mailto:[email protected]]
> > Sent: Wednesday, April 5, 2017 8:46 PM
> > To: [email protected]; [email protected]; Long Li
> > <[email protected]>; [email protected]
> > Cc: Stephen Hemminger <[email protected]>; KY Srinivasan
> > <[email protected]>
> > Subject: Re: [PATCH] block-mq: set both block queue and hardware queue
> > restart bit for restart
> >
> > On Thu, 2017-04-06 at 03:38 +0000, Long Li wrote:
> > > > -----Original Message-----
> > > > From: Bart Van Assche [mailto:[email protected]]
> > > >
> > > > Please drop this patch. I'm working on a better solution.
> > >
> > > Thank you. Looking forward to your patch.
> >
> > Hello Long,
> >
> > It would help if you could share the name of the block or SCSI driver with
> > which you ran into that lockup and also if you could share the name of the
> > I/O scheduler used in your test.
>
> The tests that indicated the issue were run Hyper-V. The driver is storvsc_drv.c
> The I/O scheduler was I think noop.

Hello Long and K.Y.,

Thank you for the feedback. Can you repeat your test with kernel v4.11-rc6? The
patches that went into the block layer for v4.11-rc6 should be sufficient to fix
this:

$ PAGER= git log --format=short v4.11-rc5..v4.11-rc6 block include/linux/blk* ?
commit 6d8c6c0f97ad8a3517c42b179c1dc8e77397d0e2
Author: Bart Van Assche <[email protected]>

????blk-mq: Restart a single queue if tag sets are shared

commit 7587a5ae7eef0439f7be31f1b5959af062bbc5ec
Author: Bart Van Assche <[email protected]>

????blk-mq: Introduce blk_mq_delay_run_hw_queue()

commit ebe8bddb6e30d7a02775b9972099271fc5910f37
Author: Omar Sandoval <[email protected]>

????blk-mq: remap queues when adding/removing hardware queues

commit 54d5329d425650fafaf90660a139c771d2d49cae
Author: Omar Sandoval <[email protected]>

????blk-mq-sched: fix crash in switch error path

commit 93252632e828da3e90241a1c0e766556abf71598
Author: Omar Sandoval <[email protected]>

????blk-mq-sched: set up scheduler tags when bringing up new queues

commit 6917ff0b5bd4139e08a3f3146529dcb3b95ba7a6
Author: Omar Sandoval <[email protected]>

????blk-mq-sched: refactor scheduler initialization

commit 81380ca10778b99dce98940cfc993214712df335
Author: Omar Sandoval <[email protected]>

????blk-mq: use the right hctx when getting a driver tag fails

commit ac77a0c463c1d7d659861f7b6d1261970dd3282a
Author: Minchan Kim <[email protected]>

????block: do not put mq context in blk_mq_alloc_request_hctx

Bart.

2017-04-13 23:12:14

by Long Li

[permalink] [raw]
Subject: RE: [PATCH] block-mq: set both block queue and hardware queue restart bit for restart



> -----Original Message-----
> From: Bart Van Assche [mailto:[email protected]]
> Sent: Monday, April 10, 2017 4:48 PM
> To: [email protected]; [email protected]; KY Srinivasan
> <[email protected]>; Long Li <[email protected]>; [email protected]
> Cc: Stephen Hemminger <[email protected]>
> Subject: Re: [PATCH] block-mq: set both block queue and hardware queue
> restart bit for restart
>
> On Thu, 2017-04-06 at 04:21 +0000, KY Srinivasan wrote:
> > > -----Original Message-----
> > > From: Bart Van Assche [mailto:[email protected]]
> > > Sent: Wednesday, April 5, 2017 8:46 PM
> > > To: [email protected]; [email protected]; Long
> > > Li <[email protected]>; [email protected]
> > > Cc: Stephen Hemminger <[email protected]>; KY Srinivasan
> > > <[email protected]>
> > > Subject: Re: [PATCH] block-mq: set both block queue and hardware
> > > queue restart bit for restart
> > >
> > > On Thu, 2017-04-06 at 03:38 +0000, Long Li wrote:
> > > > > -----Original Message-----
> > > > > From: Bart Van Assche [mailto:[email protected]]
> > > > >
> > > > > Please drop this patch. I'm working on a better solution.
> > > >
> > > > Thank you. Looking forward to your patch.
> > >
> > > Hello Long,
> > >
> > > It would help if you could share the name of the block or SCSI
> > > driver with which you ran into that lockup and also if you could
> > > share the name of the I/O scheduler used in your test.
> >
> > The tests that indicated the issue were run Hyper-V. The driver is
> > storvsc_drv.c The I/O scheduler was I think noop.
>
> Hello Long and K.Y.,
>
> Thank you for the feedback. Can you repeat your test with kernel v4.11-rc6?
> The patches that went into the block layer for v4.11-rc6 should be sufficient
> to fix
> this:
>
> $ PAGER= git log --format=short v4.11-rc5..v4.11-rc6 block include/linux/blk*
> commit 6d8c6c0f97ad8a3517c42b179c1dc8e77397d0e2
> Author: Bart Van Assche <[email protected]>
>
> ????blk-mq: Restart a single queue if tag sets are shared
>
> commit 7587a5ae7eef0439f7be31f1b5959af062bbc5ec
> Author: Bart Van Assche <[email protected]>
>
> ????blk-mq: Introduce blk_mq_delay_run_hw_queue()
>
> commit ebe8bddb6e30d7a02775b9972099271fc5910f37
> Author: Omar Sandoval <[email protected]>
>
> ????blk-mq: remap queues when adding/removing hardware queues
>
> commit 54d5329d425650fafaf90660a139c771d2d49cae
> Author: Omar Sandoval <[email protected]>
>
> ????blk-mq-sched: fix crash in switch error path
>
> commit 93252632e828da3e90241a1c0e766556abf71598
> Author: Omar Sandoval <[email protected]>
>
> ????blk-mq-sched: set up scheduler tags when bringing up new queues
>
> commit 6917ff0b5bd4139e08a3f3146529dcb3b95ba7a6
> Author: Omar Sandoval <[email protected]>
>
> ????blk-mq-sched: refactor scheduler initialization
>
> commit 81380ca10778b99dce98940cfc993214712df335
> Author: Omar Sandoval <[email protected]>
>
> ????blk-mq: use the right hctx when getting a driver tag fails
>
> commit ac77a0c463c1d7d659861f7b6d1261970dd3282a
> Author: Minchan Kim <[email protected]>
>
> ????block: do not put mq context in blk_mq_alloc_request_hctx
>
> Bart.

Thank you. We are doing tests. I will update on the results.

Long