Re: [PATCH V2 2/3] block: don't drain in-progress dispatch in blk_cleanup_queue()

Ming Lei
2019-04-02T00:38:59Z

On Mon, Apr 01, 2019 at 08:14:59AM -0700, Bart Van Assche wrote:
> On Mon, 2019-04-01 at 12:42 +0800, Ming Lei wrote:
> > Now freeing hw queue resource is moved to hctx's release handler,
> > we don't need to worry about the race between blk_cleanup_queue and
> > run queue any more.
> >
> > So don't drain in-progress dispatch in blk_cleanup_queue().
> >
> > This is basically revert of c2856ae2f315 ("blk-mq: quiesce queue before
> > freeing queue").
> >
> > Cc: Dongli Zhang <dongli.zhang@oracle.com>
> > Cc: James Smart <james.smart@broadcom.com>
> > Cc: Bart Van Assche <bart.vanassche@wdc.com>
> > Cc: linux-scsi@vger.kernel.org,
> > Cc: Martin K . Petersen <martin.petersen@oracle.com>,
> > Cc: Christoph Hellwig <hch@lst.de>,
> > Cc: James E . J . Bottomley <jejb@linux.vnet.ibm.com>,
> > Cc: jianchao wang <jianchao.w.wang@oracle.com>
> > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > ---
> > block/blk-core.c | 12 ------------
> > 1 file changed, 12 deletions(-)
> >
> > diff --git a/block/blk-core.c b/block/blk-core.c
> > index b3bbf8a5110d..491dc0295778 100644
> > --- a/block/blk-core.c
> > +++ b/block/blk-core.c
> > @@ -347,18 +347,6 @@ void blk_cleanup_queue(struct request_queue *q)
> >
> > blk_queue_flag_set(QUEUE_FLAG_DEAD, q);
> >
> > - /*
> > - * make sure all in-progress dispatch are completed because
> > - * blk_freeze_queue() can only complete all requests, and
> > - * dispatch may still be in-progress since we dispatch requests
> > - * from more than one contexts.
> > - *
> > - * We rely on driver to deal with the race in case that queue
> > - * initialization isn't done.
> > - */
> > - if (queue_is_mq(q) && blk_queue_init_done(q))
> > - blk_mq_quiesce_queue(q);
> > -
> > /* for synchronous bio-based driver finish in-flight integrity i/o */
> > blk_flush_integrity();
>
> Many block drivers clean up resources immediately after blk_cleanup_queue()
> returns. Not waiting for ongoing .queue_rq() calls to finish is wrong because
> it can cause block drivers to destroy resources that are in use by a concurrent
> .queue_rq() call.

blk_freeze_queue() has returned, so there can't be any in-flight IOs and .queue_rq(),
but there might be run queue activities, which do not call into driver.

Thanks,
Ming