[PATCH v6 0/4] shut down devices asynchronously

[PATCH v6 0/4] shut down devices asynchronously

[Stuart Hayes]

This adds the ability for the kernel to shutdown devices asynchronously
(this is V6).

It is in "safe" mode by default (where only devices whose drivers opt-in
are shut down asynchronously), but it can be "on" (where only devices
whose drivers opt-out are shut down synchronously), or "off" (legacy
behavior).

This can dramatically reduce system shutdown/reboot time on systems that
have multiple devices that take many seconds to shut down (like certain
NVMe drives). On one system tested, the shutdown time went from 11 minutes
without this patch to 55 seconds with the patch.

Changes from V5:

Separated into multiple patches to make review easier.
Reworked some code to make it more readable
Made devices wait for consumers to shut down, not just children
  (suggested by David Jeffery)

Changes from V4:

Change code to use cookies for synchronization rather than async domains
Allow async shutdown to be disabled via sysfs, and allow driver opt-in or
  opt-out of async shutdown (when not disabled), with ability to control
  driver opt-in/opt-out via sysfs

Changes from V3:

Bug fix (used "parent" not "dev->parent" in device_shutdown)

Changes from V2:

Removed recursive functions to schedule children to be shutdown before
  parents, since existing device_shutdown loop will already do this

Changes from V1:

Rewritten using kernel async code (suggested by Lukas Wunner)

Stuart Hayes (4):
  driver core: don't always lock parent in shutdown
  driver core: separate function to shutdown one device
  driver core: shut down devices asynchronously
  nvme-pci: Make driver prefer asynchronous shutdown

 drivers/base/base.h           |   3 +
 drivers/base/bus.c            |  47 +++++++++
 drivers/base/core.c           | 185 +++++++++++++++++++++++++++++-----
 drivers/nvme/host/pci.c       |   1 +
 include/linux/device/driver.h |   8 ++
 5 files changed, 218 insertions(+), 26 deletions(-)

-- 
2.39.3

[PATCH v6 1/4] driver core: don't always lock parent in shutdown

[Stuart Hayes]

Don't lock a parent device unless it is needed in device_shutdown. This
is in preparation for making device shutdown asynchronous, when it will
be needed to allow children of a common parent to shut down
simultaneously.

Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
Signed-off-by: David Jeffery <djeffery@redhat.com>
---
 drivers/base/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 5f4e03336e68..e63177314e86 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -4856,7 +4856,7 @@ void device_shutdown(void)
 		spin_unlock(&devices_kset->list_lock);
 
 		/* hold lock to avoid race with probe/release */
-		if (parent)
+		if (parent && dev->bus && dev->bus->need_parent_lock)
 			device_lock(parent);
 		device_lock(dev);
 
@@ -4880,7 +4880,7 @@ void device_shutdown(void)
 		}
 
 		device_unlock(dev);
-		if (parent)
+		if (parent && dev->bus && dev->bus->need_parent_lock)
 			device_unlock(parent);
 
 		put_device(dev);
-- 
2.39.3

[PATCH v6 2/4] driver core: separate function to shutdown one device

[Stuart Hayes]

Make a separate function for the part of device_shutdown() that does the
shutown for a single device.  This is in preparation for making device
shutdown asynchronous.

Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
Signed-off-by: David Jeffery <djeffery@redhat.com>
---
 drivers/base/core.c | 66 ++++++++++++++++++++++++---------------------
 1 file changed, 36 insertions(+), 30 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index e63177314e86..e76cba51513a 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -4819,6 +4819,41 @@ int device_change_owner(struct device *dev, kuid_t kuid, kgid_t kgid)
 }
 EXPORT_SYMBOL_GPL(device_change_owner);
 
