public inbox for driver-core@lists.linux.dev
 help / color / mirror / Atom feed
* [PATCH 1/5] driver core: do not always lock parent in shutdown
@ 2026-03-11 17:12 David Jeffery
  2026-03-11 17:12 ` [PATCH 2/5] driver core: separate function to shutdown one device David Jeffery
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: David Jeffery @ 2026-03-11 17:12 UTC (permalink / raw)
  To: linux-kernel, driver-core, linux-pci, linux-scsi,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich
  Cc: Tarun Sahu, Pasha Tatashin, Michał Cłapiński,
	Jordan Richards, Ewan Milne, John Meneghini, Lombardi, Maurizio,
	David Jeffery, Stuart Hayes, Laurence Oberman

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>
Tested-by: Laurence Oberman <loberman@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 791f9e444df8..eb54fc9680de 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -4819,7 +4819,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);
 
@@ -4843,7 +4843,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.53.0


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

* [PATCH 2/5] driver core: separate function to shutdown one device
  2026-03-11 17:12 [PATCH 1/5] driver core: do not always lock parent in shutdown David Jeffery
@ 2026-03-11 17:12 ` David Jeffery
  2026-03-11 18:00   ` Bart Van Assche
                     ` (2 more replies)
  2026-03-11 17:12 ` [PATCH 3/5] driver core: async device shutdown infrastructure David Jeffery
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 18+ messages in thread
From: David Jeffery @ 2026-03-11 17:12 UTC (permalink / raw)
  To: linux-kernel, driver-core, linux-pci, linux-scsi,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich
  Cc: Tarun Sahu, Pasha Tatashin, Michał Cłapiński,
	Jordan Richards, Ewan Milne, John Meneghini, Lombardi, Maurizio,
	David Jeffery, Stuart Hayes, Laurence Oberman

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>
Tested-by: Laurence Oberman <loberman@redhat.com>
---
 drivers/base/core.c | 65 ++++++++++++++++++++++++---------------------
 1 file changed, 35 insertions(+), 30 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index eb54fc9680de..c2c35f95f751 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -4782,6 +4782,40 @@ int device_change_owner(struct device *dev, kuid_t kuid, kgid_t kgid)
 	return error;
 }
 
+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->parent);
+	put_device(dev);
+}
+
 /**
  * device_shutdown - call ->shutdown() on each device to shutdown.
  */
@@ -4818,36 +4852,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.53.0


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

* [PATCH 3/5] driver core: async device shutdown infrastructure
  2026-03-11 17:12 [PATCH 1/5] driver core: do not always lock parent in shutdown David Jeffery
  2026-03-11 17:12 ` [PATCH 2/5] driver core: separate function to shutdown one device David Jeffery
