[PATCH v3 02/13] driver core: Enable suppliers to implement fine grained sync_state support

[Ulf Hansson]

On Fri, 8 May 2026 at 20:23, Danilo Krummrich <dakr at kernel.org> wrote:
>
> On Fri May 8, 2026 at 2:38 PM CEST, Ulf Hansson wrote:
> > diff --git a/drivers/base/base.h b/drivers/base/base.h
> > index 30b416588617..c8be24af92c3 100644
> > --- a/drivers/base/base.h
> > +++ b/drivers/base/base.h
> > @@ -196,6 +196,24 @@ static inline void dev_sync_state(struct device *dev)
> >               dev->driver->sync_state(dev);
> >  }
> >
> > +static inline bool dev_has_queue_sync_state(struct device *dev)
> > +{
> > +     struct device_driver *drv;
> > +
> > +     if (!dev)
> > +             return false;
> > +     drv = READ_ONCE(dev->driver);
> > +     if (drv && drv->queue_sync_state)
> > +             return true;
> > +     return false;
> > +}
> > +
> > +static inline void dev_queue_sync_state(struct device *dev)
> > +{
>
> Let's add device_lock_assert(dev), as this function requires it to be held.
>
> > +     if (dev->driver && dev->driver->queue_sync_state)
> > +             dev->driver->queue_sync_state(dev);
> > +}
>
> <snip>
>
> > @@ -1175,6 +1188,25 @@ static void device_links_flush_sync_list(struct list_head *list,
> >
> >               put_device(dev);
> >       }
> > +
> > +     if (!q_list)
> > +             return;
> > +
> > +     list_for_each_entry_safe(dev, tmp, q_list, links.queue_sync) {
> > +             list_del_init(&dev->links.queue_sync);
>
> Hm...this is called without the device_links_write_lock() held, so this looks
> like this could this race with the list_del_init() from
> device_links_driver_cleanup() and create an infinite loop.
>
> (The other list iterator above might have the same issue.)

Yes, you are right!

I was trying to follow the existing implementation for the regular
sync_state, but it certainly looks like we need a separate lock to
protect the lists.

Let me fold in a patch that fixes this for the regular ->sync_state()
code first.

>
> > +
> > +             if (dev != dont_lock_dev)
>
> This is pre-existing, but I think it would be good to add a device_lock_assert()
> call for this as well.

Makes perfect sense.

Again, I will fold in a patch that start by adding this for the
regular ->sync_state().

>
> > +                     device_lock(dev);
> > +
> > +             device_links_write_lock();
> > +             dev_queue_sync_state(dev);
> > +             device_links_write_unlock();
> > +
> > +             if (dev != dont_lock_dev)
> > +                     device_unlock(dev);
> > +
> > +             put_device(dev);
> > +     }
> >  }

Thanks for reviewing!

Kind regards
Uffe