Re: [PATCH 3/5] driver core: async device shutdown infrastructure

[David Jeffery]

On Mon, Mar 23, 2026 at 5:50 AM Maurizio Lombardi <mlombard@xxxxxxxxxx> wrote:
>
> On Thu Mar 19, 2026 at 3:11 PM CET, 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@xxxxxxxxxx>
> > Signed-off-by: Stuart Hayes <stuart.w.hayes@xxxxxxxxx>
> > Tested-by: Laurence Oberman <loberman@xxxxxxxxxx>
> > ---
> >  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 2e9094f5c5aa..53568b820a13 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)
> >  {
> >       device_lock(dev);
> > @@ -4805,6 +4842,8 @@ static void __shutdown_one_device(struct device *dev)
> >               dev->driver->shutdown(dev);
> >       }
> >
> > +     complete_all(&dev->p->complete);
> > +
> >       device_unlock(dev);
> >  }
> >
> > @@ -4823,6 +4862,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.
> >   */
> > @@ -4835,6 +4926,12 @@ void device_shutdown(void)
> >
> >       cpufreq_suspend();
> >
> > +     /*
> > +      * Start async device threads where possible to maximize potential
> > +      * parallelism 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.
> > @@ -4859,11 +4956,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;
> > +}
>
> Shouldn't this function just return void?

Yes, it should just be changed to a void.

David Jeffery