nvme: Don't suspend admin queue that wasn't created

PATCH New Gabriel Krisman Bertazi 3 participants 3 messages Sep 6, 2016 – Sep 7, 2016 NVMe

Share & tools

Gabriel Krisman Bertazi

author

2016-09-06 20:39 UTC #0 lore raw

PATCH 1/1 nvme: Don't suspend admin queue that wasn't created

This fixes a regression in my previous commit c21377f8366c ("nvme:
Suspend all queues before deletion"), which provoked an Oops in the removal path when removing a device that became IO incapable very early at probe (i.e. after a failed EEH recovery).

Turns out, if the error occurred very early at the probe path, before even configuring the admin queue, we might try to suspend the uninitialized admin queue, accessing bad memory.

Fixes c21377f8366c ("nvme: Suspend all queues before deletion")

Signed-off-by Gabriel Krisman Bertazi <krisman at linux.vnet.ibm.com>

---
 drivers/nvme/host/pci.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

drivers/nvme/host/pci.c +6 -1

index 8dcf5a960951..be84a84a40f7 100644
@@ -1693,7 +1693,12 @@ static void nvme_dev_disable(struct nvme_dev *dev, bool shutdown)
 		nvme_suspend_queue(dev->queues[i]);
 
 	if (csts & NVME_CSTS_CFS || !(csts & NVME_CSTS_RDY)) {
-		nvme_suspend_queue(dev->queues[0]);
+		/* A device might become IO incapable very soon during
+		 * probe, before the admin queue is configured. Thus,
+		 * queue_count can be 0 here.
+		 */
+		if (dev->queue_count)
+			nvme_suspend_queue(dev->queues[0]);
 	} else {
 		nvme_disable_io_queues(dev);
 		nvme_disable_admin_queue(dev, shutdown);

Signature · 1 line

2.7.4

J Freyensee

2016-09-07 00:07 UTC #1 lore raw

PATCH 1/1 nvme: Don't suspend admin queue that wasn't created

Quoted 30 lines · Gabriel Krisman Bertazi

This fixes a regression in my previous commit c21377f8366c ("nvme:
Suspend all queues before deletion"), which provoked an Oops in the removal path when removing a device that became IO incapable very early
at probe (i.e. after a failed EEH recovery).
Turns out, if the error occurred very early at the probe path, before even configuring the admin queue, we might try to suspend the uninitialized admin queue, accessing bad memory.
Fixes c21377f8366c ("nvme: Suspend all queues before deletion")
Signed-off-by Gabriel Krisman Bertazi <krisman at linux.vnet.ibm.com>
---
?drivers/nvme/host/pci.c | 7 ++++++-
?1 file changed, 6 insertions(+), 1 deletion(-)
drivers/nvme/host/pci.c +0 -0
index 8dcf5a960951..be84a84a40f7 100644
@@ -1693,7 +1693,12 @@ static void nvme_dev_disable(struct nvme_dev
 *dev, bool shutdown)
 ?		nvme_suspend_queue(dev->queues[i]);
?
? if (csts & NVME_CSTS_CFS || !(csts & NVME_CSTS_RDY)) {
drivers/nvme/host/pci.c +1 -1
-		nvme_suspend_queue(dev->queues[0]);
+		/* A device might become IO incapable very soon
during
drivers/nvme/host/pci.c +1 -0
+		?* probe, before the admin queue is configured.
Thus,

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

Quoted 4 lines

drivers/nvme/host/pci.c +4 -0

+		?* queue_count can be 0 here.
+		?*/
+		if (dev->queue_count)
+			nvme_suspend_queue(dev->queues[0]);

Looks like ?a good fix to me.

Reviewed-by Jay Freyensee <james_p_freyensee at linux.intel.com>

Jens Axboe

2016-09-07 14:53 UTC #2 lore raw

PATCH 1/1 nvme: Don't suspend admin queue that wasn't created

In reply to Gabriel Krisman Bertazi

Quoted 11 lines · Gabriel Krisman Bertazi

This fixes a regression in my previous commit c21377f8366c ("nvme:
Suspend all queues before deletion"), which provoked an Oops in the removal path when removing a device that became IO incapable very early at probe (i.e. after a failed EEH recovery).
Turns out, if the error occurred very early at the probe path, before even configuring the admin queue, we might try to suspend the uninitialized admin queue, accessing bad memory.
Fixes c21377f8366c ("nvme: Suspend all queues before deletion")
Signed-off-by Gabriel Krisman Bertazi <krisman at linux.vnet.ibm.com>

Applied for 4.8, thanks.

Signature · 1 line

Jens Axboe