+static void shutdown_one_device(struct device *dev)
+{
+	/* hold lock to avoid race with probe/release */
+	if (dev->parent && dev->bus && dev->bus->need_parent_lock)
+		device_lock(dev->parent);
+	device_lock(dev);
+
+	/* Don't allow any more runtime suspends */
+	pm_runtime_get_noresume(dev);
+	pm_runtime_barrier(dev);
+
+	if (dev->class && dev->class->shutdown_pre) {
+		if (initcall_debug)
+			dev_info(dev, "shutdown_pre\n");
+		dev->class->shutdown_pre(dev);
+	}
+	if (dev->bus && dev->bus->shutdown) {
+		if (initcall_debug)
+			dev_info(dev, "shutdown\n");
+		dev->bus->shutdown(dev);
+	} else if (dev->driver && dev->driver->shutdown) {
+		if (initcall_debug)
+			dev_info(dev, "shutdown\n");
+		dev->driver->shutdown(dev);
+	}
+
+	device_unlock(dev);
+	if (dev->parent && dev->bus && dev->bus->need_parent_lock)
+		device_unlock(dev->parent);
+
+	put_device(dev);
+	if (dev->parent)
+		put_device(dev->parent);
+}
+
 /**
  * device_shutdown - call ->shutdown() on each device to shutdown.
  */
@@ -4855,36 +4890,7 @@ void device_shutdown(void)
 		list_del_init(&dev->kobj.entry);
 		spin_unlock(&devices_kset->list_lock);
 
-		/* hold lock to avoid race with probe/release */
-		if (parent && dev->bus && dev->bus->need_parent_lock)
-			device_lock(parent);
-		device_lock(dev);
-
-		/* Don't allow any more runtime suspends */
-		pm_runtime_get_noresume(dev);
-		pm_runtime_barrier(dev);
-
-		if (dev->class && dev->class->shutdown_pre) {
-			if (initcall_debug)
-				dev_info(dev, "shutdown_pre\n");
-			dev->class->shutdown_pre(dev);
-		}
-		if (dev->bus && dev->bus->shutdown) {
-			if (initcall_debug)
-				dev_info(dev, "shutdown\n");
-			dev->bus->shutdown(dev);
-		} else if (dev->driver && dev->driver->shutdown) {
-			if (initcall_debug)
-				dev_info(dev, "shutdown\n");
-			dev->driver->shutdown(dev);
-		}
-
-		device_unlock(dev);
-		if (parent && dev->bus && dev->bus->need_parent_lock)
-			device_unlock(parent);
-
-		put_device(dev);
-		put_device(parent);
+		shutdown_one_device(dev);
 
 		spin_lock(&devices_kset->list_lock);
 	}
-- 
2.39.3

[PATCH v6 3/4] driver core: shut down devices asynchronously

[Stuart Hayes]

Add code to shut down devices asynchronously, while ensuring that each
device is shut down before its parents & suppliers, and allowing devices
that share a driver to be shutdown one at a time if necessary.

Add /sys/kernel/async_shutdown to allow user control of this feature:

  safe: shut down all devices synchronously, unless driver prefers async
	shutdown (driver opt-in) (default)
  on:	shut down all devices asynchronously, unless disabled by the driver
	(driver opt-out)
  off:	shut down all devices synchronously

Add async_shutdown to struct device_driver, and expose it via sysfs.
This will be used to view or change driver opt-in/opt-out of asynchronous
shutdown, if it is globally enabled.

 async:   driver opt-in to async device shutdown (devices will be shut down
          asynchronously if async_shutdown is "on" or "safe")
 sync:    driver opt-out of async device shutdown (devices will always be
          shut down synchronously)
 default: devices will be shutdown asynchronously if async_shutdown is "on"

This can dramatically reduce system shutdown/reboot time on systems that
have multiple devices that take many seconds to shut down (like certain
NVMe drives). On one system tested, the shutdown time went from 11 minutes
without this patch to 55 seconds with the patch.

Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
Signed-off-by: David Jeffery <djeffery@redhat.com>
---
 drivers/base/base.h           |   3 +
 drivers/base/bus.c            |  47 +++++++++++++
 drivers/base/core.c           | 129 +++++++++++++++++++++++++++++++++-
 include/linux/device/driver.h |   8 +++
 4 files changed, 186 insertions(+), 1 deletion(-)

