On Thu, Apr 18, 2002, David Brownell <[EMAIL PROTECTED]> wrote:
> > > ... 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.
> > If reference counting is working like it should be,
> > the only time uhci_free_dev is called is when there are no URB's, TD's
> > and QH's assigned to that device.
>
> Well, certainly no URBs ... but the higher level code can't know
> anything about HCD level state (TDs, QHs/EDs, etc). That's
> what I'd understood the HCD deallocate() call -- in this case
> called uhci_free_dev() -- was responsible for cleaning.
>
> If, as you say, deallocate() shouldn't handle that, then what does?
>
> Yes, I just noticed that deallocate is called after the refcount goes
> to zero. While that's something I want to call a bug, it clearly won't
> affect UHCI (that call is a NOP) or OHCI or EHCI (other reasons),
> another reading is that something else (but what?) is expected to
> clear that state. OHCI and EHCI both use that call as the "clean
> up device state" hook; seems we're into areas where the HCDs
> interpret the same usbcore calls differently (trouble).
>
> Like I said, I was just wondering ... I don't know the uhci innards,
> but noticed that uhci_pci_remove() calls a routine in that BUG()
> stack -- uhci_free_pending_qhs() -- only _after_ all the drivers
> were disconnected. That sure looks like uhci.c expected some
> TD/QH type state to linger on and so need "late" cleanup, so it
> provided the "something else" on that code path.
>
> 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. Let me give
you an example:
function call reference count (device)
------------- ------------------------
usb_alloc_dev 1 (starts at one)
[enumeration, module probing, etc]
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
[other processing waiting for URB to finish]
[user unplugs device]
usb_disconnect 6
usb_free_dev 5
[more processing, URB times out or fails. it wasn't explicitly unlinked]
uhci_interrupt 5
uhci_transfer_result 5
uhci_result_control 5
uhci_unlink_generic 5
uhci_remove_qh 5 (qh added to qh_remove_list)
uhci_finish_completion 5
uhci_call_completion 5
uhci_destroy_urb_priv 5
uhci_free_td 4
uhci_free_td 3
uhci_free_td 2
uhci_dec_dev_use 1 (done with URB)
[more processing, next interrupt occurs]
uhci_interrupt 1
uhci_free_pending_qhs 1
uhci_free_qh 0
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.
Thusly, there is no way uhci_free_dev can be called with any QH's, TD's
and URB's still being used. That's why uhci_free_dev is empty, because
when it's called, there's always nothing to cleanup for that device.
The only way that this can't be true would be because of a reference
counting bug somewhere. I'm pretty confident that uhci.c is correct in
the way it does reference counting.
I'll double check the uhci_pci_remove path as well. That is a little
tricky and not well tested.
JE
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