@ 2026-03-11 17:12 ` David Jeffery
  2026-03-11 19:40   ` Randy Dunlap
  2026-03-11 23:05   ` Bjorn Helgaas
  2026-03-11 17:12 ` [PATCH 4/5] pci: enable async shutdown support David Jeffery
  2026-03-11 17:12 ` [PATCH 5/5] scsi: " David Jeffery
  3 siblings, 2 replies; 18+ messages in thread
From: David Jeffery @ 2026-03-11 17:12 UTC (permalink / raw)
  To: linux-kernel, driver-core, linux-pci, linux-scsi,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich
  Cc: Tarun Sahu, Pasha Tatashin, Michał Cłapiński,
	Jordan Richards, Ewan Milne, John Meneghini, Lombardi, Maurizio,
	David Jeffery, Stuart Hayes, Laurence Oberman

Patterned after async suspend, allow devices to mark themselves as wanting
to perform async shutdown. Devices using async shutdown wait only for their
dependencies to shutdown before executing their shutdown routine.

Sync shutdown devices are shut down one at a time and will only wait for an
async shutdown device if the async device is a dependency.

Signed-off-by: David Jeffery <djeffery@redhat.com>
Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
Tested-by: Laurence Oberman <loberman@redhat.com>
---
 drivers/base/base.h    |   2 +
 drivers/base/core.c    | 104 ++++++++++++++++++++++++++++++++++++++++-
 include/linux/device.h |  13 ++++++
 3 files changed, 118 insertions(+), 1 deletion(-)

diff --git a/drivers/base/base.h b/drivers/base/base.h
index 79d031d2d845..ea2a039e7907 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -113,6 +113,7 @@ struct driver_type {
  * @device - pointer back to the struct device that this structure is
  * associated with.
  * @driver_type - The type of the bound Rust driver.
+ * @complete - completion for device shutdown ordering
  * @dead - This device is currently either in the process of or has been
  *	removed from the system. Any asynchronous events scheduled for this
  *	device should exit without taking any action.
@@ -132,6 +133,7 @@ struct device_private {
 #ifdef CONFIG_RUST
 	struct driver_type driver_type;
 #endif
+	struct completion complete;
 	u8 dead:1;
 };
 #define to_device_private_parent(obj)	\
diff --git a/drivers/base/core.c b/drivers/base/core.c
index c2c35f95f751..07f564eb5823 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/blkdev.h>
 #include <linux/cleanup.h>
 #include <linux/cpufreq.h>
@@ -37,6 +38,10 @@
 #include "physical_location.h"
 #include "power/power.h"
 
+static bool async_shutdown = true;
+module_param(async_shutdown, bool, 0644);
+MODULE_PARM_DESC(async_shutdown, "Enable asynchronous device shutdown support");
+
 /* Device links support. */
 static LIST_HEAD(deferred_sync);
 static unsigned int defer_sync_state_count = 1;
@@ -3538,6 +3543,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);
+	init_completion(&dev->p->complete);
 	return 0;
 }
 
@@ -4782,6 +4788,37 @@ int device_change_owner(struct device *dev, kuid_t kuid, kgid_t kgid)
 	return error;
 }
 
+static bool wants_async_shutdown(struct device *dev)
+{
+	return async_shutdown && dev->async_shutdown;
+}
+
+static int wait_for_device_shutdown(struct device *dev, void *data)
+{
+	bool async = *(bool *)data;
+
+	if (async || wants_async_shutdown(dev))
+		wait_for_completion(&dev->p->complete);
+
+	return 0;
+}
+
+static void wait_for_shutdown_dependencies(struct device *dev, bool async)
+{
+	struct device_link *link;
+	int idx;
+
+	device_for_each_child(dev, &async, wait_for_device_shutdown);
+
+	idx = device_links_read_lock();
+
+	dev_for_each_link_to_consumer(link, dev)
+		if (!device_link_flag_is_sync_state_only(link->flags))
+			wait_for_device_shutdown(link->consumer, &async);
+
+	device_links_read_unlock(idx);
+}
+
 static void shutdown_one_device(struct device *dev)
 {
 	/* hold lock to avoid race with probe/release */
@@ -4808,6 +4845,8 @@ static void shutdown_one_device(struct device *dev)
 		dev->driver->shutdown(dev);
 	}
 
+	complete_all(&dev->p->complete);
+
 	device_unlock(dev);
 	if (dev->parent && dev->bus && dev->bus->need_parent_lock)
 		device_unlock(dev->parent);
@@ -4816,6 +4855,58 @@ static void shutdown_one_device(struct device *dev)
 	put_device(dev);
 }
 
+static void async_shutdown_handler(void *data, async_cookie_t cookie)
+{
+	struct device *dev = data;
+
+	wait_for_shutdown_dependencies(dev, true);
+	shutdown_one_device(dev);
+}
+
+static bool shutdown_device_async(struct device *dev)
+{
+	if (async_schedule_dev_nocall(async_shutdown_handler, dev))
+		return true;
+	return false;
+}
+
+
+static void early_async_shutdown_devices(void)
+{
+	struct device *dev, *next, *needs_put = NULL;
+
+	if (!async_shutdown)
+		return;
+
+	spin_lock(&devices_kset->list_lock);
+
+	list_for_each_entry_safe_reverse(dev, next, &devices_kset->list,
+					 kobj.entry) {
+		if (wants_async_shutdown(dev)) {
+			get_device(dev->parent);
+			get_device(dev);
+
+			if (shutdown_device_async(dev)) {
+				list_del_init(&dev->kobj.entry);
+			} else {
+				/*
+				 * async failed, clean up extra references
+				 * and run from the standard shutdown loop
+				 */
+				needs_put = dev;
+				break;
+			}
+		}
+	}
+
+	spin_unlock(&devices_kset->list_lock);
+
+	if (needs_put) {
+		put_device(needs_put->parent);
+		put_device(needs_put);
+	}
+}
+
 /**
  * device_shutdown - call ->shutdown() on each device to shutdown.
  */
@@ -4828,6 +4919,12 @@ void device_shutdown(void)
 
 	cpufreq_suspend();
 
+	/*
+	 * Start async device threads where possible to maximize potential
+	 * paralellism and minimize false dependency on unrelated sync devices
+	 */
+	early_async_shutdown_devices();
+
 	spin_lock(&devices_kset->list_lock);
 	/*
 	 * Walk the devices list backward, shutting down each in turn.
@@ -4852,11 +4949,16 @@ void device_shutdown(void)
 		list_del_init(&dev->kobj.entry);
 		spin_unlock(&devices_kset->list_lock);
 
-		shutdown_one_device(dev);
+		if (!wants_async_shutdown(dev) || !shutdown_device_async(dev)) {
+			wait_for_shutdown_dependencies(dev, false);
+			shutdown_one_device(dev);
+		}
 
 		spin_lock(&devices_kset->list_lock);
 	}
 	spin_unlock(&devices_kset->list_lock);
+
+	async_synchronize_full();
 }
 
 /*
diff --git a/include/linux/device.h b/include/linux/device.h
index 0be95294b6e6..da1db7d235c9 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -551,6 +551,8 @@ struct device_physical_location {
  * @dma_skip_sync: DMA sync operations can be skipped for coherent buffers.
  * @dma_iommu: Device is using default IOMMU implementation for DMA and
  *		doesn't rely on dma_ops structure.
+ * @async_shutdown: Device shutdown may be run asynchronously and in parallel
+ *		to the shutdown of unrelated devices
  *
  * At the lowest level, every device in a Linux system is represented by an
  * instance of struct device. The device structure contains the information
@@ -669,6 +671,7 @@ struct device {
 #ifdef CONFIG_IOMMU_DMA
 	bool			dma_iommu:1;
 #endif
+	bool			async_shutdown:1;
 };
 
 /**
@@ -824,6 +827,16 @@ static inline bool device_async_suspend_enabled(struct device *dev)
 	return !!dev->power.async_suspend;
 }
 
+static inline bool device_enable_async_shutdown(struct device *dev)
+{
+	return dev->async_shutdown = true;
+}
+
+static inline bool device_async_shutdown_enabled(struct device *dev)
+{
+	return !!dev->async_shutdown;
+}
+
 static inline bool device_pm_not_required(struct device *dev)
 {
 	return dev->power.no_pm;
-- 
2.53.0


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

* [PATCH 4/5] pci: enable async shutdown support
  2026-03-11 17:12 [PATCH 1/5] driver core: do not always lock parent in shutdown David Jeffery
  2026-03-11 17:12 ` [PATCH 2/5] driver core: separate function to shutdown one device David Jeffery
  2026-03-11 17:12 ` [PATCH 3/5] driver core: async device shutdown infrastructure David Jeffery
@ 2026-03-11 17:12 ` David Jeffery
  2026-03-11 23:08   ` Bjorn Helgaas
  2026-03-12  5:09   ` Greg Kroah-Hartman
  2026-03-11 17:12 ` [PATCH 5/5] scsi: " David Jeffery
  3 siblings, 2 replies; 18+ messages in thread
From: David Jeffery @ 2026-03-11 17:12 UTC (permalink / raw)
  To: linux-kernel, driver-core, linux-pci, linux-scsi,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich
  Cc: Tarun Sahu, Pasha Tatashin, Michał Cłapiński,
	Jordan Richards, Ewan Milne, John Meneghini, Lombardi, Maurizio,
	David Jeffery, Stuart Hayes, Laurence Oberman

Like its async suspend support, allow pci device shutdown to be performed
asynchronously to improve shutdown time.

Signed-off-by: David Jeffery <djeffery@redhat.com>
Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
Tested-by: Laurence Oberman <loberman@redhat.com>
---
 drivers/pci/probe.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index bccc7a4bdd79..4d98bab2163d 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1040,6 +1040,7 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge)
 
 	bus->bridge = get_device(&bridge->dev);
 	device_enable_async_suspend(bus->bridge);
+	device_enable_async_shutdown(bus->bridge);
 	pci_set_bus_of_node(bus);
 	pci_set_bus_msi_domain(bus);
 	if (bridge->msi_domain && !dev_get_msi_domain(&bus->dev) &&
@@ -2749,6 +2750,7 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
 	pci_reassigndev_resource_alignment(dev);
 
 	pci_init_capabilities(dev);
+	device_enable_async_shutdown(&dev->dev);
 
 	/*
 	 * Add the device to our list of discovered devices
-- 
2.53.0


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

* [PATCH 5/5] scsi: enable async shutdown support
  2026-03-11 17:12 [PATCH 1/5] driver core: do not always lock parent in shutdown David Jeffery
                   ` (2 preceding siblings ...)
  2026-03-11 17:12 ` [PATCH 4/5] pci: enable async shutdown support David Jeffery
@ 2026-03-11 17:12 ` David Jeffery
  3 siblings, 0 replies; 18+ messages in thread
From: David Jeffery @ 2026-03-11 17:12 UTC (permalink / raw)
  To: linux-kernel, driver-core, linux-pci, linux-scsi,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich
  Cc: Tarun Sahu, Pasha Tatashin, Michał Cłapiński,
	Jordan Richards, Ewan Milne, John Meneghini, Lombardi, Maurizio,
	David Jeffery, Stuart Hayes, Laurence Oberman

Like scsi's async suspend support, allow scsi devices to be shut down
asynchronously to improve system shutdown time.

