[v2] driver core: shut down devices asynchronously
Commit Message
Shut down devices asynchronously, ensuring that each device is shut down
before its parents.
This can dramatically reduce system shutdown/reboot time on systems that
have devices that take many seconds to shut down, such as some 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>
---
v1->v2: rewritten using kernel async code (suggested by Lukas Wunner)
---
drivers/base/base.h | 2 +
drivers/base/core.c | 135 ++++++++++++++++++++++++------------------
include/linux/async.h | 6 ++
3 files changed, 87 insertions(+), 56 deletions(-)
Comments
Hi Stuart,
kernel test robot noticed the following build warnings:
[auto build test WARNING on driver-core/driver-core-testing]
[also build test WARNING on driver-core/driver-core-next driver-core/driver-core-linus linus/master v6.6-rc1 next-20230913]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Stuart-Hayes/driver-core-shut-down-devices-asynchronously/20230914-050611
base: driver-core/driver-core-testing
patch link: https://lore.kernel.org/r/20230913210516.3545-1-stuart.w.hayes%40gmail.com
patch subject: [PATCH v2] driver core: shut down devices asynchronously
config: parisc-allyesconfig (https://download.01.org/0day-ci/archive/20230914/202309141407.CpZtIa2w-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230914/202309141407.CpZtIa2w-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309141407.CpZtIa2w-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/base/core.c:68: warning: Function parameter or member 'flags' not described in '__fwnode_link_add'
>> drivers/base/core.c:4771: warning: Function parameter or member 'dev' not described in 'shutdown_device_and_children'
>> drivers/base/core.c:4771: warning: Function parameter or member 'data' not described in 'shutdown_device_and_children'
>> drivers/base/core.c:4771: warning: expecting prototype for device_shutdown_async(). Prototype was for shutdown_device_and_children() instead
vim +4771 drivers/base/core.c
4764
4765 /**
4766 * device_shutdown_async - schedule ->shutdown() on each device to shutdown
4767 * asynchronously, ensuring each device's children are shut down before
4768 * shutting down the device
4769 */
4770 static int shutdown_device_and_children(struct device *dev, void *data)
> 4771 {
4772 struct async_domain *domain = data;
4773 struct device_private *p = dev->p;
4774
4775 INIT_ASYNC_DOMAIN_EXCLUSIVE(&p->children_shutdown);
4776
4777 /* make sure device doesn't go away before it is shut down */
4778 get_device(dev);
4779
4780 /* schedule shutdown of children */
4781 device_for_each_child(dev, &p->children_shutdown,
4782 shutdown_device_and_children);
4783
4784 /* schedule shutdown of this device */
4785 async_schedule_domain(shutdown_device, dev, domain);
4786
4787 return 0;
4788 }
4789
On 9/13/2023 4:05 PM, Stuart Hayes wrote:
> Shut down devices asynchronously, ensuring that each device is shut down
> before its parents.
>
> This can dramatically reduce system shutdown/reboot time on systems that
> have devices that take many seconds to shut down, such as some 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>
> ---
> v1->v2: rewritten using kernel async code (suggested by Lukas Wunner)
> ---
>
> drivers/base/base.h | 2 +
> drivers/base/core.c | 135 ++++++++++++++++++++++++------------------
> include/linux/async.h | 6 ++
> 3 files changed, 87 insertions(+), 56 deletions(-)
>
Please disregard this patch... I realized this could be done in a simpler way. I'll send a V3 this week.
@@ -11,6 +11,7 @@
*
*/
#include <linux/notifier.h>
+#include <linux/async.h>
/**
* struct subsys_private - structure to hold the private to the driver core portions of the bus_type/class structure.
@@ -114,6 +115,7 @@ struct device_private {
struct list_head deferred_probe;
struct device_driver *async_driver;
char *deferred_probe_reason;
+ struct async_domain children_shutdown;
struct device *device;
u8 dead:1;
};
@@ -9,6 +9,7 @@
*/
#include <linux/acpi.h>
+#include <linux/async.h>
#include <linux/cpufreq.h>
#include <linux/device.h>
#include <linux/err.h>
@@ -4716,76 +4717,98 @@ int device_change_owner(struct device *dev, kuid_t kuid, kgid_t kgid)
}
EXPORT_SYMBOL_GPL(device_change_owner);
+static void shutdown_device(void *data, async_cookie_t cookie)
+{
+ struct device *dev = data;
+
+ async_synchronize_full_domain(&dev->p->children_shutdown);
+
+ /*
+ * Make sure the device is off the kset list, in the
+ * event that dev->*->shutdown() doesn't remove it.
+ */
+ spin_lock(&devices_kset->list_lock);
+ list_del_init(&dev->kobj.entry);
+ spin_unlock(&devices_kset->list_lock);
+
+ /* hold lock to avoid race with probe/release */
+ if (dev->parent)
+ 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)
+ device_unlock(dev->parent);
+
+ put_device(dev);
+}
+
+/**
+ * device_shutdown_async - schedule ->shutdown() on each device to shutdown
+ * asynchronously, ensuring each device's children are shut down before
+ * shutting down the device
+ */
+static int shutdown_device_and_children(struct device *dev, void *data)
+{
+ struct async_domain *domain = data;
+ struct device_private *p = dev->p;
+
+ INIT_ASYNC_DOMAIN_EXCLUSIVE(&p->children_shutdown);
+
+ /* make sure device doesn't go away before it is shut down */
+ get_device(dev);
+
+ /* schedule shutdown of children */
+ device_for_each_child(dev, &p->children_shutdown,
+ shutdown_device_and_children);
+
+ /* schedule shutdown of this device */
+ async_schedule_domain(shutdown_device, dev, domain);
+
+ return 0;
+}
+
/**
* device_shutdown - call ->shutdown() on each device to shutdown.
*/
void device_shutdown(void)
{
- struct device *dev, *parent;
+ ASYNC_DOMAIN_EXCLUSIVE(shutdown_domain);
+ struct device *dev, *temp;
wait_for_device_probe();
device_block_probing();
cpufreq_suspend();
- spin_lock(&devices_kset->list_lock);
/*
- * Walk the devices list backward, shutting down each in turn.
- * Beware that device unplug events may also start pulling
- * devices offline, even as the system is shutting down.
+ * schedule shutdown of top level devices
*/
- while (!list_empty(&devices_kset->list)) {
- dev = list_entry(devices_kset->list.prev, struct device,
- kobj.entry);
-
- /*
- * hold reference count of device's parent to
- * prevent it from being freed because parent's
- * lock is to be held
- */
- parent = get_device(dev->parent);
- get_device(dev);
- /*
- * Make sure the device is off the kset list, in the
- * event that dev->*->shutdown() doesn't remove it.
- */
- list_del_init(&dev->kobj.entry);
- spin_unlock(&devices_kset->list_lock);
-
- /* hold lock to avoid race with probe/release */
- if (parent)
- 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)
- device_unlock(parent);
-
- put_device(dev);
- put_device(parent);
-
- spin_lock(&devices_kset->list_lock);
+ list_for_each_entry_safe_reverse(dev, temp, &devices_kset->list, kobj.entry) {
+ if (dev->parent)
+ continue;
+ shutdown_device_and_children(dev, &shutdown_domain);
}
- spin_unlock(&devices_kset->list_lock);
+ async_synchronize_full_domain(&shutdown_domain);
}
/*
@@ -35,6 +35,12 @@ struct async_domain {
struct async_domain _name = { .pending = LIST_HEAD_INIT(_name.pending), \
.registered = 0 }
+static inline void INIT_ASYNC_DOMAIN_EXCLUSIVE(struct async_domain *domain)
+{
+ INIT_LIST_HEAD(&domain->pending);
+ domain->registered = 0;
+}
+
async_cookie_t async_schedule_node(async_func_t func, void *data,
int node);
async_cookie_t async_schedule_node_domain(async_func_t func, void *data,