Re: [PATCH] driver core: faux: Check device binding state by dedicated API device_is_bound()

[Zijun Hu]

On 2025/3/7 22:04, Greg Kroah-Hartman wrote:
>> -	if (!dev->driver) {
>> +	/* Do not need to device_lock(dev) due to no race here */
>> +	if (!device_is_bound(dev)) {
> Ick, this feels wrong.  This is the driver core code, it can poke in

both 'dev->driver' and device_is_bound() is okay with this very special
context.

'dev->driver'    :     is binding or have bound successfully.
device_is_bound():     have bound successfully.

device_is_bound() may be more accurate here.

> dev->driver if it wants to, and as the lock is not held, and there is no
> lock even mentioned in this driver, the comment is confusing unless you
> dig and go read that device_is_bound() requires this.
> 

agree.

> Also, when we start to add locking requirements to functions like
> device_is_bound() (which we really should) this call will fail the
> check, right?  How are you going to explain that?  🙂

Generically, for 'dev->driver' and device_is_bound(), device_lock()
should be hold firstly to avoid race as driver core codes do.

this patch may be still okay if device_is_bound() is changed to
bool device_is_bound(struct device *dev)
{
	bool ret;
	
	device_lock(dev);
	ret = dev->p && klist_node_attached(&dev->p->knode_driver);
	device_unlock(dev);

	return ret;
}