On Fri, 11 Aug 2006, Greg KH wrote:
> > This won't work right if an error occurs. The value you return to the
> > caller of usb_new_device() reflects only errors in creating the new
> > thread. It doesn't reflect any errors the thread itself may encounter.
> >
> > So the caller won't realize it if __usb_new_device() fails.
>
> Yes, I realise this, but the callers of usb_new_device() don't always do
> something "useful" with that information anyway, so it's not a real loss
> here :)
Well, let's take a look at the callers. There are only two. One is
hub_port_connect_change() in hub.c. Here's what it does:
if (!status) {
status = usb_new_device(udev);
if (status) {
spin_lock_irq(&device_state_lock);
hdev->children[port1-1] = NULL;
spin_unlock_irq(&device_state_lock);
}
}
if (status)
goto loop_disable;
You could fold the assignment to hdev->children into __usb_new_device, but
what about the conditional jump to loop_disable?
The other caller is register_root_hub() in hcd.c. Here's what it does:
retval = usb_new_device (usb_dev);
if (retval) {
dev_err (parent_dev, "can't register root hub for %s, %d\n",
usb_dev->dev.bus_id, retval);
}
mutex_unlock(&usb_bus_list_lock);
if (retval == 0) {
spin_lock_irq (&hcd_root_hub_lock);
hcd->rh_registered = 1;
spin_unlock_irq (&hcd_root_hub_lock);
/* Did the HC die before the root hub was registered? */
if (hcd->state == HC_STATE_HALT)
usb_hc_died (hcd); /* This time clean up */
}
return retval;
Merging that into __usb_new_device will be hard, since hcd_root_hub_lock
is private to hcd.c.
Furthermore, there are serial dependencies here. What if
register_root_hub apparently succeeds, but then the HCD quickly decides to
unregister the root hub before the child thread has had a chance to
register it?
As I see it, there's no advantage in making register_root_hub run in a
new thread. There _is_ an advantage in making the call from the hub
driver run in a new thread, but there are also dangers. Just as one
example, what's to prevent someone from doing "rmmod usbcore" while these
sub-threads are still running?
Alan Stern
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
