Re: [PATCH v2] driver core: Don't let a device probe until it's ready
From: Doug Anderson
Date: Tue Mar 31 2026 - 11:34:53 EST
Hi,
On Tue, Mar 31, 2026 at 7:42 AM Danilo Krummrich <dakr@xxxxxxxxxx> wrote:
>
> > @@ -848,6 +848,18 @@ static int __driver_probe_device(const struct device_driver *drv, struct device
> > if (dev->driver)
> > return -EBUSY;
> >
> > + /*
> > + * In device_add(), the "struct device" gets linked into the subsystem's
> > + * list of devices and broadcast to userspace (via uevent) before we're
> > + * quite ready to probe. Those open pathways to driver probe before
> > + * we've finished enough of device_add() to reliably support probe.
> > + * Detect this and tell other pathways to try again later. device_add()
> > + * itself will also try to probe immediately after setting
> > + * "ready_to_probe".
> > + */
> > + if (!dev->ready_to_probe)
> > + return dev_err_probe(dev, -EPROBE_DEFER, "Device not ready_to_probe");
>
> Are we sure this dev->ready_to_probe dance does not introduce a new subtle bug
> considering that ready_to_probe is within a bitfield of struct device?
>
> I.e. are we sure there are no potential concurrent modifications of other fields
> in this bitfield that are not protected with the device lock?
>
> For instance, in __driver_attach() we set dev->can_match if
> driver_match_device() returns -EPROBE_DEFER without the device lock held.
Bleh. Thank you for catching this. I naively assumed the device lock
protected the bitfield, but I didn't verify that.
> This is exactly the case you want to protect against, i.e. device_add() racing
> with __driver_attach().
>
> So, there is a chance that the dev->ready_to_probe change gets interleaved with
> a dev->can_match change.
>
> I think all this goes away if we stop using bitfields for synchronization; we
> should convert some of those to flags that we can modify with set_bit() and
> friends instead.
That sounds reasonable to me. Do you want me to send a v3 where I
create a new "unsigned long flags" in struct device and introduce this
as the first flag? If there are additional bitfields you want me to
convert, I can send them as additional patches in the series as long
as it's not too big of a change...
-Doug