Re: [patch 2.6.29] usb: ehci-sched.c: EHCI SITD scheduling bugfix (resend)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Apr 13, 2009 at 7:38 PM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> On Mon, 13 Apr 2009, Alan Stern wrote:
>
>> > The tt_available function should (with the -1 removed) only schedule a
>> > multi-uframe isoc transfer starting in an empty uframe,
>>
>> Why?  What's wrong with scheduling a multi-uframe isoc transfer to
>> begin in a partially-occupied uframe?
>
> Never mind; I went back to the specs and did some rereading...
>
> First of all, IN transfers can never have more than one SSPLIT in any
> case.
>
> Secondly, interrupt transfers never have more than one SSPLIT.

I know; and SSPLITs can be buffered by the TT for up to 3 uframes
after the TT receives it.  In TT periodic scheduling, the high-speed
SSPLITs always happens exactly when they are scheduled to happen (i.e.
in the uframe when it's scheduled), and the TT has to buffer the data
until it can execute it (or reject the SSPLIT, which is not retried in
the next uframe).

And yes, large isoc in transfers can start in the middle of a uframe,
but calculating the bandwidth for this is more complicated.  As I
said, the carryover math can be improved; starting all multi-uframe
transfers in an empty uframe is simply easier.

> Which leaves only isoc OUT.  Both the USB and EHCI specs say (although
> you have to look pretty closely to see it) that if an isoc OUT SSPLIT
> is scheduled for both uframes N and N+1, then in uframe N it must be
> scheduled for 188 bytes.  Ergo, it has to occupy the entire uframe.

Clearly; the TT can't do only part of an isoc transfer, then do
another transfer, then go back to the first transfer.  It has to do
the entire isoc transfer on the full-speed bus as a single transfer.
It breaks that up on the high-speed side by uframes.

> (By the way, this is a stronger requirement than saying that transfers
> larger than 188 bytes have to start at the beginning of a uframe.  It
> also rules out the possibility of allowing a small transfer to occupy
> parts of two successive uframes.)

No, this is incorrect.  A transfer can "carry over" into the next
uframe.  The TT performs the low/full-speed transfers on a queued
basis.

> On the other hand, I still don't see how this patch would fix the bug
> as originally reported.

I'm not sure what you are missing? ;-)  The carryover bandwidth logic
prevents any transfer from happening (in best case) more than 1 uframe
after its SSPLIT, with the precondition that no transfer is more than
1 uframe.  The special-case of transfers larger than 1 uframe is
handled by scheduling them only in empty uframes (except the partially
used uframe).  The bug of -1 misses the last fully-used uframe.
Removing the -1 fixes this.

In fact it fixes the problem so it does exactly what you just said; it
enforces scheduling multi-uframe transfers only starting in an empty
uframe, with empty uframes for each of the following fully-used
uframes, except the last partially-used uframe.  Currently, the code
misses the last fully-used uframe because of the -1 bug.

> At least one loose end doesn't seem to have been
> mentioned anywhere: The USB spec clearly states that periodic split
> transactions must be scheduled so as not to exceed the relevant time
> limits on both the full/low-speed bus _and_ the high-speed bus.
> AFAICT, ehci-sched.c doesn't calculate the bandwidth required on the
> high-speed bus for split transactions.

That may be, I only added the low/full-speed bandwidth management.  I
did not do any high-speed bandwidth management changes.  But that does
not have anything to do with the bug being discussed. :-)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux