Note, retitled this thread -- I think the problem Andries reported is most likely tied to the seemingly flakey enumerations he saw (that can continue with the original subject line), but I'd like to find out if this is a real issue in uhci.
> > > > ... though I wonder if the way uhci.c works, it might not leave the > > > > qh->dev field valid rather "late" after unlinking associated URBs. > > > > > > qh->dev is always valid. > > > > > > > That'd be fine, except uhci_free_dev() doesn't ensure that such > > > > device state is all gone. (No such logic in usb-uhci.c either.) > > > > > > What device state? > > > > Those non-null qh->dev pointers. > > It may be non-null, but qh is not on any list. There is no way that it > can be referenced after uhci_free_qh is called. It's not uhci_free_qh() that's in question, but uhci_free_dev(). There'd be trouble if you were wrong about uhci_free_qh()! (Does qh->dev really need to be valid for each qh on the qh_remove_list, though? Could that be invalidated when each qh was put on that list -- usb_dec_dev_use and null the pointer?) > > ... > > What I was trying to to point out was that such cleanup would also > > be needed in more typical disconnect cases, and I don't see any > > way to force it to happen ... certainly not where I'd have expected > > it to be, in the deallocate() code! > > I don't think you understand how reference counting works. Perhaps not in the uhci context, but if so that's something that should get fixed: all the HCDs should behave the same! > usb_control_msg (send 8 bytes) 1 > uhci_submit_urb 1 > uhci_inc_dev_use 2 (once for the urb) > uhci_submit_control 2 > uhci_alloc_td 3 (SETUP) > uhci_alloc_td 4 (DATA) > uhci_alloc_td 5 (STATUS) > uhci_alloc_qh 6 In short, "uhci" tracks both qh->dev and td->dev counts. Fine, no problem. > [other processing waiting for URB to finish] > [user unplugs device] > > usb_disconnect 6 That must have been called by khubd. Which then invokes every driver's disconnect() method ... the details of your scenario aren't exactly clear to me. Looks like you're describing a case where some driver has a thread blocked in a control message, which its disconnect() code is going to have to somehow make sure completes before the driver returns from disconnect(). > usb_free_dev 5 You must mean usb_dec_dev_use() at this point. Who's calling it though? Remember khubd only calls this when every disconnect() routine finished. > [more processing, URB times out or fails. it wasn't explicitly unlinked] But usb_control_msg() does its own timeouts, and DOES use explicit unlinking ... ... so the rest of your call chain won't be exactly what happens. (I trimmed it in the interest of shorter messages.) I know there are some complex goings-on there (which I won't claim to understand :), but it's also not clear to me where the uhci_free_dev() call happens, you didn't show it. > As you can see, the only time the reference count would hit 0 is when > uhci.c is done with all QH's, TD's and URB's for that device. The only > way uhci_free_dev can be called is when the reference count is 0. Actually I still don't see that. To the extent that you expect interrupts to clean up the qh_remove_list it still seems like those might come after the uhci_free_dev() call returned. If the qh_remove_list is non-empty when uhci_free_dev() is called, then shouldn't that call block until an interrupt empties that list? At least, so long as the qh->dev pointers there are valid? Otherwise, so long as each qh on the remove list has a dev pointer, it still seems to me that khubd and uhci might race on the state of devices during disconnect processing. (And the symptom of the wrong winner would be a different BUG than Andries reported, FWIW -- it'd be in usb.c, not usb.h!!) - Dave _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
