[PATCH V2] nvme: don't wait freeze during resetting

[PATCH V2] nvme: don't wait freeze during resetting

[Ming Lei]

First it isn't necessary to call nvme_wait_freeze during reset.
For nvme-pci, 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.

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
Reviewed-by: Chao Leng <lengchao@huawei.com>
Reviewed-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
V2:
	- remove the change in rdma/tcp which may make non-mpath reset
	time much longer, as pointed by Sagi

 drivers/nvme/host/apple.c | 1 -
 drivers/nvme/host/pci.c   | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/nvme/host/apple.c b/drivers/nvme/host/apple.c
index 5fc5ea196b40..9cd02b57fc85 100644
--- a/drivers/nvme/host/apple.c
+++ b/drivers/nvme/host/apple.c
@@ -1126,7 +1126,6 @@ static void apple_nvme_reset_work(struct work_struct *work)
 	anv->ctrl.queue_count = nr_io_queues + 1;
 
 	nvme_start_queues(&anv->ctrl);
-	nvme_wait_freeze(&anv->ctrl);
 	blk_mq_update_nr_hw_queues(&anv->tagset, 1);
 	nvme_unfreeze(&anv->ctrl);
 
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 98864b853eef..985b216907fc 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);
 		if (!dev->ctrl.tagset)
 			nvme_pci_alloc_tag_set(dev);
 		else
-- 
2.31.1

Re: [PATCH V2] nvme: don't wait freeze during resetting

[Christoph Hellwig]

On Mon, Sep 26, 2022 at 09:34:04AM +0800, Ming Lei wrote:
> First it isn't necessary to call nvme_wait_freeze during reset.
> For nvme-pci, 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.
> 
> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
> Reviewed-by: Chao Leng <lengchao@huawei.com>
> Reviewed-by: Keith Busch <kbusch@kernel.org>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
> V2:
> 	- remove the change in rdma/tcp which may make non-mpath reset
> 	time much longer, as pointed by Sagi

So how can this be corret for PCI and apple but not for rdma and TCP?

Also please don't update two drivers in a single patch.

Re: [PATCH V2] nvme: don't wait freeze during resetting

[Ming Lei]

On Mon, Oct 10, 2022 at 10:25:33AM +0200, Christoph Hellwig wrote:
> On Mon, Sep 26, 2022 at 09:34:04AM +0800, Ming Lei wrote:
> > First it isn't necessary to call nvme_wait_freeze during reset.
> > For nvme-pci, 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.
> > 
> > Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
> > Reviewed-by: Chao Leng <lengchao@huawei.com>
> > Reviewed-by: Keith Busch <kbusch@kernel.org>
> > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > ---
> > V2:
> > 	- remove the change in rdma/tcp which may make non-mpath reset
> > 	time much longer, as pointed by Sagi
> 
> So how can this be corret for PCI and apple but not for rdma and TCP?

It doesn't mean it isn't correct for rdma and TCP, just the similar
patch may increase timeout delay a lot for the two, nvme_wait_freeze_timeout(ctrl,
NVME_IO_TIMEOUT) waits 30 secs at default, but nvme_wait_freeze() may wait
much longer if retry is involved.

> 
> Also please don't update two drivers in a single patch.

But the change is same for both two, and we have lots of such example,
same change on lots of subsystem.


thanks,
Ming