diff --git a/drivers/base/base.h b/drivers/base/base.h
index 0738ccad08b2..ab80a0721b2e 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -10,6 +10,7 @@
  * shared outside of the drivers/base/ directory.
  *
  */
+#include <linux/async.h>
 #include <linux/notifier.h>
 
 /**
@@ -97,6 +98,7 @@ struct driver_private {
  *	the device; typically because it depends on another driver getting
  *	probed first.
  * @async_driver - pointer to device driver awaiting probe via async_probe
+ * @shutdown_after - used during async shutdown to ensure correct shutdown ordering.
  * @device - pointer back to the struct device that this structure is
  * associated with.
  * @dead - This device is currently either in the process of or has been
@@ -114,6 +116,7 @@ struct device_private {
 	struct list_head deferred_probe;
 	struct device_driver *async_driver;
 	char *deferred_probe_reason;
+	async_cookie_t shutdown_after;
 	struct device *device;
 	u8 dead:1;
 };
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index daee55c9b2d9..403eecab22a3 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -10,6 +10,7 @@
  */
 
 #include <linux/async.h>
+#include <linux/capability.h>
 #include <linux/device/bus.h>
 #include <linux/device.h>
 #include <linux/module.h>
@@ -635,6 +636,46 @@ static ssize_t uevent_store(struct device_driver *drv, const char *buf,
 }
 static DRIVER_ATTR_WO(uevent);
 
