Linux-NVME Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] nvme/pci: move cqe check after device shutdown
@ 2020-02-12 16:52 Keith Busch
  2020-02-12 17:40 ` Christoph Hellwig
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Keith Busch @ 2020-02-12 16:52 UTC (permalink / raw)
  To: linux-nvme, hch, sagi, chaitanya.kulkarni; +Cc: Keith Busch

Many users have reported nvme triggered irq_startup() warnings during
shutdown. The driver uses the nvme queue's irq to synchronize scanning
for completions, and enabling an interrupt affined to only offline CPUs
triggers the alarming warning.

Move the final CQE check to after disabling the device and all
registered interrupts have been torn down so that we do not have any
IRQ to synchronize.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=206509
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 drivers/nvme/host/pci.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index da392b50f73e..9c80f9f08149 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -1401,6 +1401,23 @@ static void nvme_disable_admin_queue(struct nvme_dev *dev, bool shutdown)
 	nvme_poll_irqdisable(nvmeq, -1);
 }
 
+/*
+ * Called only on a device that has been disabled and after all other threads
+ * that can check this device's completion queues have synced. This is the
+ * last chance for the driver to see a natural completion before
+ * nvme_cancel_request() terminates all incomplete requests.
+ */
+static void nvme_reap_pending_cqes(struct nvme_dev *dev)
+{
+	u16 start, end;
+	int i;
+
+	for (i = dev->ctrl.queue_count - 1; i > 0; i--) {
+		nvme_process_cq(&dev->queues[i], &start, &end, -1);
+		nvme_complete_cqes(&dev->queues[i], start, end);
+	}
+}
+
 static int nvme_cmb_qdepth(struct nvme_dev *dev, int nr_io_queues,
 				int entry_size)
 {
@@ -2235,11 +2252,6 @@ static bool __nvme_disable_io_queues(struct nvme_dev *dev, u8 opcode)
 		if (timeout == 0)
 			return false;
 
-		/* handle any remaining CQEs */
-		if (opcode == nvme_admin_delete_cq &&
-		    !test_bit(NVMEQ_DELETE_ERROR, &nvmeq->flags))
-			nvme_poll_irqdisable(nvmeq, -1);
-
 		sent--;
 		if (nr_queues)
 			goto retry;
@@ -2428,6 +2440,7 @@ static void nvme_dev_disable(struct nvme_dev *dev, bool shutdown)
 	nvme_suspend_io_queues(dev);
 	nvme_suspend_queue(&dev->queues[0]);
 	nvme_pci_disable(dev);
+	nvme_reap_pending_cqes(dev);
 
 	blk_mq_tagset_busy_iter(&dev->tagset, nvme_cancel_request, &dev->ctrl);
 	blk_mq_tagset_busy_iter(&dev->admin_tagset, nvme_cancel_request, &dev->ctrl);
-- 
2.21.0


_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

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

* Re: [PATCH] nvme/pci: move cqe check after device shutdown
  2020-02-12 16:52 [PATCH] nvme/pci: move cqe check after device shutdown Keith Busch
@ 2020-02-12 17:40 ` Christoph Hellwig
  2020-02-12 19:34 ` Sagi Grimberg
  2020-02-13 15:31 ` David Milburn
  2 siblings, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2020-02-12 17:40 UTC (permalink / raw)
  To: Keith Busch; +Cc: chaitanya.kulkarni, hch, linux-nvme, sagi

s#nvme/pci#nvme-pci# in the subject.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

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

* Re: [PATCH] nvme/pci: move cqe check after device shutdown
  2020-02-12 16:52 [PATCH] nvme/pci: move cqe check after device shutdown Keith Busch
  2020-02-12 17:40 ` Christoph Hellwig
@ 2020-02-12 19:34 ` Sagi Grimberg
  2020-02-13 15:31 ` David Milburn
  2 siblings, 0 replies; 4+ messages in thread
From: Sagi Grimberg @ 2020-02-12 19:34 UTC (permalink / raw)
  To: Keith Busch, linux-nvme, hch, chaitanya.kulkarni

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

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

* Re: [PATCH] nvme/pci: move cqe check after device shutdown
  2020-02-12 16:52 [PATCH] nvme/pci: move cqe check after device shutdown Keith Busch
  2020-02-12 17:40 ` Christoph Hellwig
  2020-02-12 19:34 ` Sagi Grimberg
@ 2020-02-13 15:31 ` David Milburn
  2 siblings, 0 replies; 4+ messages in thread
