[RFC PATCH 0/4] USB: HCD/EHCI: giveback of URB in tasklet context

[Ming Lei]

On Wed, Jun 12, 2013 at 10:35 PM, Alan Stern <stern at rowland.harvard.edu> wrote:
> On Wed, 12 Jun 2013, Ming Lei wrote:
>
>> If not, I am wondering why one USB driver need register another hard
>> interrupt handler? Could you share such examples? I understand the
>> case only exists with timer handler as discussed with Oliver, don't I?
>
> Here's another possibility.  A USB driver has a private lock, and it
> acquires this lock in its completion handler.  But it also acquires
> this lock in other routines, and those other routines may be called by
> other drivers or subsystems in interrupt context.

OK, so the problem is more complicated than I imaged at start, :-(

>
>> > The drivers _do_ need to be changed, as explained above.  And that
>> > explanation was only one failure scenario.  There may be others.
>>
>> OK, I'd like to know what the others are? :-)
>
> Me too.  :-)
>
>> >> And finally, USB HCDs themselves will benefit from the change, won't they?
>> >
>> > How will they benefit?  Merely from not releasing their private locks?
>>
>> The interrupt of one HCD will be responded lately since another HCD's
>> interrupt handler takes very long time. With the change, the problem can
>> be avoided.
>
> For reasonably sophisticated host controllers like EHCI, the delay in
> responding to an interrupt doesn't matter much.  But for simpler host

EHCI might still benefit from the change: suppose uvc video is playing on
one EHCI bus, and mass storage's performance over another EHCI may
decrease because of the IRQ latency introduced.

> controllers (for example, those using PIO instead of DMA) it matters
> more, so those other HCDs will benefit more from the conversion.

Indeed, musb and ehci/ohci are coexisted on OMAP3/4/5.

>> >> Compared with "usb-storage" task's schedule latency, the tasklet
>> >> schedule delay should be lower at most of situations since the tasklet
>> >> is scheduled inside irq handler.
>> >
>> > True.  But the two latencies add.
>>
>> I think it should be one tasklet latency added, but what is the other one?
>
> I meant that the latency in tasklet scheduling has to be added to the
> latency in scheduling the usb-storage thread.
>
>> > Or you could have a separate tasklet for each host controller.
>>
>> Yes, but I will compare tasklet with interrupt threaded handler first.
>
> Switching to a threaded interrupt handler offers more possibilities.
> Instead of moving just the givebacks to the thread, we could put almost
> all the processing there.  (At least, we can for ehci-hcd.  Other HCDs
> may not be able to do this.)
>
> At this point we are talking about multiple changes:
>
>         Moving the givebacks to a tasklet or threaded handler.
>
>         Changing the USB completion handlers so that they can be called
>         with interrupts enabled.
>
>         Allowing the tasklet/thread to run with interrupts enabled.
>
>         Moving more of the HCD processing into the tasklet/thread.
>
> Maybe other things too.  In principle, the first and second items can
> be done simultaneously.

Very good summery, we can push the 1st change with disabling local IRQ
when calling complete(), so that at least DMA unmapping can be done
with IRQ enabled, and at the same time do the API change, which
should be a bit slow but the mechanical way proposed by Oliver may
be OK.

The 3rd item is easy once the 1st two items are completed.

For the 4th one, it might be a long term thing, since refactoring
HCD interrupt handler is a bit complicated. Also, when the 1st
three items are completed, hard interrupt handler takes less time,
often less than 20us at average about EHCI, so I am wondering
if the 4th is worthy.


Thanks,
--
Ming Lei