+static ssize_t async_shutdown_show(struct device_driver *drv, char *buf)
+{
+	char *output;
+
+	switch (drv->shutdown_type) {
+	case SHUTDOWN_DEFAULT_STRATEGY:
+		output = "default";
+		break;
+	case SHUTDOWN_PREFER_ASYNCHRONOUS:
+		output = "enabled";
+		break;
+	case SHUTDOWN_FORCE_SYNCHRONOUS:
+		output = "disabled";
+		break;
+	default:
+		output = "unknown";
+	}
+	return sysfs_emit(buf, "%s\n", output);
+}
+
+static ssize_t async_shutdown_store(struct device_driver *drv, const char *buf,
+			      size_t count)
+{
+	if (!capable(CAP_SYS_BOOT))
+		return -EPERM;
+
+	if (!strncmp(buf, "disabled", 8))
+		drv->shutdown_type = SHUTDOWN_FORCE_SYNCHRONOUS;
+	else if (!strncmp(buf, "enabled", 2))
+		drv->shutdown_type = SHUTDOWN_PREFER_ASYNCHRONOUS;
+	else if (!strncmp(buf, "default", 4))
+		drv->shutdown_type = SHUTDOWN_DEFAULT_STRATEGY;
+	else
+		return -EINVAL;
+
+	return count;
+}
+
+static DRIVER_ATTR_RW(async_shutdown);
+
 /**
  * bus_add_driver - Add a driver to the bus.
  * @drv: driver.
@@ -697,6 +738,12 @@ int bus_add_driver(struct device_driver *drv)
 		}
 	}
 
+	error = driver_create_file(drv, &driver_attr_async_shutdown);
+	if (error) {
+		pr_err("%s: async_shutdown attr (%s) failed\n",
+			__func__, drv->name);
+	}
+
 	return 0;
 
 out_del_list:
diff --git a/drivers/base/core.c b/drivers/base/core.c
index e76cba51513a..1f71282741f8 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -9,6 +9,7 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/async.h>
 #include <linux/cpufreq.h>
 #include <linux/device.h>
 #include <linux/err.h>
@@ -46,6 +47,65 @@ static bool fw_devlink_drv_reg_done;
 static bool fw_devlink_best_effort;
 static struct workqueue_struct *device_link_wq;
 
+enum async_device_shutdown_enabled {
+	ASYNC_DEV_SHUTDOWN_DISABLED,
+	ASYNC_DEV_SHUTDOWN_SAFE,
+	ASYNC_DEV_SHUTDOWN_ENABLED,
+};
+
+static enum async_device_shutdown_enabled
+			async_device_shutdown_enabled = ASYNC_DEV_SHUTDOWN_SAFE;
+
+static ssize_t async_device_shutdown_show(struct kobject *kobj,
+					  struct kobj_attribute *attr, char *buf)
+{
+	const char *output;
+
+	switch (async_device_shutdown_enabled) {
+	case ASYNC_DEV_SHUTDOWN_DISABLED:
+		output = "off";
+		break;
+	case ASYNC_DEV_SHUTDOWN_SAFE:
+		output = "safe";
+		break;
+	case ASYNC_DEV_SHUTDOWN_ENABLED:
+		output = "on";
+		break;
+	default:
+		output = "unknown";
+	}
+
+	return sysfs_emit(buf, "%s\n", output);
+}
+
+static ssize_t async_device_shutdown_store(struct kobject *kobj,
+					   struct kobj_attribute *attr,
+					   const char *buf, size_t count)
+{
+	if (!capable(CAP_SYS_BOOT))
+		return -EPERM;
+
+	if (!strncmp(buf, "off", 3))
+		async_device_shutdown_enabled = ASYNC_DEV_SHUTDOWN_DISABLED;
+	else if (!strncmp(buf, "safe", 4))
+		async_device_shutdown_enabled = ASYNC_DEV_SHUTDOWN_SAFE;
+	else if (!strncmp(buf, "on", 2))
+		async_device_shutdown_enabled = ASYNC_DEV_SHUTDOWN_ENABLED;
+	else
+		return -EINVAL;
+
+	return count;
+}
+
+static struct kobj_attribute async_device_shutdown_attr = __ATTR_RW(async_device_shutdown);
+
+static int __init async_shutdown_sysfs_init(void)
+{
+	return sysfs_create_file(kernel_kobj, &async_device_shutdown_attr.attr);
+}
+
+late_initcall(async_shutdown_sysfs_init);
+
 /**
  * __fwnode_link_add - Create a link between two fwnode_handles.
  * @con: Consumer end of the link.
@@ -3569,6 +3629,7 @@ static int device_private_init(struct device *dev)
 	klist_init(&dev->p->klist_children, klist_children_get,
 		   klist_children_put);
 	INIT_LIST_HEAD(&dev->p->deferred_probe);
+	dev->p->shutdown_after = 0;
 	return 0;
 }
 
@@ -4819,6 +4880,23 @@ int device_change_owner(struct device *dev, kuid_t kuid, kgid_t kgid)
 }
 EXPORT_SYMBOL_GPL(device_change_owner);
 
+static ASYNC_DOMAIN(sd_domain);
+
+static bool async_shutdown_allowed(struct device *dev)
+{
+	if (!dev->driver)
+		return false;
+
+	switch (async_device_shutdown_enabled) {
+	case ASYNC_DEV_SHUTDOWN_ENABLED:
+		return !(dev->driver->shutdown_type == SHUTDOWN_FORCE_SYNCHRONOUS);
+	case ASYNC_DEV_SHUTDOWN_SAFE:
+		return (dev->driver->shutdown_type == SHUTDOWN_PREFER_ASYNCHRONOUS);
+	default:
+		return false;
+	}
+}
+
 static void shutdown_one_device(struct device *dev)
 {
 	/* hold lock to avoid race with probe/release */
@@ -4854,12 +4932,30 @@ static void shutdown_one_device(struct device *dev)
 		put_device(dev->parent);
 }
 
