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

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

[Long Li]

From: Long Li <longli@microsoft.com>

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 <longli@microsoft.com>
---
 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

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

[Bart Van Assche]

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 <longli@microsoft.com>
> ---
>  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.

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

[Long Li]

> -----Original Message-----
> From: Bart Van Assche [mailto:Bart.VanAssche@sandisk.com]
> Sent: Wednesday, April 5, 2017 5:32 PM
> To: linux-kernel@vger.kernel.org; linux-block@vger.kernel.org; Long Li
> <longli@microsoft.com>; axboe@kernel.dk
> Cc: Stephen Hemminger <sthemmin@microsoft.com>; KY Srinivasan
> <kys@microsoft.com>; Long Li <longli@microsoft.com>
> 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 <longli@microsoft.com>
> > ---
> >  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.

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

[Bart Van Assche]

On Thu, 2017-04-06 at 03:38 +0000, Long Li wrote:
> > -----Original Message-----
> > From: Bart Van Assche [mailto:Bart.VanAssche@sandisk.com]
> > 
> > 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.

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

[KY Srinivasan]

> -----Original Message-----
> From: Bart Van Assche [mailto:Bart.VanAssche@sandisk.com]
> Sent: Wednesday, April 5, 2017 8:46 PM
> To: linux-kernel@vger.kernel.org; linux-block@vger.kernel.org; Long Li
> <longli@microsoft.com>; axboe@kernel.dk
> Cc: Stephen Hemminger <sthemmin@microsoft.com>; KY Srinivasan
> <kys@microsoft.com>
> 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:Bart.VanAssche@sandisk.com]
> > >
> > > 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.

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

[Long Li]

> -----Original Message-----
> From: KY Srinivasan
> Sent: Wednesday, April 5, 2017 9:21 PM
> To: Bart Van Assche <Bart.VanAssche@sandisk.com>; linux-
> kernel@vger.kernel.org; linux-block@vger.kernel.org; Long Li
> <longli@microsoft.com>; axboe@kernel.dk
> Cc: Stephen Hemminger <sthemmin@microsoft.com>
> Subject: RE: [PATCH] block-mq: set both block queue and hardware queue
> restart bit for restart
> 
> 
> 
> > -----Original Message-----
> > From: Bart Van Assche [mailto:Bart.VanAssche@sandisk.com]
> > Sent: Wednesday, April 5, 2017 8:46 PM
> > To: linux-kernel@vger.kernel.org; linux-block@vger.kernel.org; Long Li
> > <longli@microsoft.com>; axboe@kernel.dk
> > Cc: Stephen Hemminger <sthemmin@microsoft.com>; KY Srinivasan
> > <kys@microsoft.com>
> > 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:Bart.VanAssche@sandisk.com]
> > > >
> > > > 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.

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

[Bart Van Assche]

On Thu, 2017-04-06 at 04:21 +0000, KY Srinivasan wrote:
> > -----Original Message-----
> > From: Bart Van Assche [mailto:Bart.VanAssche@sandisk.com]
> > Sent: Wednesday, April 5, 2017 8:46 PM
> > To: linux-kernel@vger.kernel.org; linux-block@vger.kernel.org; Long Li
> > <longli@microsoft.com>; axboe@kernel.dk
> > Cc: Stephen Hemminger <sthemmin@microsoft.com>; KY Srinivasan
> > <kys@microsoft.com>
> > 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:Bart.VanAssche@sandisk.com]
> > > > 
> > > > 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 <bart.vanassche@sandisk.com>

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

commit 7587a5ae7eef0439f7be31f1b5959af062bbc5ec
Author: Bart Van Assche <bart.vanassche@sandisk.com>

    blk-mq: Introduce blk_mq_delay_run_hw_queue()

commit ebe8bddb6e30d7a02775b9972099271fc5910f37
Author: Omar Sandoval <osandov@fb.com>

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

commit 54d5329d425650fafaf90660a139c771d2d49cae
Author: Omar Sandoval <osandov@fb.com>

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

commit 93252632e828da3e90241a1c0e766556abf71598
Author: Omar Sandoval <osandov@fb.com>

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

commit 6917ff0b5bd4139e08a3f3146529dcb3b95ba7a6
Author: Omar Sandoval <osandov@fb.com>

    blk-mq-sched: refactor scheduler initialization

commit 81380ca10778b99dce98940cfc993214712df335
Author: Omar Sandoval <osandov@fb.com>

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

commit ac77a0c463c1d7d659861f7b6d1261970dd3282a
Author: Minchan Kim <minchan@kernel.org>

    block: do not put mq context in blk_mq_alloc_request_hctx

Bart.

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

[Long Li]