From: David Milburn @ 2020-02-13 15:31 UTC (permalink / raw)
  To: Keith Busch, linux-nvme, hch, sagi, chaitanya.kulkarni; +Cc: yanghliu, Yi Zhang

Hi Keith,

On 02/12/2020 10:52 AM, Keith Busch wrote:
> Many users have reported nvme triggered irq_startup() warnings during
> shutdown. The driver uses the nvme queue's irq to synchronize scanning
> for completions, and enabling an interrupt affined to only offline CPUs
> triggers the alarming warning.
> 
> Move the final CQE check to after disabling the device and all
> registered interrupts have been torn down so that we do not have any
> IRQ to synchronize.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206509

Yi Zhang and Yanghang Liu both saw the above stack trace in virtual 
environment when removing NVMe device. This patch resolves both
cases.

Thanks,
David

> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
>   drivers/nvme/host/pci.c | 23 ++++++++++++++++++-----
>   1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index da392b50f73e..9c80f9f08149 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -1401,6 +1401,23 @@ static void nvme_disable_admin_queue(struct nvme_dev *dev, bool shutdown)
>   	nvme_poll_irqdisable(nvmeq, -1);
>   }
>   
> +/*
> + * Called only on a device that has been disabled and after all other threads
> + * that can check this device's completion queues have synced. This is the
> + * last chance for the driver to see a natural completion before
> + * nvme_cancel_request() terminates all incomplete requests.
> + */
> +static void nvme_reap_pending_cqes(struct nvme_dev *dev)
> +{
> +	u16 start, end;
> +	int i;
> +
> +	for (i = dev->ctrl.queue_count - 1; i > 0; i--) {
> +		nvme_process_cq(&dev->queues[i], &start, &end, -1);
> +		nvme_complete_cqes(&dev->queues[i], start, end);
> +	}
> +}
> +
>   static int nvme_cmb_qdepth(struct nvme_dev *dev, int nr_io_queues,
>   				int entry_size)
>   {
> @@ -2235,11 +2252,6 @@ static bool __nvme_disable_io_queues(struct nvme_dev *dev, u8 opcode)
>   		if (timeout == 0)
>   			return false;
>   
> -		/* handle any remaining CQEs */
> -		if (opcode == nvme_admin_delete_cq &&
> -		    !test_bit(NVMEQ_DELETE_ERROR, &nvmeq->flags))
> -			nvme_poll_irqdisable(nvmeq, -1);
> -
>   		sent--;
>   		if (nr_queues)
>   			goto retry;
> @@ -2428,6 +2440,7 @@ static void nvme_dev_disable(struct nvme_dev *dev, bool shutdown)
>   	nvme_suspend_io_queues(dev);
>   	nvme_suspend_queue(&dev->queues[0]);
>   	nvme_pci_disable(dev);
> +	nvme_reap_pending_cqes(dev);
>   
>   	blk_mq_tagset_busy_iter(&dev->tagset, nvme_cancel_request, &dev->ctrl);
>   	blk_mq_tagset_busy_iter(&dev->admin_tagset, nvme_cancel_request, &dev->ctrl);
> 


_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

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

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-12 16:52 [PATCH] nvme/pci: move cqe check after device shutdown Keith Busch
2020-02-12 17:40 ` Christoph Hellwig
2020-02-12 19:34 ` Sagi Grimberg
2020-02-13 15:31 ` David Milburn

Linux-NVME Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nvme/0 linux-nvme/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nvme linux-nvme/ https://lore.kernel.org/linux-nvme \
		linux-nvme@lists.infradead.org
	public-inbox-index linux-nvme

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-nvme


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git