Signed-off-by: David Jeffery <djeffery@redhat.com>
Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
Tested-by: Laurence Oberman <loberman@redhat.com>
---
 drivers/scsi/hosts.c      | 3 +++
 drivers/scsi/scsi_scan.c  | 1 +
 drivers/scsi/scsi_sysfs.c | 4 ++++
 3 files changed, 8 insertions(+)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index e047747d4ecf..dfb38a82c0bf 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -273,6 +273,7 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct device *dev,
 	pm_runtime_set_active(&shost->shost_gendev);
 	pm_runtime_enable(&shost->shost_gendev);
 	device_enable_async_suspend(&shost->shost_gendev);
+	device_enable_async_shutdown(&shost->shost_gendev);
 
 	error = device_add(&shost->shost_gendev);
 	if (error)
@@ -282,6 +283,7 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct device *dev,
 	get_device(shost->shost_gendev.parent);
 
 	device_enable_async_suspend(&shost->shost_dev);
+	device_enable_async_shutdown(&shost->shost_dev);
 
 	get_device(&shost->shost_gendev);
 	error = device_add(&shost->shost_dev);
@@ -519,6 +521,7 @@ struct Scsi_Host *scsi_host_alloc(const struct scsi_host_template *sht, int priv
 	shost->shost_gendev.bus = &scsi_bus_type;
 	shost->shost_gendev.type = &scsi_host_type;
 	scsi_enable_async_suspend(&shost->shost_gendev);
+	device_enable_async_shutdown(&shost->shost_gendev);
 
 	device_initialize(&shost->shost_dev);
 	shost->shost_dev.parent = &shost->shost_gendev;
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 60c06fa4ec32..c42b33e8ea8a 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -518,6 +518,7 @@ static struct scsi_target *scsi_alloc_target(struct device *parent,
 	dev->bus = &scsi_bus_type;
 	dev->type = &scsi_target_type;
 	scsi_enable_async_suspend(dev);
+	device_enable_async_shutdown(dev);
 	starget->id = id;
 	starget->channel = channel;
 	starget->can_queue = 0;
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 6b8c5c05f294..c76ba17b206f 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1370,6 +1370,7 @@ static int scsi_target_add(struct scsi_target *starget)
 	pm_runtime_set_active(&starget->dev);
 	pm_runtime_enable(&starget->dev);
 	device_enable_async_suspend(&starget->dev);
+	device_enable_async_shutdown(&starget->dev);
 
 	return 0;
 }
@@ -1396,6 +1397,7 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
 	transport_configure_device(&starget->dev);
 
 	device_enable_async_suspend(&sdev->sdev_gendev);
+	device_enable_async_shutdown(&sdev->sdev_gendev);
 	scsi_autopm_get_target(starget);
 	pm_runtime_set_active(&sdev->sdev_gendev);
 	if (!sdev->rpm_autosuspend)
@@ -1415,6 +1417,7 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
 	}
 
 	device_enable_async_suspend(&sdev->sdev_dev);
+	device_enable_async_shutdown(&sdev->sdev_dev);
 	error = device_add(&sdev->sdev_dev);
 	if (error) {
 		sdev_printk(KERN_INFO, sdev,
@@ -1670,6 +1673,7 @@ void scsi_sysfs_device_initialize(struct scsi_device *sdev)
 	sdev->sdev_gendev.bus = &scsi_bus_type;
 	sdev->sdev_gendev.type = &scsi_dev_type;
 	scsi_enable_async_suspend(&sdev->sdev_gendev);
+	device_enable_async_shutdown(&sdev->sdev_gendev);
 	dev_set_name(&sdev->sdev_gendev, "%d:%d:%d:%llu",
 		     sdev->host->host_no, sdev->channel, sdev->id, sdev->lun);
 	sdev->sdev_gendev.groups = hostt->sdev_groups;
-- 
2.53.0


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

* Re: [PATCH 2/5] driver core: separate function to shutdown one device
  2026-03-11 17:12 ` [PATCH 2/5] driver core: separate function to shutdown one device David Jeffery
@ 2026-03-11 18:00   ` Bart Van Assche
  2026-03-11 21:37     ` Bjorn Helgaas
  2026-03-12 13:39     ` David Jeffery
  2026-03-12  1:49   ` kernel test robot
  2026-03-12  5:10   ` kernel test robot
  2 siblings, 2 replies; 18+ messages in thread
From: Bart Van Assche @ 2026-03-11 18:00 UTC (permalink / raw)
  To: David Jeffery, linux-kernel, driver-core, linux-pci, linux-scsi,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich
  Cc: Tarun Sahu, Pasha Tatashin, Michał Cłapiński,
	Jordan Richards, Ewan Milne, John Meneghini, Lombardi, Maurizio,
	Stuart Hayes, Laurence Oberman, Marco Elver

On 3/11/26 10:12 AM, David Jeffery wrote:
> +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->parent);
> +	put_device(dev);
> +}

Please keep the following code in the caller:

	if (dev->parent && dev->bus && dev->bus->need_parent_lock)
		device_lock(dev->parent);

	if (dev->parent && dev->bus && dev->bus->need_parent_lock)
		device_unlock(dev->parent);

	put_device(dev->parent);
	put_device(dev);

