Re: [PATCH 2/5] driver core: separate function to shutdown one device

[David Jeffery]

On Wed, Mar 11, 2026 at 2:03 PM Bart Van Assche <bvanassche@xxxxxxx> 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@xxxxxxxxxx/).
> 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