Re: [PATCH v4 1/9] driver core: Don't let a device probe until it's ready

Danilo Krummrich
2026-04-05T20:58:07Z

On Sat Apr 4, 2026 at 2:04 AM CEST, Douglas Anderson wrote:
> Instead of adding another flag to the bitfields already in "struct
> device", instead add a new "flags" field and use that. This allows us
> to freely change the bit from different thread without holding the
> device lock and without worrying about corrupting nearby bits.

I was just about to pick up this patch series (Greg mentioned to pick it up next
week, but we agreed offlist that I will pick it now, so it gets a few more
cycles in linux-next).

Due to this, taking a second glance at the code, I noticed the below issue.

> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 09b98f02f559..f07745659de3 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -3688,6 +3688,19 @@ int device_add(struct device *dev)
> fw_devlink_link_device(dev);
> }
>
> + /*
> + * The moment the device was linked into the bus's "klist_devices" in
> + * bus_add_device() then it's possible that probe could have been
> + * attempted in a different thread via userspace loading a driver
> + * matching the device. "ready_to_prove" being unset would have
> + * blocked those attempts. Now that all of the above initialization has
> + * happened, unblock probe. If probe happens through another thread
> + * after this point but before bus_probe_device() runs then it's fine.
> + * bus_probe_device() -> device_initial_probe() -> __device_attach()
> + * will notice (under device_lock) that the device is already bound.
> + */
> + dev_set_ready_to_probe(dev);

By converting this to a bitop, we now avoid races with other bitfields (such as
dev->can_match), but I think we still need to take the device lock for this one
specifically:

Task 0 (device_add): Task 1 (__driver_probe_device):

dev->fwnode->dev = dev;
device_lock(dev);
device_lock(dev); if (dev_ready_to_probe())
dev_set_ready_to_probe() access(fwnode->dev);
device_unlock(dev); device_unlock(dev);

Otherwise, nothing prevents the above dev->fwnode->dev = dev assignment to be
re-ordered with dev_set_ready_to_probe() and we are back to the problem the
commit attempts to solve in the first place.

(Technically, this could also be solved with explicit memory barriers - here and
below -, but __driver_probe_device() is always called with the device lock held,
so just taking the device lock seems *much* simpler. Also, in the absolute
majority of cases the lock should be uncontended in device_add() anyways.)

> +
> bus_probe_device(dev);
>
> /*
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 37c7e54e0e4c..8ec93128ea98 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -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(dev))
> + return dev_err_probe(dev, -EPROBE_DEFER, "Device not ready to probe\n");
> +
> dev->can_match = true;

Focused on ordering from the above, I also noticed that this ordering of
dev_ready_to_probe() and dev->can_match = true is actually pretty subtle and we
should add the following comment.

/*
* Set can_match = true after calling dev_ready_to_probe(), so
* driver_deferred_probe_add() won't actually add the device to the
* deferred probe list when dev_ready_to_probe() returns false.
*
* When dev_ready_to_probe() returns false, it means that device_add()
* will do another probe() attempt for us.
*/

As it would be nice to land this for v7.1-rc1, I can apply both changes on
apply, i.e. not need to resend AFAIC.

Greg, Rafael: does that work for you?

Thanks,
Danilo