> -----Original Message-----
> From: Bart Van Assche [mailto:Bart.VanAssche@sandisk.com]
> Sent: Monday, April 10, 2017 4:48 PM
> To: linux-kernel@vger.kernel.org; linux-block@vger.kernel.org; KY Srinivasan
> <kys@microsoft.com>; Long Li <longli@microsoft.com>; axboe@kernel.dk
> Cc: Stephen Hemminger <sthemmin@microsoft.com>
> 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:Bart.VanAssche@sandisk.com]
> > > Sent: Wednesday, April 5, 2017 8:46 PM
> > > To: linux-kernel@vger.kernel.org; linux-block@vger.kernel.org; Long
> > > Li <longli@microsoft.com>; axboe@kernel.dk
> > > Cc: Stephen Hemminger <sthemmin@microsoft.com>; KY Srinivasan
> > > <kys@microsoft.com>
> > > 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:Bart.VanAssche@sandisk.com]
> > > > >
> > > > > 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 <bart.vanassche@sandisk.com>
> 
>     blk-mq: Restart a single queue if tag sets are shared
> 
> commit 7587a5ae7eef0439f7be31f1b5959af062bbc5ec
> Author: Bart Van Assche <bart.vanassche@sandisk.com>
> 
>     blk-mq: Introduce blk_mq_delay_run_hw_queue()
> 
> commit ebe8bddb6e30d7a02775b9972099271fc5910f37
> Author: Omar Sandoval <osandov@fb.com>
> 
>     blk-mq: remap queues when adding/removing hardware queues
> 
> commit 54d5329d425650fafaf90660a139c771d2d49cae
> Author: Omar Sandoval <osandov@fb.com>
> 
>     blk-mq-sched: fix crash in switch error path
> 
> commit 93252632e828da3e90241a1c0e766556abf71598
> Author: Omar Sandoval <osandov@fb.com>
> 
>     blk-mq-sched: set up scheduler tags when bringing up new queues
> 
> commit 6917ff0b5bd4139e08a3f3146529dcb3b95ba7a6
> Author: Omar Sandoval <osandov@fb.com>
> 
>     blk-mq-sched: refactor scheduler initialization
> 
> commit 81380ca10778b99dce98940cfc993214712df335
> Author: Omar Sandoval <osandov@fb.com>
> 
>     blk-mq: use the right hctx when getting a driver tag fails
> 
> commit ac77a0c463c1d7d659861f7b6d1261970dd3282a
> Author: Minchan Kim <minchan@kernel.org>
> 
>     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

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

[Long Li]

> -----Original Message-----
> From: linux-block-owner@vger.kernel.org [mailto:linux-block-
> owner@vger.kernel.org] On Behalf Of Long Li
> Sent: Friday, April 14, 2017 1:12 AM
> To: Bart Van Assche <Bart.VanAssche@sandisk.com>; linux-
> kernel@vger.kernel.org; linux-block@vger.kernel.org; KY Srinivasan
> <kys@microsoft.com>; axboe@kernel.dk
> Cc: Stephen Hemminger <sthemmin@microsoft.com>
> Subject: RE: [PATCH] block-mq: set both block queue and hardware queue
> restart bit for restart
> 
> 
> 
> > -----Original Message-----
> > From: Bart Van Assche [mailto:Bart.VanAssche@sandisk.com]
> > Sent: Monday, April 10, 2017 4:48 PM
> > To: linux-kernel@vger.kernel.org; linux-block@vger.kernel.org; KY
> > Srinivasan <kys@microsoft.com>; Long Li <longli@microsoft.com>;
> > axboe@kernel.dk
> > Cc: Stephen Hemminger <sthemmin@microsoft.com>
> > 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:Bart.VanAssche@sandisk.com]
> > > > Sent: Wednesday, April 5, 2017 8:46 PM
> > > > To: linux-kernel@vger.kernel.org; linux-block@vger.kernel.org;
> > > > Long Li <longli@microsoft.com>; axboe@kernel.dk
> > > > Cc: Stephen Hemminger <sthemmin@microsoft.com>; KY Srinivasan
> > > > <kys@microsoft.com>
> > > > 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:Bart.VanAssche@sandisk.com]
> > > > > >
> > > > > > 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 <bart.vanassche@sandisk.com>
> >
> >     blk-mq: Restart a single queue if tag sets are shared
> >
> > commit 7587a5ae7eef0439f7be31f1b5959af062bbc5ec
> > Author: Bart Van Assche <bart.vanassche@sandisk.com>
> >
> >     blk-mq: Introduce blk_mq_delay_run_hw_queue()
> >
> > commit ebe8bddb6e30d7a02775b9972099271fc5910f37
> > Author: Omar Sandoval <osandov@fb.com>
> >
> >     blk-mq: remap queues when adding/removing hardware queues
> >
> > commit 54d5329d425650fafaf90660a139c771d2d49cae
> > Author: Omar Sandoval <osandov@fb.com>
> >
> >     blk-mq-sched: fix crash in switch error path
> >
> > commit 93252632e828da3e90241a1c0e766556abf71598
> > Author: Omar Sandoval <osandov@fb.com>
> >
> >     blk-mq-sched: set up scheduler tags when bringing up new queues
> >
> > commit 6917ff0b5bd4139e08a3f3146529dcb3b95ba7a6
> > Author: Omar Sandoval <osandov@fb.com>
> >
> >     blk-mq-sched: refactor scheduler initialization
> >
> > commit 81380ca10778b99dce98940cfc993214712df335
> > Author: Omar Sandoval <osandov@fb.com>
> >
> >     blk-mq: use the right hctx when getting a driver tag fails
> >
> > commit ac77a0c463c1d7d659861f7b6d1261970dd3282a
> > Author: Minchan Kim <minchan@kernel.org>
> >
> >     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

Testing went well. There were no I/O hangs.

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

[Bart Van Assche]

On Fri, 2017-05-05 at 08:58 +0000, Long Li wrote:
> > > From: Bart Van Assche [mailto:Bart.VanAssche@sandisk.com]
> > > 
> > > 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: [ ... ]
> 
> Testing went well. There were no I/O hangs.

Hello Long,

Thanks for having run these tests and thanks for the feedback!

Bart.