[PATCH 0/2] Asynchronous device shutdown

[PATCH 0/2] Asynchronous device shutdown

[Keith Busch]

Device shutdown synchronization was recently reverted due to complications
from unrelated 'async_schedule's holding up shutdown forever and the
fact that no device driver was making use of the capability anyway.

I would like to make use of this capability though; I'm told by some
vendors that devices I develop drivers for may take a while to safely
shutdown and there might be a lot of these devices in a machine, so we
would benefit from letting this happen in parallel.

This patch set adds a slightly different shutdown synchronization using
a new domain, and I've added an implementation to the nvm-express driver
here so there's at least one user, assuming this is acceptable.

Keith Busch (2):
  driver-core: allow asynchronous device shutdown
  NVMe: Complete shutdown asynchronously

 drivers/base/core.c       |    4 ++++
 drivers/block/nvme-core.c |   28 ++++++++++++++++++++++++++--
 include/linux/device.h    |    1 +
 include/linux/nvme.h      |    1 +
 4 files changed, 32 insertions(+), 2 deletions(-)

-- 
1.7.10.4

[PATCH 1/2] driver-core: allow asynchronous device shutdown

[Keith Busch]

A patch to allow .shutdown to execute asynchronously. Some devices may
take a long time to complete a shutdown, so this patch lets a driver
safely shutdown multiple devices asynchronously.

This uses an exclusive asynchronous domain so other unrelated async
tasks can't cause shutdown to hold up indefinitely.

Signed-off-by: Keith Busch <keith.busch@intel.com>
---
 drivers/base/core.c    |    4 ++++
 include/linux/device.h |    1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 20da3ad..71b83bb 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -10,6 +10,7 @@
  *
  */
 
+#include <linux/async.h>
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/init.h>
@@ -1926,6 +1927,8 @@ out:
 }
 EXPORT_SYMBOL_GPL(device_move);
 
+ASYNC_DOMAIN_EXCLUSIVE(shutdown_domain);
+EXPORT_SYMBOL(shutdown_domain);
 /**
  * device_shutdown - call ->shutdown() on each device to shutdown.
  */
@@ -1986,6 +1989,7 @@ void device_shutdown(void)
 		spin_lock(&devices_kset->list_lock);
 	}
 	spin_unlock(&devices_kset->list_lock);
+	async_synchronize_full_domain(&shutdown_domain);
 }
 
 /*
diff --git a/include/linux/device.h b/include/linux/device.h
index d1d1c05..95ed9f1 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -997,6 +997,7 @@ static inline int devtmpfs_mount(const char *mountpoint) { return 0; }
 
 /* drivers/base/power/shutdown.c */
 extern void device_shutdown(void);
+extern struct async_domain shutdown_domain;
 
 /* debugging and troubleshooting/diagnostic helpers. */
 extern const char *dev_driver_string(const struct device *dev);
-- 
1.7.10.4

Re: [PATCH 1/2] driver-core: allow asynchronous device shutdown

[Greg KH]

On Thu, May 08, 2014 at 12:07:33PM -0600, Keith Busch wrote:
> A patch to allow .shutdown to execute asynchronously. Some devices may
> take a long time to complete a shutdown, so this patch lets a driver
> safely shutdown multiple devices asynchronously.
> 
> This uses an exclusive asynchronous domain so other unrelated async
> tasks can't cause shutdown to hold up indefinitely.
> 
> Signed-off-by: Keith Busch <keith.busch@intel.com>
> ---
>  drivers/base/core.c    |    4 ++++
>  include/linux/device.h |    1 +
>  2 files changed, 5 insertions(+)
> 
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 20da3ad..71b83bb 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -10,6 +10,7 @@
>   *
>   */
>  
> +#include <linux/async.h>
>  #include <linux/device.h>
>  #include <linux/err.h>
>  #include <linux/init.h>
> @@ -1926,6 +1927,8 @@ out:
>  }
>  EXPORT_SYMBOL_GPL(device_move);
>  
> +ASYNC_DOMAIN_EXCLUSIVE(shutdown_domain);
> +EXPORT_SYMBOL(shutdown_domain);

EXPORT_SYMBOL_GPL() please, for driver core functions.

thanks,

greg k-h

[PATCH 2/2] NVMe: Complete shutdown asynchronously

[Keith Busch]

Some devices may take a long time to complete a shutdown.  The driver
can perform the device shutdown asynchronously so multiple devices may
be done in parallel and speed up the overall system shutdown.

PCI disables MSI/MSI-x after the driver's .shutdown returns, so the
driver enables the INTx irq if performing the shutdown asynchronously.

Signed-off-by: Keith Busch <keith.busch@intel.com>
---
 drivers/block/nvme-core.c |   28 ++++++++++++++++++++++++++--
 include/linux/nvme.h      |    1 +
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index cd8a8bc7..a9982b8 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -13,6 +13,7 @@
  */
 
 #include <linux/nvme.h>
+#include <linux/async.h>
 #include <linux/bio.h>
 #include <linux/bitops.h>
 #include <linux/blkdev.h>
@@ -1401,7 +1402,7 @@ static int nvme_shutdown_ctrl(struct nvme_dev *dev)
 	cc = (readl(&dev->bar->cc) & ~NVME_CC_SHN_MASK) | NVME_CC_SHN_NORMAL;
 	writel(cc, &dev->bar->cc);
 
-	timeout = 2 * HZ + jiffies;
+	timeout = 20 * HZ + jiffies;
 	while ((readl(&dev->bar->csts) & NVME_CSTS_SHST_MASK) !=
 							NVME_CSTS_SHST_CMPLT) {
 		msleep(100);
@@ -2739,6 +2740,7 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	dev->reset_workfn = nvme_reset_failed_dev;
 	INIT_WORK(&dev->reset_work, nvme_reset_workfn);
 	dev->pci_dev = pdev;
+	dev->intx_irq = pdev->irq;
 	pci_set_drvdata(pdev, dev);
 	result = nvme_set_instance(dev);
 	if (result)
@@ -2791,10 +2793,32 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	return result;
 }
 
+static void nvme_async_shutdown(void *data, async_cookie_t cookie)
+{
+	struct nvme_dev *dev = data;
+	nvme_dev_shutdown(dev);
+	free_irq(dev->intx_irq, raw_nvmeq(dev, 0));
+}
+
 static void nvme_shutdown(struct pci_dev *pdev)
 {
 	struct nvme_dev *dev = pci_get_drvdata(pdev);
-	nvme_dev_shutdown(dev);
+	struct nvme_queue *adminq = raw_nvmeq(dev, 0);
+	bool use_async = true;
+
+	/* MSI/MSI-x are disabled after returning from this function */
+	if (dev->pci_dev->msi_enabled || dev->pci_dev->msix_enabled) {
+		if (request_irq(dev->intx_irq, nvme_irq, IRQF_SHARED,
+						adminq->irqname, adminq))
+			use_async = false;
+		else
+			pci_intx(dev->pci_dev, 1);
+	}
+	if (!use_async)
+		nvme_dev_shutdown(dev);
+	else
+		async_schedule_domain(nvme_async_shutdown, dev,
+							&shutdown_domain);
 }
 
 static void nvme_remove(struct pci_dev *pdev)
diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index 1813cfd..3df7c48 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -80,6 +80,7 @@ struct nvme_dev {
 	unsigned queue_count;
 	unsigned online_queues;
 	unsigned max_qid;
+	unsigned intx_irq;
 	int q_depth;
 	u32 db_stride;
 	u32 ctrl_config;
-- 
1.7.10.4