On Tue, Jun 08, 2021 at 07:07:49AM +0100, Christoph Hellwig wrote:
> > index 36d0c654ea6124..03591f82251302 100644
> > +++ b/drivers/base/bus.c
> > @@ -212,13 +212,9 @@ static ssize_t bind_store(struct device_driver *drv, const char *buf,
> > dev = bus_find_device_by_name(bus, NULL, buf);
> > if (dev && dev->driver == NULL && driver_match_device(drv, dev)) {
> > err = device_driver_attach(drv, dev);
> > -
> > - if (err > 0) {
> > + if (!err) {
> > /* success */
> > err = count;
> > - } else if (err == 0) {
> > - /* driver didn't accept device */
> > - err = -ENODEV;
> > }
> > }
>
> I think we can also drop the dev->driver == NULL check above given
> that device_driver_attach covers it now.
I'm glad you noticed this because it is wonky today:
static ssize_t bind_store() {
int err = -ENODEV;
if (dev && dev->driver == NULL && driver_match_device(drv, dev)) {
err = device_driver_attach() {
int ret = 0;
__device_driver_lock(dev, dev->parent);
if (!dev->p->dead && !dev->driver)
..
return ret;
}
}
return err;
Thus if dev->driver == NULL this will usually return -ENODEV unless it
races just right and returns 0. So I fixed it up to always return
-EBUSY and always read dev->driver under the lock.
Jason
