On Thu, Apr 18, 2002, David Brownell <[EMAIL PROTECTED]> wrote: > > 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?)
It doesn't have to be, we can decrement the reference earlier, but it won't change how correct the code is, it'll just result in the memory being freed 1ms earlier. > > 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! I think that goes without saying. > > 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. Yes, it's designed that way to ensure that we do want to dereference ->dev, it'll still be a valid pointer. > > [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(). Yes, I'm explaining a situation which would show any problems. > > 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. At the end of usb_disconnect. usb_dec_dev_use is an alias for usb_free_dev, so it's the same code. Note the indentation I used for the calling order. > > [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 ... Not a timeout in that sense. A timeout in the sense that UHCI tries to transfer data to the now nonexistant device and it'll eventually be returned back uhci.c with a CRC/Timeo error. > ... so the rest of your call chain won't be exactly what happens. > (I trimmed it in the interest of shorter messages.) Yes it will. > 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. It's at the end. usb_free_dev will call it when the reference count is 0. Look at the code. > > 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. I guess you don't understand how reference counting occurs. uhci_free_dev will never be called before qh_remove_list removes the QH because the reference count won't be 0 until qh_remove_list is processed. > 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? Not a problem because uhci_free_dev can't be called until the reference count is 0. > 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!!) No race, because of the reference counting. JE _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