+/**
+ * shutdown_one_device_async
+ * @data: the pointer to the struct device to be shutdown
+ * @cookie: not used
+ *
+ * Shuts down one device, after waiting for dev's shutdown_after to
+ * complete first.
+ */
+static void shutdown_one_device_async(void *data, async_cookie_t cookie)
+{
+	struct device *dev = data;
+
+	async_synchronize_cookie_domain(dev->p->shutdown_after + 1, &sd_domain);
+
+	shutdown_one_device(dev);
+}
+
 /**
  * device_shutdown - call ->shutdown() on each device to shutdown.
  */
 void device_shutdown(void)
 {
 	struct device *dev, *parent;
+	async_cookie_t cookie = 0;
 
 	wait_for_device_probe();
 	device_block_probing();
@@ -4890,11 +4986,42 @@ void device_shutdown(void)
 		list_del_init(&dev->kobj.entry);
 		spin_unlock(&devices_kset->list_lock);
 
-		shutdown_one_device(dev);
+		if (async_device_shutdown_enabled) {
+			struct device_link *link;
+			int idx;
+
+			/*
+			 * Wait for previous device to shut down if synchronous
+			 */
+			if (!async_shutdown_allowed(dev))
+				dev->p->shutdown_after = cookie;
+
+			get_device(dev);
+			get_device(parent);
+
+			cookie = async_schedule_domain(shutdown_one_device_async,
+						       dev, &sd_domain);
+			/*
+			 * Ensure parent & suppliers wait for this device to shut down
+			 */
+			if (parent) {
+				parent->p->shutdown_after = cookie;
+				put_device(parent);
+			}
+
+			idx = device_links_read_lock();
+			list_for_each_entry_rcu(link, &dev->links.suppliers, c_node,
+					device_links_read_lock_held())
+				link->supplier->p->shutdown_after = cookie;
+			device_links_read_unlock(idx);
+			put_device(dev);
+		} else
+			shutdown_one_device(dev);
 
 		spin_lock(&devices_kset->list_lock);
 	}
 	spin_unlock(&devices_kset->list_lock);
+	async_synchronize_full_domain(&sd_domain);
 }
 
 /*
diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h
index 7738f458995f..f414c8a6f814 100644
--- a/include/linux/device/driver.h
+++ b/include/linux/device/driver.h
@@ -48,6 +48,12 @@ enum probe_type {
 	PROBE_FORCE_SYNCHRONOUS,
 };
 
+enum shutdown_type {
+	SHUTDOWN_DEFAULT_STRATEGY,
+	SHUTDOWN_PREFER_ASYNCHRONOUS,
+	SHUTDOWN_FORCE_SYNCHRONOUS,
+};
+
 /**
  * struct device_driver - The basic device driver structure
  * @name:	Name of the device driver.
@@ -56,6 +62,7 @@ enum probe_type {
  * @mod_name:	Used for built-in modules.
  * @suppress_bind_attrs: Disables bind/unbind via sysfs.
  * @probe_type:	Type of the probe (synchronous or asynchronous) to use.
+ * @shutdown_type: Type of the shutdown (synchronous or asynchronous) to use.
  * @of_match_table: The open firmware table.
  * @acpi_match_table: The ACPI match table.
  * @probe:	Called to query the existence of a specific device,
@@ -102,6 +109,7 @@ struct device_driver {
 
 	bool suppress_bind_attrs;	/* disables bind/unbind via sysfs */
 	enum probe_type probe_type;
+	enum shutdown_type shutdown_type;
 
 	const struct of_device_id	*of_match_table;
 	const struct acpi_device_id	*acpi_match_table;
-- 
2.39.3

Re: [PATCH v6 3/4] driver core: shut down devices asynchronously

[Christoph Hellwig]

