All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] nvme: Don't suspend admin queue that wasn't created
@ 2016-09-06 20:39 Gabriel Krisman Bertazi
  2016-09-07  0:07 ` J Freyensee
  2016-09-07 14:53 ` Jens Axboe
  0 siblings, 2 replies; 3+ messages in thread
From: Gabriel Krisman Bertazi @ 2016-09-06 20:39 UTC (permalink / raw)


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(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 8dcf5a960951..be84a84a40f7 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -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);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH] nvme: Don't suspend admin queue that wasn't created
  2016-09-06 20:39 [PATCH] nvme: Don't suspend admin queue that wasn't created Gabriel Krisman Bertazi
@ 2016-09-07  0:07 ` J Freyensee
  2016-09-07 14:53 ` Jens Axboe
  1 sibling, 0 replies; 3+ messages in thread
From: J Freyensee @ 2016-09-07  0:07 UTC (permalink / raw)


On Tue, 2016-09-06@17:39 -0300, Gabriel Krisman Bertazi wrote:
> 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(-)
> 
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 8dcf5a960951..be84a84a40f7 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -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]);

Looks like ?a good fix to me.

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH] nvme: Don't suspend admin queue that wasn't created
  2016-09-06 20:39 [PATCH] nvme: Don't suspend admin queue that wasn't created Gabriel Krisman Bertazi
  2016-09-07  0:07 ` J Freyensee
@ 2016-09-07 14:53 ` Jens Axboe
  1 sibling, 0 replies; 3+ messages in thread
From: Jens Axboe @ 2016-09-07 14:53 UTC (permalink / raw)


On 09/06/2016 02:39 PM, Gabriel Krisman Bertazi wrote:
> 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.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-09-07 14:53 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-06 20:39 [PATCH] nvme: Don't suspend admin queue that wasn't created Gabriel Krisman Bertazi
2016-09-07  0:07 ` J Freyensee
2016-09-07 14:53 ` Jens Axboe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.