On Sun, 17 Apr 2005, David Brownell wrote:

> Or other IRQs, etc.  I punted the detailed analysis, but briefly:
> 
>   * OHCI typically gets a completion (WDH) IRQ at the start of a frame,
>     then if there's anything on the control or bulk queues it'll do
>     that for 10% of a frame and then start periodic processing.  So it's
>     got about 100 usec ... and when I last measured, it had no problem
>     processing an interrupt transfer completion and reissuing it so that
>     a 1/msec poll rate would work.  (Clearly, load dependent.)
> 
>  * EHCI typically gets completion IRQs every microframe (as needed, but
>    this is a tuning thing) which means it usually has up to 850 usec to
>    satisfy the 1/msec polling rate (again with a single non-queued URB).
> 
> As I understand what UHCI hardware does, it won't do as well on either of
> those cases.  But certainly for OHCI, that 100 usec available to handle
> the IRQ (so far as stick-urb-back-on-schedule) is a bit tight; anything
> to stretch out the latency would hurt the ability to do back-to-back
> transfers with a resubmit in between.

Well, I never said that OHCI or EHCI should start using a tasklet!

With UHCI the detailed timing information isn't available.  However I
believe it's essentially impossible for a completion handler for a
period-1 interrupt URB to resubmit in time for the next frame.  Most
likely the window of opportunity has already closed by the time the
handler is invoked.  And that's with the current code.  This is because
UHCI controllers do periodic processing first, at the start of each new
frame.

> > The delay for resubmission by a completion handler also won't be very 
> > long.  After all, the completion handler is called _by the tasklet_, so 
> > the tasklet is already running.  There won't be any overhead for starting 
> > it up.
> 
> As I said, depending on how it's coded; you're assuming some merging.

Sorry, I don't follow you.  Merging?

> But you're also not denying there'd be additional latencies added on
> the resubmit paths.

No more than the usual IRQ -> tasklet delay.  And there's a question of
competing requirements.  Suppose two URBs happen to complete in the same
frame, and the handler for the first one wants to resubmit.  With the
current code we have:

        IRQ handler calls completion handler for URB 1
        Handler resubmits, URB is relinked back into the schedule
        IRQ handler calls completion handler for URB 2

With the proposed tasklet we would have instead:

        IRQ handler marks tasklet for running
        Tasklet calls completion handler for URB 1
        Handler resubmits, URB is left on a request queue
        Tasklet calls completion handler for URB 2
        Tasklet links URB 1 back into the schedule

Let's ignore the delay involved in firing up the tasklet (which applies to 
all completions, not just those involving resubmission) and consider only
what happens after the tasklet starts running.  Yes, there is additional
latency in the resubmit path for URB 1.  But that latency has been removed
from the completion path for URB 2!  You see there are two things to do:
complete URB 2 and relink URB 1.  One has to come before the other, and no
matter which way it's written you're going to complain that somebody has
been delayed.  A delay for one or the other is inevitable.


> >       There's no reason the
> > driver can't make its own copy of the struct pt_regs and pass a pointer to
> > the copy to a completion handler later on.  (Not a copy of the original
> > pointer; that would be useless, as you said.)
> 
> Nothing except the fact that it'd be invalid (i.e. not current) at that time.
> Better to just pass a null pointer.

I'm not convinced of that.  Those register values aren't useful for
anything other than debugging and statistics gathering, so far as I know.  
For those purposes it doesn't matter if the values aren't fully current.  
(Of course, I don't know all about it.)  Furthermore, what makes you so 
sure that the contents wouldn't be current?  Consider this:

        IRQ arrives; the system saves the registers in pt_regs
        Handler is invoked, copy of pt_regs is saved
        Tasklet is run, pointer to copy of pt_regs is passed
                to completion routine
        System returns to what it was doing before the IRQ

Complications arise when another IRQ arrives while the tasklet is running;  
which copy of pt_regs will then be passed to completion routines?  Still,
I don't think this is too bad.


> > > By data stream corruption when the hardware keeps processing the
> > > requests.  Yes, tricky to detect directly, and not common ... the
> > > point of the current (pre-patch) API was to preclude such trouble
> > 
> > But if the HCD works correctly there won't be any data stream corruption.  
> > Certainly processing URBs 1-14 won't cause corruption, if URB 15 was 
> > the one that got unlinked.
> 
> Corruption would be 1-14 followed by 16 ... possible if the queue
> wasn't stopped.

But the queue _will_ be stopped before URB 16 is reached.  That guarantee
is part of the API update.

> > The idea I'm trying to get across here is that the queue doesn't have to 
> > be stopped _at the time the completion handler runs_.  It suffices to stop 
> > the queue any time before the hardware reaches the unlinked URB.  If the 
> > HCD can do so safely, and if that time doesn't occur until after the 
> > completion handler has finished, then the handler can run while the queue 
> > is active.
> 
> That sounds more sensible, though ...
> 
> 
> > That's what my documentation change was intended to convey.  The old text 
> > said that the queue would be stopped when the handler runs.  The new text 
> > gives the HCD more leeway; it doesn't have to stop the queue ASAP but only 
> > when stopping becomes necessary.
> 
> ... I guess I still don't see how the HCD could guarantee that
> without first stopping the queue ... if it's running, it could end
> up running past that point.

It can be done by having two queues: the actual hardware queue, plus a
"request" queue for URBs that have been submitted but aren't on the
hardware queue because the tasklet hasn't linked them in yet.  If URBs
15-20 are still on the request queue, then the hardware queue will stop
all by itself at an appropriate time with no need for the HCD to do
anything.


> > > But there _is_ a new mechanism you want to require, as you said.
> > > Holding back all fault completions iff an unlink is pending.
> > 
> > That's right.  Your case 3b needs something like this if it is to follow 
> > the API's guidelines.  The way the HCDs currently handle it doesn't agree 
> > with either the old or the new documentation.
> 
> Which was being addressed separately ... not entirely to my
> satisfaction, but safely for now.  I think we agree that some
> changes closer to usbcore/hcd are needed for that case.  Just
> what those changes should be is a different issue.  :)

Yes, they are not truly relevant to this discussion.

Alan Stern



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to