On Thu, May 16, 2024 at 10:49:19AM -0500, Stuart Hayes wrote:
> Add /sys/kernel/async_shutdown to allow user control of this feature:
> 
>   safe: shut down all devices synchronously, unless driver prefers async
> 	shutdown (driver opt-in) (default)
>   on:	shut down all devices asynchronously, unless disabled by the driver
> 	(driver opt-out)
>   off:	shut down all devices synchronously

The on option seems very odd.  IMHO safe is the only really sensible
option, and maybe we have to support off as a bandaid due to userspace
behavior dependent on synchronous shutdown, but I'd rather try even
without that first.

Re: [PATCH v6 3/4] driver core: shut down devices asynchronously

[stuart hayes]

On 5/28/2024 1:31 AM, Christoph Hellwig wrote:
> On Thu, May 16, 2024 at 10:49:19AM -0500, Stuart Hayes wrote:
>> Add /sys/kernel/async_shutdown to allow user control of this feature:
>>
>>    safe: shut down all devices synchronously, unless driver prefers async
>> 	shutdown (driver opt-in) (default)
>>    on:	shut down all devices asynchronously, unless disabled by the driver
>> 	(driver opt-out)
>>    off:	shut down all devices synchronously
> 
> The on option seems very odd.  IMHO safe is the only really sensible
> option, and maybe we have to support off as a bandaid due to userspace
> behavior dependent on synchronous shutdown, but I'd rather try even
> without that first.
> 

I added the option because of comments from Greg K-H on the v4 submission
of this patch--see

https://lore.kernel.org/lkml/2023102151-rejoicing-studio-6126@gregkh/T/#m5d0459480bc0fda4563040cab2036839bcbb79a8).

I thought it would be nice to have the option for testing, even if it gets
removed later, but I'll certainly remove it now if necessary.

Re: [PATCH v6 3/4] driver core: shut down devices asynchronously

[Greg Kroah-Hartman]

On Tue, May 28, 2024 at 10:35:27PM -0500, stuart hayes wrote:
> 
> 
> On 5/28/2024 1:31 AM, Christoph Hellwig wrote:
> > On Thu, May 16, 2024 at 10:49:19AM -0500, Stuart Hayes wrote:
> > > Add /sys/kernel/async_shutdown to allow user control of this feature:
> > > 
> > >    safe: shut down all devices synchronously, unless driver prefers async
> > > 	shutdown (driver opt-in) (default)
> > >    on:	shut down all devices asynchronously, unless disabled by the driver
> > > 	(driver opt-out)
> > >    off:	shut down all devices synchronously
> > 
> > The on option seems very odd.  IMHO safe is the only really sensible
> > option, and maybe we have to support off as a bandaid due to userspace
> > behavior dependent on synchronous shutdown, but I'd rather try even
> > without that first.
> > 
> 
> I added the option because of comments from Greg K-H on the v4 submission
> of this patch--see
> 
> https://lore.kernel.org/lkml/2023102151-rejoicing-studio-6126@gregkh/T/#m5d0459480bc0fda4563040cab2036839bcbb79a8).
> 
> I thought it would be nice to have the option for testing, even if it gets
> removed later, but I'll certainly remove it now if necessary.

Opt-in is the requirement here, that's all I asked for.  The "on" can
probably be removed, and by doing that, you can make this option simpler
as well.

thanks,

greg k-h

Re: [PATCH v6 3/4] driver core: shut down devices asynchronously

[Keith Busch]

