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 Thu, 16 Apr 2009, Dan Streetman wrote:

> > To put it another way...  Suppose isoc-OUT transfer A has length 50 and
> > its SSPLIT is scheduled in uframe 0, and you want now to schedule
> > isoc-OUT transfer B.  If B has length 180, then you're saying it's okay
> > to schedule B's SSPLIT in uframe 0 also, letting the TT buffer the
> > excess 42 output bytes across the uframe boundary.
> 
> I believe this is ok per USB spec, and this is how the current code
> should behave.

Okay, I'll go along with you on this.

> > But if B has length 190, then according to what you wrote earlier it's
> > _not_ okay to schedule B's first SSPLIT in uframe 0 (with the second
> > 2-byte SSPLIT in uframe 1), letting the TT buffer the excess 50 output
> > bytes over the uframe boundary.  What reason is there for this
> > difference?  Don't the two cases appear pretty much the same to the TT?
> 
> I believe this is ok per USB spec, but the current code does not do
> this simply because it was easier to calculate availability by putting
> a transfer larger than 1 uframe into an empty uframe.  I do not think
> that the USB spec mandates putting a > 1uframe transfer into its own
> empty starting uframe (although for isoc OUT you do have to
> SSPLIT-begin with 188 bytes, but I believe you can do this even if
> there is another SSPLIT in that same uframe).

I still think the logic in tt_available() is inadequate.

Let's consider another example.  Suppose transfer A (length 100) is
scheduled in uframe 0, transfer C (length 180) is scheduled in uframe
1, and transfer D (length 4) is also scheduled in uframe 1.  Now the
caller wants to know if transfer B, with length 100, can be inserted in
uframe 0.

Since B's length is < 125 usec, the tests for fully-occupied uframes
don't get applied.  The overflow handler sees that uframe 0 can't hold
100 + 100 bytes, so the last 12 bytes of B are carried over to uframe
1.  Now uframe 1 has got 12 + 180 + 4 bytes, so the last 4 bytes of C
and all of D are carried over to uframe 2.  But now the start of D is
delayed past the end of the uframe when it was supposed to occur!  
That shouldn't be allowed to happen.  Yes, the TT probably could handle 
it okay, but it's still a bug.

The problem is that tt_available() doesn't keep track of the amount of
time allocated to each transfer in each uframe; it only keeps track of
the total time allocated in each uframe.  That's why you have those
peculiar heuristics for dealing with multi-uframe transfers.  You
wouldn't need them if you kept the additional information.

The real criterion should be that inserting the new transfer at the
requested spot mustn't push the start of any other transfer back into a
different uframe from where it currently is.  (Strictly speaking, it's
also necessary to check that the end of any other isoc-IN transfer
doesn't get pushed back into a different uframe, because that would 
require a change to the C-mask.)

See how this works:  Suppose short transfer B is already scheduled in
uframe 1, and you want to add 376-byte transfer A in uframe 0.  Since A
would then occupy all of uframes 0 and 1, the start of B would be
pushed back to uframe 2, so the insertion isn't allowed.  Conversely,
suppose short transfer B is already scheduled in uframe 0 and you want
to add 376-byte transfer A in uframe 0.  If A comes before B then the
same reasoning as before shows the insertion isn't allowed.  But if A
is inserted after B in the uframe then there's no violation.

Also, your criterion for detecting when the frame is overfull is wrong.  
You've got it set for a maximum of 30 usec in uframe 6.  But in fact
it's allowable to use up all of uframe 6 (and indeed, even part of
uframe 7), so long as the total periodic bandwidth is under the 90%
limit.  Your code enforces a limit of 6*125 + 30 = 780 usec instead of 
the correct limit of 900 usec.

Alan Stern

--
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