[PATCH 2/2] nvme-pci: don't wait freeze during resetting

[Keith Busch]

On Thu, Aug 25, 2022 at 07:34:30PM +0800, Ming Lei wrote:
> On Thu, Aug 25, 2022 at 06:05:30PM +0800, Chao Leng wrote:
> > 
> > 
> > On 2022/8/21 16:47, Ming Lei wrote:
> > > First it isn't necessary to call nvme_wait_freeze in nvme_reset_work().
> > > If tagset isn't allocated, there can't be any inflight IOs; otherwise
> > > blk_mq_update_nr_hw_queues can freeze & wait queues.
> > > 
> > > Second, since commit bdd6316094e0 ("block: Allow unfreezing of a queue
> > > while requests are in progress"), it is fine to unfreeze queue without
> > > draining inflight IOs.
> > > 
> > > Signed-off-by: Ming Lei <ming.lei at redhat.com>
> > > ---
> > >   drivers/nvme/host/pci.c | 1 -
> > >   1 file changed, 1 deletion(-)
> > > 
> > > diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> > > index 3a1c37f32f30..91b2903fcc24 100644
> > > --- a/drivers/nvme/host/pci.c
> > > +++ b/drivers/nvme/host/pci.c
> > > @@ -2910,7 +2910,6 @@ static void nvme_reset_work(struct work_struct *work)
> > >   		nvme_free_tagset(dev);
> > >   	} else {
> > >   		nvme_start_queues(&dev->ctrl);
> > > -		nvme_wait_freeze(&dev->ctrl);
> > It is not safe.
> > nvme_dev_add may call blk_mq_update_nr_hw_queues, blk_mq_update_nr_hw_queues
> > require the state is frozen.
> 
> blk_mq_update_nr_hw_queues() calls blk_freeze_queue() for every queue in
> this tagset, so it needn't nvme's freeze wait.

Right, the blk-mq realloc should be fine, and it's really a very uncommon event
that nvme needs to realloc the tag set.

I'm trying to remember why I added the wait freeze here in the first place, and
I don't recall a good reason for it. I think I had a controller that always
timed out IO, and the resets would just continue indefinitely. Waiting for
freeze within a reset causes the next IO timeout to trigger a driver detach.
That doesn't sound like a good idea, though, since it tears down the admin
queue too.

Anyway, this looks good to me.

Reviewed-by: Keith Busch <kbusch at kernel.org>