Skip to contentSkip to conversation

nvme/pci: Fix stuck nvme reset

PATCH New Keith Busch 4 participants 4 messages Jun 27, 2017 – Jun 28, 2017 NVMe

lore
Share & tools
lore

Keith Busch

author
#0 lore raw

PATCH 1/1 nvme/pci: Fix stuck nvme reset

The controller state is set to resetting prior to disabling the controller, so this patch accounts for that state when deciding if it needs to freeze the queues. Without this, an 'nvme reset /dev/nvme0' blocks forever because the queues were never frozen.

Fixes 82b057caefaf ("nvme-pci: fix multiple ctrl removal scheduling")
Signed-off-by Keith Busch <keith.busch at intel.com>
---
 drivers/nvme/host/pci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/nvme/host/pci.c +2 -1
index 951042a..0bfda30 100644
@@ -1805,7 +1805,8 @@ static void nvme_dev_disable(struct nvme_dev *dev, bool shutdown)
 	if (pci_is_enabled(pdev)) {
 		u32 csts = readl(dev->bar + NVME_REG_CSTS);
 
-		if (dev->ctrl.state == NVME_CTRL_LIVE)
+		if (dev->ctrl.state == NVME_CTRL_LIVE ||
+				dev->ctrl.state == NVME_CTRL_RESETTING)
 			nvme_start_freeze(&dev->ctrl);
 		dead = !!((csts & NVME_CSTS_CFS) || !(csts & NVME_CSTS_RDY) ||
 			pdev->error_state  != pci_channel_io_normal);
Signature · 1 line
2.5.5
maintainer
#1 lore raw

PATCH 1/1 nvme/pci: Fix stuck nvme reset

Quoted 4 lines · Keith Busch

The controller state is set to resetting prior to disabling the controller, so this patch accounts for that state when deciding if it needs to freeze the queues. Without this, an 'nvme reset /dev/nvme0' blocks forever because the queues were never frozen.

Ouch.

drivers/nvme/host/pci.c static void nvme_dev_disable(struct nvme_dev *dev, bool shutdown) · line 1809

Quoted 3 lines
drivers/nvme/host/pci.c +2 -1
-		if (dev->ctrl.state == NVME_CTRL_LIVE)
+		if (dev->ctrl.state == NVME_CTRL_LIVE ||
+				dev->ctrl.state == NVME_CTRL_RESETTING)

Can you align this a little nicer:

		if (dev->ctrl.state == NVME_CTRL_LIVE ||
		    dev->ctrl.state == NVME_CTRL_RESETTING)

Otherwise this looks fine:

Reviewed-by Christoph Hellwig <hch at lst.de>

Maybe Jens should pick this up directly for the block tree? No point in a nvme pull request just for this.

maintainer
#2 lore raw

PATCH 1/1 nvme/pci: Fix stuck nvme reset

Quoted 17 lines · Christoph Hellwig
Quoted 4 lines · Keith Busch

The controller state is set to resetting prior to disabling the controller, so this patch accounts for that state when deciding if it needs to freeze the queues. Without this, an 'nvme reset /dev/nvme0' blocks forever because the queues were never frozen.

Ouch.

drivers/nvme/host/pci.c static void nvme_dev_disable(struct nvme_dev *dev, bool shutdown) · line 1809

Quoted 3 lines
drivers/nvme/host/pci.c +2 -1
-		if (dev->ctrl.state == NVME_CTRL_LIVE)
+		if (dev->ctrl.state == NVME_CTRL_LIVE ||
+				dev->ctrl.state == NVME_CTRL_RESETTING)

Can you align this a little nicer:

		if (dev->ctrl.state == NVME_CTRL_LIVE ||
		    dev->ctrl.state == NVME_CTRL_RESETTING)
Quoted 6 lines

Otherwise this looks fine:

Reviewed-by Christoph Hellwig <hch at lst.de>

Maybe Jens should pick this up directly for the block tree? No point in a nvme pull request just for this.

Looks good to me too. As it so happens, I just hit this today, but didn't have time to look into why. I'll add it for this series.

Signature · 1 line
Jens Axboe
maintainer
#3 lore raw

PATCH 1/1 nvme/pci: Fix stuck nvme reset

In reply to Christoph Hellwig
Quoted 9 lines

Can you align this a little nicer:

		if (dev->ctrl.state == NVME_CTRL_LIVE ||
		    dev->ctrl.state == NVME_CTRL_RESETTING)

Otherwise this looks fine:

Reviewed-by Christoph Hellwig <hch at lst.de>

Same here,

Reviewed-by Sagi Grimberg <sagi at grimbeg.me>