On Thu, May 16, 2024 at 10:49:19AM -0500, Stuart Hayes wrote:
> Add code to shut down devices asynchronously, while ensuring that each
> device is shut down before its parents & suppliers, and allowing devices
> that share a driver to be shutdown one at a time if necessary.
> 
> Add /sys/kernel/async_shutdown to allow user control of this feature:
> 
>   safe: shut down all devices synchronously, unless driver prefers async
> 	shutdown (driver opt-in) (default)
>   on:	shut down all devices asynchronously, unless disabled by the driver
> 	(driver opt-out)
>   off:	shut down all devices synchronously
> 
> Add async_shutdown to struct device_driver, and expose it via sysfs.
> This will be used to view or change driver opt-in/opt-out of asynchronous
> shutdown, if it is globally enabled.
> 
>  async:   driver opt-in to async device shutdown (devices will be shut down
>           asynchronously if async_shutdown is "on" or "safe")
>  sync:    driver opt-out of async device shutdown (devices will always be
>           shut down synchronously)
>  default: devices will be shutdown asynchronously if async_shutdown is "on"
> 
> This can dramatically reduce system shutdown/reboot time on systems that
> have multiple devices that take many seconds to shut down (like certain
> NVMe drives). On one system tested, the shutdown time went from 11 minutes
> without this patch to 55 seconds with the patch.

I've successfully tested this out on a few systems, and noticing a very
decent shutdown time on my nvme systems. I also like the current
solution here, as the two-pass method was harder to follow.

So I think just remove the extra options that Christoph mentioned and
always use the driver's preferred shutdown method, then this would all
look good to me.

Re: [PATCH v6 3/4] driver core: shut down devices asynchronously

[Jeremy Allison]

On Wed, Jun 12, 2024 at 02:55:11PM -0600, Keith Busch wrote:
>On Thu, May 16, 2024 at 10:49:19AM -0500, Stuart Hayes wrote:
>> Add code to shut down devices asynchronously, while ensuring that each
>> device is shut down before its parents & suppliers, and allowing devices
>> that share a driver to be shutdown one at a time if necessary.
>>
>> Add /sys/kernel/async_shutdown to allow user control of this feature:
>>
>>   safe: shut down all devices synchronously, unless driver prefers async
>> 	shutdown (driver opt-in) (default)
>>   on:	shut down all devices asynchronously, unless disabled by the driver
>> 	(driver opt-out)
>>   off:	shut down all devices synchronously
>>
>> Add async_shutdown to struct device_driver, and expose it via sysfs.
>> This will be used to view or change driver opt-in/opt-out of asynchronous
>> shutdown, if it is globally enabled.
>>
>>  async:   driver opt-in to async device shutdown (devices will be shut down
>>           asynchronously if async_shutdown is "on" or "safe")
>>  sync:    driver opt-out of async device shutdown (devices will always be
>>           shut down synchronously)
>>  default: devices will be shutdown asynchronously if async_shutdown is "on"
>>
>> This can dramatically reduce system shutdown/reboot time on systems that
>> have multiple devices that take many seconds to shut down (like certain
>> NVMe drives). On one system tested, the shutdown time went from 11 minutes
>> without this patch to 55 seconds with the patch.
>
>I've successfully tested this out on a few systems, and noticing a very
>decent shutdown time on my nvme systems. I also like the current
>solution here, as the two-pass method was harder to follow.
>
>So I think just remove the extra options that Christoph mentioned and
>always use the driver's preferred shutdown method, then this would all
>look good to me.

Yes, I have tested this patch on my systems and am greatly in
favour of this instead of the two-pass version I was trying
to make work. It is easy to understand and fixes the problem
for my NVME issue.

Jeremy.

[PATCH v6 4/4] nvme-pci: Make driver prefer asynchronous shutdown

[Stuart Hayes]

Set the driver default to prefer asynchronous shutdown.

Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
Signed-off-by: David Jeffery <djeffery@redhat.com>
---
 drivers/nvme/host/pci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 710043086dff..df7bd88f50f2 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -3530,6 +3530,7 @@ static struct pci_driver nvme_driver = {
 	.shutdown	= nvme_shutdown,
 	.driver		= {
 		.probe_type	= PROBE_PREFER_ASYNCHRONOUS,
+		.shutdown_type	= SHUTDOWN_PREFER_ASYNCHRONOUS,
 #ifdef CONFIG_PM_SLEEP
 		.pm		= &nvme_dev_pm_ops,
 #endif
-- 
2.39.3