Additionally, please make sure that the caller is made compatible with
lock context analysis (see also
https://lore.kernel.org/all/20250206181711.1902989-1-elver@google.com/).
All that is required to make this code compatible with lock context
analysis is to organize it as follows:

	if (dev->parent && dev->bus && dev->bus->need_parent_lock) {
		device_lock(dev->parent);
		shutdown_one_device(dev);
		device_unlock(dev->parent);
	} else {
		shutdown_one_device(dev);
	}

Thanks,

Bart.

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

* Re: [PATCH 3/5] driver core: async device shutdown infrastructure
  2026-03-11 17:12 ` [PATCH 3/5] driver core: async device shutdown infrastructure David Jeffery
@ 2026-03-11 19:40   ` Randy Dunlap
  2026-03-11 23:05   ` Bjorn Helgaas
  1 sibling, 0 replies; 18+ messages in thread
From: Randy Dunlap @ 2026-03-11 19:40 UTC (permalink / raw)
  To: David Jeffery, linux-kernel, driver-core, linux-pci, linux-scsi,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich
  Cc: Tarun Sahu, Pasha Tatashin, Michał Cłapiński,
	Jordan Richards, Ewan Milne, John Meneghini, Lombardi, Maurizio,
	Stuart Hayes, Laurence Oberman

Hi,

On 3/11/26 10:12 AM, David Jeffery wrote:
> Patterned after async suspend, allow devices to mark themselves as wanting
> to perform async shutdown. Devices using async shutdown wait only for their
> dependencies to shutdown before executing their shutdown routine.
> 
> Sync shutdown devices are shut down one at a time and will only wait for an
> async shutdown device if the async device is a dependency.
> 
> Signed-off-by: David Jeffery <djeffery@redhat.com>
> Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
> Tested-by: Laurence Oberman <loberman@redhat.com>
> ---
>  drivers/base/base.h    |   2 +
>  drivers/base/core.c    | 104 ++++++++++++++++++++++++++++++++++++++++-
>  include/linux/device.h |  13 ++++++
>  3 files changed, 118 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/base.h b/drivers/base/base.h
> index 79d031d2d845..ea2a039e7907 100644
> --- a/drivers/base/base.h
> +++ b/drivers/base/base.h

I know this isn't directly about this patch, but would you mind adding
descriptions that are currently missing for a couple of struct member fields?

Warning: drivers/base/base.h:59 struct member 'drivers_autoprobe' not described in 'subsys_private'
Warning: drivers/base/base.h:135 struct member 'deferred_probe_reason' not described in 'device_private'

thanks.
-- 
~Randy


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

* Re: [PATCH 2/5] driver core: separate function to shutdown one device
  2026-03-11 18:00   ` Bart Van Assche
@ 2026-03-11 21:37     ` Bjorn Helgaas
  2026-03-11 21:42       ` Bart Van Assche
  2026-03-12 13:39     ` David Jeffery
  1 sibling, 1 reply; 18+ messages in thread
From: Bjorn Helgaas @ 2026-03-11 21:37 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: David Jeffery, linux-kernel, driver-core, linux-pci, linux-scsi,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	Tarun Sahu, Pasha Tatashin, Michał Cłapiński,
	Jordan Richards, Ewan Milne, John Meneghini, Lombardi, Maurizio,
	Stuart Hayes, Laurence Oberman, Marco Elver

On Wed, Mar 11, 2026 at 11:00:11AM -0700, Bart Van Assche wrote:
> On 3/11/26 10:12 AM, David Jeffery wrote:
> > +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->parent);
> > +	put_device(dev);
> > +}
> 
> Please keep the following code in the caller:
> 
> 	if (dev->parent && dev->bus && dev->bus->need_parent_lock)
> 		device_lock(dev->parent);
> 
> 	if (dev->parent && dev->bus && dev->bus->need_parent_lock)
> 		device_unlock(dev->parent);
> 
> 	put_device(dev->parent);
> 	put_device(dev);

Can you elaborate on this a little bit?  I can see that doing this in
the caller is simpler in some ways, although there are two callers
that would need this.  Maybe it's just the lock anti-pattern below?

> Additionally, please make sure that the caller is made compatible with
> lock context analysis (see also
> https://lore.kernel.org/all/20250206181711.1902989-1-elver@google.com/).
> All that is required to make this code compatible with lock context
> analysis is to organize it as follows:
> 
> 	if (dev->parent && dev->bus && dev->bus->need_parent_lock) {
> 		device_lock(dev->parent);
> 		shutdown_one_device(dev);
> 		device_unlock(dev->parent);
> 	} else {
> 		shutdown_one_device(dev);
> 	}

I guess avoiding the "conditional acquisition and later conditional
release" pattern mentioned at [1] is what makes this compatible with
lock context analysis?

I guess this is another way of expressing the "no conditionally held
locks" rule [2], which is more concise and fits better in my pea
brain.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/dev-tools/context-analysis.rst?id=v7.0-rc1#n42
[2] https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#no-conditionally-held-locks

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

* Re: [PATCH 2/5] driver core: separate function to shutdown one device
  2026-03-11 21:37     ` Bjorn Helgaas
@ 2026-03-11 21:42       ` Bart Van Assche
  0 siblings, 0 replies; 18+ messages in thread
From: Bart Van Assche @ 2026-03-11 21:42 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: David Jeffery, linux-kernel, driver-core, linux-pci, linux-scsi,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	Tarun Sahu, Pasha Tatashin, Michał Cłapiński,
	Jordan Richards, Ewan Milne, John Meneghini, Lombardi, Maurizio,
	Stuart Hayes, Laurence Oberman, Marco Elver

On 3/11/26 2:37 PM, Bjorn Helgaas wrote:
> Maybe it's just the lock anti-pattern below?

Yes, that's my only concern.

> I guess avoiding the "conditional acquisition and later conditional
> release" pattern mentioned at [1] is what makes this compatible with
> lock context analysis?

Correct.

> I guess this is another way of expressing the "no conditionally held
> locks" rule [2], which is more concise and fits better in my pea
> brain.
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/dev-tools/context-analysis.rst?id=v7.0-rc1#n42
> [2] https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#no-conditionally-held-locks

Is this perhaps intended as a suggestion for simplifying the language in
Documentation/dev-tools/context-analysis.rst?

s/such as conditional acquisition and later conditional release in the 
same function/no conditionally held locks/

Thanks,

Bart.

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

* Re: [PATCH 3/5] driver core: async device shutdown infrastructure
  2026-03-11 17:12 ` [PATCH 3/5] driver core: async device shutdown infrastructure David Jeffery
  2026-03-11 19:40   ` Randy Dunlap
@ 2026-03-11 23:05   ` Bjorn Helgaas
  2026-03-12 14:01     ` David Jeffery
  1 sibling, 1 reply; 18+ messages in thread
From: Bjorn Helgaas @ 2026-03-11 23:05 UTC (permalink / raw)
  To: David Jeffery
  Cc: linux-kernel, driver-core, linux-pci, linux-scsi,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	Tarun Sahu, Pasha Tatashin, Michał Cłapiński,
	Jordan Richards, Ewan Milne, John Meneghini, Lombardi, Maurizio,
	Stuart Hayes, Laurence Oberman

On Wed, Mar 11, 2026 at 01:12:07PM -0400, David Jeffery wrote:
> Patterned after async suspend, allow devices to mark themselves as wanting
> to perform async shutdown. Devices using async shutdown wait only for their
> dependencies to shutdown before executing their shutdown routine.

I'm not an expert on dependencies.  Is it obvious to everybody else
how these dependencies are expressed?  What would I look at to verify
that, for example, PCI devices are dependencies of the PCI bridges
leading to them?  I suppose it's the same dependencies used for
suspend?

From wait_for_shutdown_dependencies() below, it looks like we'll wait
for each child of dev and then wait for each consumer of dev before
shutting down dev itself.

> Sync shutdown devices are shut down one at a time and will only wait for an
> async shutdown device if the async device is a dependency.

> @@ -132,6 +133,7 @@ struct device_private {
>  #ifdef CONFIG_RUST
>  	struct driver_type driver_type;
>  #endif
> +	struct completion complete;

I thought "complete" might be a little too generic, but I guess async
suspend uses "dev->power.completion" :)

> +static void wait_for_shutdown_dependencies(struct device *dev, bool async)
> +{
> +	struct device_link *link;
> +	int idx;
> +
> +	device_for_each_child(dev, &async, wait_for_device_shutdown);
> +
> +	idx = device_links_read_lock();
> +
> +	dev_for_each_link_to_consumer(link, dev)
> +		if (!device_link_flag_is_sync_state_only(link->flags))
> +			wait_for_device_shutdown(link->consumer, &async);
> +
> +	device_links_read_unlock(idx);
> +}

> @@ -4828,6 +4919,12 @@ void device_shutdown(void)
>  
>  	cpufreq_suspend();
>  
> +	/*
> +	 * Start async device threads where possible to maximize potential
> +	 * paralellism and minimize false dependency on unrelated sync devices

s/paralellism/parallelism/

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

* Re: [PATCH 4/5] pci: enable async shutdown support
  2026-03-11 17:12 ` [PATCH 4/5] pci: enable async shutdown support David Jeffery
@ 2026-03-11 23:08   ` Bjorn Helgaas
  2026-03-12 13:46     ` David Jeffery
  2026-03-12  5:09   ` Greg Kroah-Hartman
  1 sibling, 1 reply; 18+ messages in thread
From: Bjorn Helgaas @ 2026-03-11 23:08 UTC (permalink / raw)
  To: David Jeffery
  Cc: linux-kernel, driver-core, linux-pci, linux-scsi,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	Tarun Sahu, Pasha Tatashin, Michał Cłapiński,
	Jordan Richards, Ewan Milne, John Meneghini, Lombardi, Maurizio,
	Stuart Hayes, Laurence Oberman

In subject, to match history:

  PCI: Enable async shutdown support

On Wed, Mar 11, 2026 at 01:12:08PM -0400, David Jeffery wrote:
> Like its async suspend support, allow pci device shutdown to be performed
> asynchronously to improve shutdown time.

s/pci/PCI/
s/improve/reduce/

I like how simple this looks, so I hope it all works out.

BTW, something seems messed up in your post threading.  I assume this
series is supposed to go with the cover letter at
https://lore.kernel.org/all/20260311170956.9146-1-djeffery@redhat.com,
but the patches don't seem to be replies to the cover letter.

>  drivers/pci/probe.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index bccc7a4bdd79..4d98bab2163d 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1040,6 +1040,7 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge)
>  
>  	bus->bridge = get_device(&bridge->dev);
>  	device_enable_async_suspend(bus->bridge);
> +	device_enable_async_shutdown(bus->bridge);
>  	pci_set_bus_of_node(bus);
>  	pci_set_bus_msi_domain(bus);
>  	if (bridge->msi_domain && !dev_get_msi_domain(&bus->dev) &&
> @@ -2749,6 +2750,7 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>  	pci_reassigndev_resource_alignment(dev);
>  
>  	pci_init_capabilities(dev);
> +	device_enable_async_shutdown(&dev->dev);
>  
>  	/*
>  	 * Add the device to our list of discovered devices
> -- 
> 2.53.0
> 

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

* Re: [PATCH 2/5] driver core: separate function to shutdown one device
  2026-03-11 17:12 ` [PATCH 2/5] driver core: separate function to shutdown one device David Jeffery
  2026-03-11 18:00   ` Bart Van Assche
@ 2026-03-12  1:49   ` kernel test robot
  2026-03-12  5:10   ` kernel test robot
  2 siblings, 0 replies; 18+ messages in thread
From: kernel test robot @ 2026-03-12  1:49 UTC (permalink / raw)
  To: David Jeffery, linux-kernel, driver-core, linux-pci, linux-scsi,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich
  Cc: oe-kbuild-all, Tarun Sahu, Pasha Tatashin,
	Michał Cłapiński, Jordan Richards, Ewan Milne,
	John Meneghini, Lombardi, Maurizio, David Jeffery, Stuart Hayes,
	Laurence Oberman

Hi David,

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 jejb-scsi/for-next mkp-scsi/for-next linus/master v7.0-rc3 next-20260311]
[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/David-Jeffery/driver-core-separate-function-to-shutdown-one-device/20260312-011646
base:   driver-core/driver-core-testing
patch link:    https://lore.kernel.org/r/20260311171209.9205-2-djeffery%40redhat.com
patch subject: [PATCH 2/5] driver core: separate function to shutdown one device
config: arc-allnoconfig (https://download.01.org/0day-ci/archive/20260312/202603120917.gDcyYG9H-lkp@intel.com/config)
compiler: arc-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260312/202603120917.gDcyYG9H-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/202603120917.gDcyYG9H-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/base/core.c: In function 'device_shutdown':
>> drivers/base/core.c:4824:30: warning: variable 'parent' set but not used [-Wunused-but-set-variable]
    4824 |         struct device *dev, *parent;
         |                              ^~~~~~


vim +/parent +4824 drivers/base/core.c

f9dcdf9ae03c40 David Jeffery      2026-03-11  4818  
37b0c020343080 Greg Kroah-Hartman 2007-11-26  4819  /**
37b0c020343080 Greg Kroah-Hartman 2007-11-26  4820   * device_shutdown - call ->shutdown() on each device to shutdown.
37b0c020343080 Greg Kroah-Hartman 2007-11-26  4821   */
37b0c020343080 Greg Kroah-Hartman 2007-11-26  4822  void device_shutdown(void)
37b0c020343080 Greg Kroah-Hartman 2007-11-26  4823  {
f123db8e9d6c84 Benson Leung       2013-09-24 @4824  	struct device *dev, *parent;
6245838fe4d2ce Hugh Daschbach     2010-03-22  4825  
3297c8fc65af5d Pingfan Liu        2018-07-19  4826  	wait_for_device_probe();
3297c8fc65af5d Pingfan Liu        2018-07-19  4827  	device_block_probing();
3297c8fc65af5d Pingfan Liu        2018-07-19  4828  
65650b35133ff2 Rafael J. Wysocki  2019-10-09  4829  	cpufreq_suspend();
65650b35133ff2 Rafael J. Wysocki  2019-10-09  4830  
6245838fe4d2ce Hugh Daschbach     2010-03-22  4831  	spin_lock(&devices_kset->list_lock);
6245838fe4d2ce Hugh Daschbach     2010-03-22  4832  	/*
6245838fe4d2ce Hugh Daschbach     2010-03-22  4833  	 * Walk the devices list backward, shutting down each in turn.
6245838fe4d2ce Hugh Daschbach     2010-03-22  4834  	 * Beware that device unplug events may also start pulling
6245838fe4d2ce Hugh Daschbach     2010-03-22  4835  	 * devices offline, even as the system is shutting down.
6245838fe4d2ce Hugh Daschbach     2010-03-22  4836  	 */
6245838fe4d2ce Hugh Daschbach     2010-03-22  4837  	while (!list_empty(&devices_kset->list)) {
6245838fe4d2ce Hugh Daschbach     2010-03-22  4838  		dev = list_entry(devices_kset->list.prev, struct device,
6245838fe4d2ce Hugh Daschbach     2010-03-22  4839  				kobj.entry);
d1c6c030fcec6f Ming Lei           2012-06-22  4840  
d1c6c030fcec6f Ming Lei           2012-06-22  4841  		/*
d1c6c030fcec6f Ming Lei           2012-06-22  4842  		 * hold reference count of device's parent to
d1c6c030fcec6f Ming Lei           2012-06-22  4843  		 * prevent it from being freed because parent's
d1c6c030fcec6f Ming Lei           2012-06-22  4844  		 * lock is to be held
d1c6c030fcec6f Ming Lei           2012-06-22  4845  		 */
f123db8e9d6c84 Benson Leung       2013-09-24  4846  		parent = get_device(dev->parent);
6245838fe4d2ce Hugh Daschbach     2010-03-22  4847  		get_device(dev);
6245838fe4d2ce Hugh Daschbach     2010-03-22  4848  		/*
6245838fe4d2ce Hugh Daschbach     2010-03-22  4849  		 * Make sure the device is off the kset list, in the
6245838fe4d2ce Hugh Daschbach     2010-03-22  4850  		 * event that dev->*->shutdown() doesn't remove it.
6245838fe4d2ce Hugh Daschbach     2010-03-22  4851  		 */
6245838fe4d2ce Hugh Daschbach     2010-03-22  4852  		list_del_init(&dev->kobj.entry);
6245838fe4d2ce Hugh Daschbach     2010-03-22  4853  		spin_unlock(&devices_kset->list_lock);
fe6b91f47080eb Alan Stern         2011-12-06  4854  
f9dcdf9ae03c40 David Jeffery      2026-03-11  4855  		shutdown_one_device(dev);
6245838fe4d2ce Hugh Daschbach     2010-03-22  4856  
6245838fe4d2ce Hugh Daschbach     2010-03-22  4857  		spin_lock(&devices_kset->list_lock);
37b0c020343080 Greg Kroah-Hartman 2007-11-26  4858  	}
6245838fe4d2ce Hugh Daschbach     2010-03-22  4859  	spin_unlock(&devices_kset->list_lock);
37b0c020343080 Greg Kroah-Hartman 2007-11-26  4860  }
99bcf217183e02 Joe Perches        2010-06-27  4861  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH 4/5] pci: enable async shutdown support
  2026-03-11 17:12 ` [PATCH 4/5] pci: enable async shutdown support David Jeffery
  2026-03-11 23:08   ` Bjorn Helgaas
@ 2026-03-12  5:09   ` Greg Kroah-Hartman
  2026-03-12 13:54     ` David Jeffery
  1 sibling, 1 reply; 18+ messages in thread
From: Greg Kroah-Hartman @ 2026-03-12  5:09 UTC (permalink / raw)
  To: David Jeffery
  Cc: linux-kernel, driver-core, linux-pci, linux-scsi,
	Rafael J. Wysocki, Danilo Krummrich, Tarun Sahu, Pasha Tatashin,
	Michał Cłapiński, Jordan Richards, Ewan Milne,
	John Meneghini, Lombardi, Maurizio, Stuart Hayes,
	Laurence Oberman

On Wed, Mar 11, 2026 at 01:12:08PM -0400, David Jeffery wrote:
> Like its async suspend support, allow pci device shutdown to be performed
> asynchronously to improve shutdown time.
> 
> Signed-off-by: David Jeffery <djeffery@redhat.com>
> Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
> Tested-by: Laurence Oberman <loberman@redhat.com>
> ---
>  drivers/pci/probe.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index bccc7a4bdd79..4d98bab2163d 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1040,6 +1040,7 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge)
>  
>  	bus->bridge = get_device(&bridge->dev);
>  	device_enable_async_suspend(bus->bridge);
> +	device_enable_async_shutdown(bus->bridge);
>  	pci_set_bus_of_node(bus);
>  	pci_set_bus_msi_domain(bus);
>  	if (bridge->msi_domain && !dev_get_msi_domain(&bus->dev) &&
> @@ -2749,6 +2750,7 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>  	pci_reassigndev_resource_alignment(dev);
>  
>  	pci_init_capabilities(dev);
> +	device_enable_async_shutdown(&dev->dev);

For all PCI devices?  Are you sure this is ok?  That feels like it is
going to be ripe with race conditions...

How was this tested?

thanks,

greg k-h

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

* Re: [PATCH 2/5] driver core: separate function to shutdown one device
  2026-03-11 17:12 ` [PATCH 2/5] driver core: separate function to shutdown one device David Jeffery
  2026-03-11 18:00   ` Bart Van Assche
  2026-03-12  1:49   ` kernel test robot
@ 2026-03-12  5:10   ` kernel test robot
  2 siblings, 0 replies; 18+ messages in thread
From: kernel test robot @ 2026-03-12  5:10 UTC (permalink / raw)
  To: David Jeffery, linux-kernel, driver-core, linux-pci, linux-scsi,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich
  Cc: llvm, oe-kbuild-all, Tarun Sahu, Pasha Tatashin,
	Michał Cłapiński, Jordan Richards, Ewan Milne,
	John Meneghini, Lombardi, Maurizio, David Jeffery, Stuart Hayes,
	Laurence Oberman

Hi David,

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 jejb-scsi/for-next mkp-scsi/for-next linus/master v7.0-rc3 next-20260311]
[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/David-Jeffery/driver-core-separate-function-to-shutdown-one-device/20260312-011646
base:   driver-core/driver-core-testing
patch link:    https://lore.kernel.org/r/20260311171209.9205-2-djeffery%40redhat.com
patch subject: [PATCH 2/5] driver core: separate function to shutdown one device
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20260312/202603120643.h7j4avWA-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260312/202603120643.h7j4avWA-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/202603120643.h7j4avWA-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/base/core.c:4824:23: warning: variable 'parent' set but not used [-Wunused-but-set-variable]
    4824 |         struct device *dev, *parent;
         |                              ^
   1 warning generated.


vim +/parent +4824 drivers/base/core.c

f9dcdf9ae03c404 David Jeffery      2026-03-11  4818  
37b0c0203430802 Greg Kroah-Hartman 2007-11-26  4819  /**
37b0c0203430802 Greg Kroah-Hartman 2007-11-26  4820   * device_shutdown - call ->shutdown() on each device to shutdown.
37b0c0203430802 Greg Kroah-Hartman 2007-11-26  4821   */
37b0c0203430802 Greg Kroah-Hartman 2007-11-26  4822  void device_shutdown(void)
37b0c0203430802 Greg Kroah-Hartman 2007-11-26  4823  {
f123db8e9d6c84c Benson Leung       2013-09-24 @4824  	struct device *dev, *parent;
6245838fe4d2ce4 Hugh Daschbach     2010-03-22  4825  
3297c8fc65af5d4 Pingfan Liu        2018-07-19  4826  	wait_for_device_probe();
3297c8fc65af5d4 Pingfan Liu        2018-07-19  4827  	device_block_probing();
3297c8fc65af5d4 Pingfan Liu        2018-07-19  4828  
65650b35133ff20 Rafael J. Wysocki  2019-10-09  4829  	cpufreq_suspend();
65650b35133ff20 Rafael J. Wysocki  2019-10-09  4830  
6245838fe4d2ce4 Hugh Daschbach     2010-03-22  4831  	spin_lock(&devices_kset->list_lock);
6245838fe4d2ce4 Hugh Daschbach     2010-03-22  4832  	/*
6245838fe4d2ce4 Hugh Daschbach     2010-03-22  4833  	 * Walk the devices list backward, shutting down each in turn.
6245838fe4d2ce4 Hugh Daschbach     2010-03-22  4834  	 * Beware that device unplug events may also start pulling
6245838fe4d2ce4 Hugh Daschbach     2010-03-22  4835  	 * devices offline, even as the system is shutting down.
6245838fe4d2ce4 Hugh Daschbach     2010-03-22  4836  	 */
6245838fe4d2ce4 Hugh Daschbach     2010-03-22  4837  	while (!list_empty(&devices_kset->list)) {
6245838fe4d2ce4 Hugh Daschbach     2010-03-22  4838  		dev = list_entry(devices_kset->list.prev, struct device,
6245838fe4d2ce4 Hugh Daschbach     2010-03-22  4839  				kobj.entry);
d1c6c030fcec6f8 Ming Lei           2012-06-22  4840  
d1c6c030fcec6f8 Ming Lei           2012-06-22  4841  		/*
d1c6c030fcec6f8 Ming Lei           2012-06-22  4842  		 * hold reference count of device's parent to
d1c6c030fcec6f8 Ming Lei           2012-06-22  4843  		 * prevent it from being freed because parent's
d1c6c030fcec6f8 Ming Lei           2012-06-22  4844  		 * lock is to be held
d1c6c030fcec6f8 Ming Lei           2012-06-22  4845  		 */
f123db8e9d6c84c Benson Leung       2013-09-24  4846  		parent = get_device(dev->parent);
6245838fe4d2ce4 Hugh Daschbach     2010-03-22  4847  		get_device(dev);
6245838fe4d2ce4 Hugh Daschbach     2010-03-22  4848  		/*
6245838fe4d2ce4 Hugh Daschbach     2010-03-22  4849  		 * Make sure the device is off the kset list, in the
6245838fe4d2ce4 Hugh Daschbach     2010-03-22  4850  		 * event that dev->*->shutdown() doesn't remove it.
6245838fe4d2ce4 Hugh Daschbach     2010-03-22  4851  		 */
6245838fe4d2ce4 Hugh Daschbach     2010-03-22  4852  		list_del_init(&dev->kobj.entry);
6245838fe4d2ce4 Hugh Daschbach     2010-03-22  4853  		spin_unlock(&devices_kset->list_lock);
fe6b91f47080eb1 Alan Stern         2011-12-06  4854  
f9dcdf9ae03c404 David Jeffery      2026-03-11  4855  		shutdown_one_device(dev);
6245838fe4d2ce4 Hugh Daschbach     2010-03-22  4856  
6245838fe4d2ce4 Hugh Daschbach     2010-03-22  4857  		spin_lock(&devices_kset->list_lock);
37b0c0203430802 Greg Kroah-Hartman 2007-11-26  4858  	}
6245838fe4d2ce4 Hugh Daschbach     2010-03-22  4859  	spin_unlock(&devices_kset->list_lock);
37b0c0203430802 Greg Kroah-Hartman 2007-11-26  4860  }
99bcf217183e02e Joe Perches        2010-06-27  4861  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH 2/5] driver core: separate function to shutdown one device
  2026-03-11 18:00   ` Bart Van Assche
  2026-03-11 21:37     ` Bjorn Helgaas
@ 2026-03-12 13:39     ` David Jeffery
  1 sibling, 0 replies; 18+ messages in thread
From: David Jeffery @ 2026-03-12 13:39 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: linux-kernel, driver-core, linux-pci, linux-scsi,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	Tarun Sahu, Pasha Tatashin, Michał Cłapiński,
	Jordan Richards, Ewan Milne, John Meneghini, Lombardi, Maurizio,
	Stuart Hayes, Laurence Oberman, Marco Elver

On Wed, Mar 11, 2026 at 2:03 PM Bart Van Assche <bvanassche@acm.org> wrote:
>
> On 3/11/26 10:12 AM, David Jeffery wrote:
> > +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->parent);
> > +     put_device(dev);
> > +}
>
> Please keep the following code in the caller:
>
>         if (dev->parent && dev->bus && dev->bus->need_parent_lock)
>                 device_lock(dev->parent);
>
>         if (dev->parent && dev->bus && dev->bus->need_parent_lock)
>                 device_unlock(dev->parent);
>
>         put_device(dev->parent);
>         put_device(dev);
>
> Additionally, please make sure that the caller is made compatible with
> lock context analysis (see also
> https://lore.kernel.org/all/20250206181711.1902989-1-elver@google.com/).
> All that is required to make this code compatible with lock context
> analysis is to organize it as follows:
>
>         if (dev->parent && dev->bus && dev->bus->need_parent_lock) {
>                 device_lock(dev->parent);
>                 shutdown_one_device(dev);
>                 device_unlock(dev->parent);
>         } else {
>                 shutdown_one_device(dev);
>         }
>

This would also need to either grab another reference or move the
reference drops since shutdown_one_device may drop the last reference
the task owns to parent and dev. Since shutdown_one_device ends up
called in 2 places in the next patch, perhaps would be best to split
the bulk into another function, then have shutdown_one_device be a
wrapper like:

         if (dev->parent && dev->bus && dev->bus->need_parent_lock) {
                 device_lock(dev->parent);
                 __shutdown_one_device(dev);
                 device_unlock(dev->parent);
         } else {
                 __shutdown_one_device(dev);
         }
         put_device(dev->parent);
         put_device(dev);

David Jeffery


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

* Re: [PATCH 4/5] pci: enable async shutdown support
  2026-03-11 23:08   ` Bjorn Helgaas
@ 2026-03-12 13:46     ` David Jeffery
  0 siblings, 0 replies; 18+ messages in thread
From: David Jeffery @ 2026-03-12 13:46 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-kernel, driver-core, linux-pci, linux-scsi,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	Tarun Sahu, Pasha Tatashin, Michał Cłapiński,
	Jordan Richards, Ewan Milne, John Meneghini, Lombardi, Maurizio,
	Stuart Hayes, Laurence Oberman

On Wed, Mar 11, 2026 at 7:09 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> In subject, to match history:
>
>   PCI: Enable async shutdown support
>
> On Wed, Mar 11, 2026 at 01:12:08PM -0400, David Jeffery wrote:
> > Like its async suspend support, allow pci device shutdown to be performed
> > asynchronously to improve shutdown time.
>
> s/pci/PCI/
> s/improve/reduce/

Sure, I can clean up the wording.

> I like how simple this looks, so I hope it all works out.
>
> BTW, something seems messed up in your post threading.  I assume this
> series is supposed to go with the cover letter at
> https://lore.kernel.org/all/20260311170956.9146-1-djeffery@redhat.com,
> but the patches don't seem to be replies to the cover letter.

This was from an email issue I should have handled better. For some
reason, send-email hit an error with smtp after sending the cover
letter and failed to send the rest. When I then sent the rest
separately, I failed to consider the threading and didn't think to use
the option to force it to work as a reply to an ID from the already
sent cover letter.

> >  drivers/pci/probe.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index bccc7a4bdd79..4d98bab2163d 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -1040,6 +1040,7 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge)
> >
> >       bus->bridge = get_device(&bridge->dev);
> >       device_enable_async_suspend(bus->bridge);
> > +     device_enable_async_shutdown(bus->bridge);
> >       pci_set_bus_of_node(bus);
> >       pci_set_bus_msi_domain(bus);
> >       if (bridge->msi_domain && !dev_get_msi_domain(&bus->dev) &&
> > @@ -2749,6 +2750,7 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> >       pci_reassigndev_resource_alignment(dev);
> >
> >       pci_init_capabilities(dev);
> > +     device_enable_async_shutdown(&dev->dev);
> >
> >       /*
> >        * Add the device to our list of discovered devices
> > --
> > 2.53.0
> >
>


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

* Re: [PATCH 4/5] pci: enable async shutdown support
  2026-03-12  5:09   ` Greg Kroah-Hartman
@ 2026-03-12 13:54     ` David Jeffery
  0 siblings, 0 replies; 18+ messages in thread
From: David Jeffery @ 2026-03-12 13:54 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, driver-core, linux-pci, linux-scsi,
	Rafael J. Wysocki, Danilo Krummrich, Tarun Sahu, Pasha Tatashin,
	Michał Cłapiński, Jordan Richards, Ewan Milne,
	John Meneghini, Lombardi, Maurizio, Stuart Hayes,
	Laurence Oberman

On Thu, Mar 12, 2026 at 1:09 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Wed, Mar 11, 2026 at 01:12:08PM -0400, David Jeffery wrote:
> > Like its async suspend support, allow pci device shutdown to be performed
> > asynchronously to improve shutdown time.
> >
> > Signed-off-by: David Jeffery <djeffery@redhat.com>
> > Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
> > Tested-by: Laurence Oberman <loberman@redhat.com>
> > ---
> >  drivers/pci/probe.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index bccc7a4bdd79..4d98bab2163d 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -1040,6 +1040,7 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge)
> >
> >       bus->bridge = get_device(&bridge->dev);
> >       device_enable_async_suspend(bus->bridge);
> > +     device_enable_async_shutdown(bus->bridge);
> >       pci_set_bus_of_node(bus);
> >       pci_set_bus_msi_domain(bus);
> >       if (bridge->msi_domain && !dev_get_msi_domain(&bus->dev) &&
> > @@ -2749,6 +2750,7 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> >       pci_reassigndev_resource_alignment(dev);
> >
> >       pci_init_capabilities(dev);
> > +     device_enable_async_shutdown(&dev->dev);
>
> For all PCI devices?  Are you sure this is ok?  That feels like it is
> going to be ripe with race conditions...

I did not see or find any issues. PCI already enables async suspend on
all devices, it just does so in a power management specific function
so I didn't put device_enable_async_shutdown next to the
device_enable_async_suspend call.

> How was this tested?

It has been running on various VMs and x86_64 physical machines.

> thanks,
>
> greg k-h
>

David Jeffery


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

* Re: [PATCH 3/5] driver core: async device shutdown infrastructure
  2026-03-11 23:05   ` Bjorn Helgaas
@ 2026-03-12 14:01     ` David Jeffery
  0 siblings, 0 replies; 18+ messages in thread
From: David Jeffery @ 2026-03-12 14:01 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-kernel, driver-core, linux-pci, linux-scsi,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	Tarun Sahu, Pasha Tatashin, Michał Cłapiński,
	Jordan Richards, Ewan Milne, John Meneghini, Lombardi, Maurizio,
	Stuart Hayes, Laurence Oberman

On Wed, Mar 11, 2026 at 7:05 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Wed, Mar 11, 2026 at 01:12:07PM -0400, David Jeffery wrote:
> > Patterned after async suspend, allow devices to mark themselves as wanting
> > to perform async shutdown. Devices using async shutdown wait only for their
> > dependencies to shutdown before executing their shutdown routine.
>
> I'm not an expert on dependencies.  Is it obvious to everybody else
> how these dependencies are expressed?  What would I look at to verify
> that, for example, PCI devices are dependencies of the PCI bridges
> leading to them?  I suppose it's the same dependencies used for
> suspend?

Yes, it uses the same dependencies as async suspend does.

> From wait_for_shutdown_dependencies() below, it looks like we'll wait
> for each child of dev and then wait for each consumer of dev before
> shutting down dev itself.

Right, just like async suspend, all children and consumers need to
shut down first for async shutdown.

>
> > Sync shutdown devices are shut down one at a time and will only wait for an
> > async shutdown device if the async device is a dependency.
>
> > @@ -132,6 +133,7 @@ struct device_private {
> >  #ifdef CONFIG_RUST
> >       struct driver_type driver_type;
> >  #endif
> > +     struct completion complete;
>
> I thought "complete" might be a little too generic, but I guess async
> suspend uses "dev->power.completion" :)

I am open to a better name, but anything I came up with ended up
excessively long in my opinion, so I settled on the current and boring
"complete".

>
> > +static void wait_for_shutdown_dependencies(struct device *dev, bool async)
> > +{
> > +     struct device_link *link;
> > +     int idx;
> > +
> > +     device_for_each_child(dev, &async, wait_for_device_shutdown);
> > +
> > +     idx = device_links_read_lock();
> > +
> > +     dev_for_each_link_to_consumer(link, dev)
> > +             if (!device_link_flag_is_sync_state_only(link->flags))
> > +                     wait_for_device_shutdown(link->consumer, &async);
> > +
> > +     device_links_read_unlock(idx);
> > +}
>
> > @@ -4828,6 +4919,12 @@ void device_shutdown(void)
> >
> >       cpufreq_suspend();
> >
> > +     /*
> > +      * Start async device threads where possible to maximize potential
> > +      * paralellism and minimize false dependency on unrelated sync devices
>
> s/paralellism/parallelism/
>

Gah, I will correct it.

David Jeffery


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

end of thread, other threads:[~2026-03-12 14:01 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-11 17:12 [PATCH 1/5] driver core: do not always lock parent in shutdown David Jeffery
2026-03-11 17:12 ` [PATCH 2/5] driver core: separate function to shutdown one device David Jeffery
2026-03-11 18:00   ` Bart Van Assche
2026-03-11 21:37     ` Bjorn Helgaas
2026-03-11 21:42       ` Bart Van Assche
2026-03-12 13:39     ` David Jeffery
2026-03-12  1:49   ` kernel test robot
2026-03-12  5:10   ` kernel test robot
2026-03-11 17:12 ` [PATCH 3/5] driver core: async device shutdown infrastructure David Jeffery
2026-03-11 19:40   ` Randy Dunlap
2026-03-11 23:05   ` Bjorn Helgaas
2026-03-12 14:01     ` David Jeffery
2026-03-11 17:12 ` [PATCH 4/5] pci: enable async shutdown support David Jeffery
2026-03-11 23:08   ` Bjorn Helgaas
2026-03-12 13:46     ` David Jeffery
2026-03-12  5:09   ` Greg Kroah-Hartman
2026-03-12 13:54     ` David Jeffery
2026-03-11 17:12 ` [PATCH 5/5] scsi: " David Jeffery

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox