Re: [PATCH] Driver core: Don't ignore bus_attach_device() retval

On Thu, 12 Oct 2006 11:59:45 -0400 (EDT),
Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:

* device_bind_driver() failed to create some symlinks. We may
consider not to fail in this case, since sysfs_remove_link() is fine
even for non-existing links.

It would be okay to fail in this case, because the driver would not have
been probed at all.

In really_probe(), it is called after ->probe has been called (though
the code does nothing but complain in the failure case).

* probing failed for one possible driver with something other than
-ENODEV or -ENXIO. Not sure if we really should abort in this case.
We'd just end up with an unbound device, and a driver returning (for
example) -ENOMEM for probing may just be a really dumb driver trying to
allocate an insane amount of memory (and the next driver might just be
fine).

Yes, this is exactly what I meant. Having an unbound device is okay.

Maybe ignoring all probe errors would be best here? (Calling
device_bind_driver() only on success, of course.)

I haven't looked at the driver core much since multithreaded probing was
added. The multithreading part has a bad locking bug: the new thread
doesn't acquire the necessary semaphores. Also it probes multiple drivers
for the same device in parallel, which seems wrong. Since multiple probes
can't run concurrently there's no reason to have a separate thread for
each one. There should be only one new thread per device.

Currently, it is the device driver which specifies whether multithreaded
probe should be done. Maybe this should rather be specified per
subsystem? Having several bus_for_each_drv(..., dev, __device_attach)
run in parallel makes more sense to me than the current approach.

One way to fix this would be to make device_bind_driver() always
succeed (even without symlinks),

Hmm... If device_bind_driver() fails -- because of the symlinks -- then
the device is still on the driver's klist, because the klist_add_tail() in
driver_bound() never gets undone. Another bug.

There's already been a fix:
http://marc.theaimsgroup.com/?l=linux-kernel&m=116062971226779&w=2

Clearly
device_bind_driver() should call driver_sysfs_add() before driver_bound(),
not after. Or else it should never fail.

Or that. I'm currently a bit in favour of ignoring symlink errors.

It's a lot safer and easier just to switch the order of the calls in
device_bind_driver().

?? Did you mean really_probe() here?

I
think in all cases, bus_attach_device() really should ignore the return
code from device_attach().

This would be the only sensible approach if we had one probing thread
per device. And device_bind_driver() should probably always succeed.

--
Cornelia Huck
Linux for zSeries Developer
Tel.: +49-7031-16-4837, Mail: cornelia.huck@xxxxxxxxxx
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Relevant Pages