Re: [PATCH] blk-mq: Fix blk_execute_rq_nowait() handling of dying queues

[Bart Van Assche <Bart.VanAssche@sandisk.com>]

On Wed, 2017-04-12 at 13:01 +0800, Ming Lei wrote:
> On Wed, Apr 12, 2017 at 7:58 AM, Bart Van Assche
> <bart.vanassche@sandisk.com> wrote:
> > 
> > diff --git a/block/blk-exec.c b/block/blk-exec.c
> > index 8cd0e9bc8dc8..f7d9bed2cb15 100644
> > --- a/block/blk-exec.c
> > +++ b/block/blk-exec.c
> > @@ -57,10 +57,13 @@ void blk_execute_rq_nowait(struct request_queue *q, struct gendisk *bd_disk,
> >         rq->end_io = done;
> > 
> >         /*
> > -        * don't check dying flag for MQ because the request won't
> > -        * be reused after dying flag is set
> > +        * The blk_freeze_queue() call in blk_set_queue_dying() and the
> > +        * test of the "dying" flag in blk_queue_enter() guarantee that
> > +        * blk_execute_rq_nowait() won't be called anymore after the "dying"
> > +        * flag has been set.
> 
> That never be guaranteed, see the following case:
> 
> 1) blk_get_request() is called just before queue is set as dying in another path
> 
> 2) the request is allocated successfully and passed to
> blk_execute_rq_nowait() even
> though queue has been set as dying

Hello Ming,

Shouldn't the blk-mq code guarantee that blk_execute_rq_nowait() won't be
called anymore after the "dying" flag has been set? I think changing the
blk_freeze_queue_start() call into blk_freeze_queue() in blk_set_queue_dying()
is sufficient to achieve this.

Note: after I had posted this patch I have been able to reproduce the issue
described in the patch description. Although I still think we need the patch
at the start of this e-mail thread, it doesn't fix the issue I described.

Bart.