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

["Danilo Krummrich" <dakr@kernel.org>]

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