> Very interesting. The UHCI Design Guide document is _not_ very specific > about what happens with short packets. It does say that if the SPD bit is > set then the TD is marked inactive, the QH element ptr is not updated, and > an interrupt is requested. It doesn't say what happens if the SPD bit > isn't set; I'm guessing that it does the same thing but doesn't request > an interrupt. One could easily argue that this behavior is a design flaw > and that the QH element ptr's Terminate bit should get set. > > The Design Guide also doesn't say how the schedule execution works when > the HC retrieves a TD marked inactive. I'll assume that it doesn't > process the TD any farther but just follows the QH horizontal link ptr. > > It _does_ say this (table 3.2.2 bit 29 = SPD): "The behavior is undefined > when this bit is set with output packets or packets outside of queues." > I think it's pretty clear that this means uhci_submit_common() isn't > setting SPD correctly. It should _always_ be set for queued (not iso.) IN > packets regardless of URB_SHORT_NOT_OK, which is handled in > uhci_result_common(). It should _never_ be set for OUT packets. Of > course, that's not the cause of the problem here. > > > So look at what happens in uhci_irq(). It calls uhci_transfer_result(), > which in turn calls uhci_unlink_generic() for completed URBs. That calls > uhci_delete_queued_urb(), which unlinks the URB's QH from the schedule. > > Then later on uhci_irq() calls uhci_finish_completion(), which in turn > calls uhci_finish_urb() for each completed URB. That calls > uhci_destroy_urb_priv(), which among other things deallocates the URB's > TDs. > > Apparently there's an assumption here that once an URB is completed, the > HC scheduler won't look at its TDs any more. But that's not necessarily > true. Consider first that the SPD interrupt doesn't arrive until the end > of the frame. The CPU won't start processing the interrupt until the HC > has already begun the next frame, and if the schedule isn't very full then > the HC may easily reach the URB's QH before uhci_delete_queued_urb() has > managed to unlink it. With FSBR turned on, the HC may even read that QH > several times before it gets unlinked. > > Consider second that if the URB completes because of a short read, the > QH's element ptr _won't_ point to the next queued QH; it will still point > to the TD that got the short packet and is now marked inactive. (That's > true even if there isn't another URB queued for the same endpoint. And > it's also true for URBs that complete because of an error, not just a > short read.) As far as the scheduler is concerned that won't make any > difference; when it sees the inactive TD it will ignore it and follow the > QH link ptr anyway. But the HC still has to decode that TD! > > The upshot of all this is that when uhci_destroy_urb_priv() removes and > frees the TD, the HC might be about to read it. Now I don't know how > likely that is since there's clearly a race; the CPU would have to unlink > the QH and free the TD all during the time the HC is processing the QH. > But it would explain what Duncan is seeing. > > It's exactly as Johannes said; the driver doesn't wait for the next > interrupt after unlinking the QH before freeing the TDs. > > Would it make more sense to store the td_list header in the uhci_qh > rather than in the urb_priv? That way the TDs could be freed along with > the QH in uhci_free_pending_qhs(), which does execute at the right time. > Maybe there's a less invasive way to accomplish the same thing, but I > can't think of one.
Hi Alan, if this scenario is correct then the same problem could occur even if urbs are not being queued, right? So the fact I only see this with queued urbs would be a coincidence: getting the timing just right so as to hit the race... Ciao, Duncan. ------------------------------------------------------- This SF.Net email is sponsored by: INetU Attention Web Developers & Consultants: Become An INetU Hosting Partner. Refer Dedicated Servers. We Manage Them. You Get 10% Monthly Commission! INetU Dedicated Managed Hosting http://www.inetu.net/partner/index.php _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
