2005-12-26 10:24:59

by Bodo Eggert

[permalink] [raw]
Subject: [PATCH] USB_BANDWIDTH documentation change

Document the current status of CONFIG_USB_BANDWITH implementation.

Signed-Off-By: Bodo Eggert <[email protected]>
---
As discussed in news:[email protected], USB_BANDWIDTH does not
work correctly. I took the time to add this fact to the documentation.

--- 2.6.14/drivers/usb/core/Kconfig.ori 2005-12-26 10:44:49.000000000 +0100
+++ 2.6.14/drivers/usb/core/Kconfig 2005-12-26 10:49:05.000000000 +0100
@@ -49,6 +49,11 @@ config USB_BANDWIDTH
about USB bandwidth usage to be logged and some devices or
drivers may not work correctly.

+ Currently this option does not work with uhci-hcd (never worked) or
+ ehci-hcd (problems involved with scheduling periodic transfers to a
+ full-speed device connected through a high-speed hub). Don't enable
+ this option for these controlers.
+
config USB_DYNAMIC_MINORS
bool "Dynamic USB minor allocation (EXPERIMENTAL)"
depends on USB && EXPERIMENTAL

--
A. Top posters
Q. What's the most annoying thing on Usenet?


2005-12-26 15:16:48

by Lee Revell

[permalink] [raw]
Subject: Re: [PATCH] USB_BANDWIDTH documentation change

On Mon, 2005-12-26 at 11:25 +0100, Bodo Eggert wrote:
> Document the current status of CONFIG_USB_BANDWITH implementation.

Since most systems use uhci-hcd and/or ehci-hcd maybe we should just
mark it BROKEN? Or EXPERIMENTAL?

Lee

2005-12-26 21:48:18

by Bodo Eggert

[permalink] [raw]
Subject: Re: [PATCH] USB_BANDWIDTH documentation change

On Mon, 26 Dec 2005, Lee Revell wrote:
> On Mon, 2005-12-26 at 11:25 +0100, Bodo Eggert wrote:

> > Document the current status of CONFIG_USB_BANDWITH implementation.
>
> Since most systems use uhci-hcd and/or ehci-hcd maybe we should just
> mark it BROKEN? Or EXPERIMENTAL?

It is EXPERIMENTAL, but the current documentation sounds like "YOU REALLY
WANT THIS !!!1", and I /guess/ that would be true for ohci-hcd users.

--
The complexity of a weapon is inversely proportional to the IQ of the
weapon's operator.

2005-12-26 22:35:31

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH] USB_BANDWIDTH documentation change

On Mon, 26 Dec 2005, Bodo Eggert wrote:

> On Mon, 26 Dec 2005, Lee Revell wrote:
> > On Mon, 2005-12-26 at 11:25 +0100, Bodo Eggert wrote:
>
> > > Document the current status of CONFIG_USB_BANDWITH implementation.
> >
> > Since most systems use uhci-hcd and/or ehci-hcd maybe we should just
> > mark it BROKEN? Or EXPERIMENTAL?
>
> It is EXPERIMENTAL, but the current documentation sounds like "YOU REALLY
> WANT THIS !!!1", and I /guess/ that would be true for ohci-hcd users.

CONFIG_USB_BANDWIDTH isn't _really_ needed. What it does (or rather, what
it would do if it worked properly) is prevent the kernel from
overcommitting on USB bandwidth.

If it's not set, the kernel will allow drivers to reserve more bandwidth
than is actually available, with the result that data transfers will fail.
If it is set, drivers will not be allowed to reserve too much bandwidth,
so again the I/O will fail (but at an earlier stage).

Provided drivers never try to overcommit, it doesn't matter whether the
option is set or not. And note that a single driver is most unlikely to
overcommit on bandwidth; the problems arise when you have more than one
driver all trying to use a lot of bandwidth at the same time.

Alan Stern

2005-12-27 04:20:04

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] USB_BANDWIDTH documentation change

On Mon, Dec 26, 2005 at 05:35:29PM -0500, Alan Stern wrote:
> On Mon, 26 Dec 2005, Bodo Eggert wrote:
>
> > On Mon, 26 Dec 2005, Lee Revell wrote:
> > > On Mon, 2005-12-26 at 11:25 +0100, Bodo Eggert wrote:
> >
> > > > Document the current status of CONFIG_USB_BANDWITH implementation.
> > >
> > > Since most systems use uhci-hcd and/or ehci-hcd maybe we should just
> > > mark it BROKEN? Or EXPERIMENTAL?
> >
> > It is EXPERIMENTAL, but the current documentation sounds like "YOU REALLY
> > WANT THIS !!!1", and I /guess/ that would be true for ohci-hcd users.
>
> CONFIG_USB_BANDWIDTH isn't _really_ needed. What it does (or rather, what
> it would do if it worked properly) is prevent the kernel from
> overcommitting on USB bandwidth.

I just saw (but can't find again, sorry) a gentoo bug of an external usb
driver on x86-64 that oopses _unless_ this config option is set. So for
some people it is necessary and not broken.

thanks,

greg k-h

2005-12-27 17:28:32

by David Brownell

[permalink] [raw]
Subject: Re: [linux-usb-devel] Re: [PATCH] USB_BANDWIDTH documentation change

On Monday 26 December 2005 8:17 pm, Greg KH wrote:
>
> I just saw (but can't find again, sorry) a gentoo bug of an external usb
> driver on x86-64 that oopses _unless_ this config option is set. So for
> some people it is necessary and not broken.

USB should never overcommit. IMO the right answer is to remove the option
and always check. And also remove the old usb_check_bandwidth() call, which
doesn't even have an accurate model for that reservation.

- Dave

2005-12-27 17:28:05

by David Brownell

[permalink] [raw]
Subject: Re: [linux-usb-devel] Re: [PATCH] USB_BANDWIDTH documentation change

On Monday 26 December 2005 2:35 pm, Alan Stern wrote:
s.
>
> CONFIG_USB_BANDWIDTH isn't _really_ needed.

I think it was there historically because the first implementations
didn't work correctly. In fact, the model underlying that current
usb_check_bandwidth() call is incorrect ... reservations for periodic
bandwidth (isochronous and interrupt transfers) are per-endpoint,
not per-urb.


> What it does (or rather, what
> it would do if it worked properly) is prevent the kernel from
> overcommitting on USB bandwidth.

It's also completly ignored for

- ohci-hcd, which never overcommits;
- sl811-hcd, works just like ohci in that respect;
- isp116x-hcd, ditto;
- ehci-hcd, can't risk overcommit with transaction translators(*);

The only HCDs that use usb_check_bandwidth() are the CRIS HCD
(which, last I heard, neither built nor, after fixing build errors,
worked) and UHCI. Which is why this patch is incorrect ...

The long term solution is to get rid of that CONFIG_ symbol and
the code backing it, and then have all the HCD properly reserve
periodic bandwidth, using a per-endpoint approach.

- Dave

(*) The issues folk have mentioned with bandwidth reservation for
EHCI are more "full and low speed devcies can't use all the
available transaction translator bandwidth" than anything else.
As a rule high speed devices don't see such issues ... only the
needing more complex scheduling models than usb_check_bandwidth
supports, just to work -- in even simple scenarios. Which is why
EHCI never has/will use the code now protected by USB_BANDWIDTH.


2005-12-29 19:42:04

by Dan Streetman

[permalink] [raw]
Subject: EHCI TT bandwidth (was Re: [PATCH] USB_BANDWIDTH documentation change)


On Tue, 27 Dec 2005, David Brownell wrote:

>(*) The issues folk have mentioned with bandwidth reservation for
> EHCI are more "full and low speed devcies can't use all the
> available transaction translator bandwidth" than anything else.

The patches I just sent to the linux-usb-devel list (couple days ago) take
care of those scheduling restrictions...do you have any comments on them?
It would be great to get them in the kernel so EHCI can fully schedule any
lowspeed or fullspeed buses that it manages. I even put the changes
inside a kernel CONFIG option so people can test out the patches fully
before replacing the old model.




--
Dan Streetman
[email protected]
---------------------
186,272 miles per second:
It isn't just a good idea, it's the law!

2005-12-29 19:59:49

by Lee Revell

[permalink] [raw]
Subject: Re: EHCI TT bandwidth (was Re: [PATCH] USB_BANDWIDTH documentation change)

On Thu, 2005-12-29 at 14:41 -0500, Dan Streetman wrote:
> On Tue, 27 Dec 2005, David Brownell wrote:
>
> >(*) The issues folk have mentioned with bandwidth reservation for
> > EHCI are more "full and low speed devcies can't use all the
> > available transaction translator bandwidth" than anything else.
>
> The patches I just sent to the linux-usb-devel list (couple days ago) take
> care of those scheduling restrictions...do you have any comments on them?
> It would be great to get them in the kernel so EHCI can fully schedule any
> lowspeed or fullspeed buses that it manages. I even put the changes
> inside a kernel CONFIG option so people can test out the patches fully
> before replacing the old model.
>

How do I test them? Should this make USB audio work with
CONFIG_USB_BANDWIDTH?

Lee

2005-12-29 20:12:51

by David Brownell

[permalink] [raw]
Subject: Re: EHCI TT bandwidth (was Re: [PATCH] USB_BANDWIDTH documentation change)

On Thursday 29 December 2005 11:41 am, Dan Streetman wrote:
>
> On Tue, 27 Dec 2005, David Brownell wrote:
>
> >(*) The issues folk have mentioned with bandwidth reservation for
> > EHCI are more "full and low speed devcies can't use all the
> > available transaction translator bandwidth" than anything else.
>
> The patches I just sent to the linux-usb-devel list (couple days ago) take
> care of those scheduling restrictions...do you have any comments on them?

Those are the ones I was thinking of, yes. I need to make time to
look at those (even try them out!) but I don't have it just now.

Though I did notice you were using microframe "7" as an error code;
best to have some #defined constant, and one that would later allow
for use of FSTN nodes in the periodic schedule.

If other folk sent testing reports in, that'd help. It'd affect
full speed devices down-stream of high speed hubs, when they use
isochronous transfers (audio, video) or interrupt ones (mostly
hubs and input devices like keyboards, mice, tablets). And the
test cases of most interest would include several of those being
used at the same time ...


> It would be great to get them in the kernel so EHCI can fully schedule any
> lowspeed or fullspeed buses that it manages. I even put the changes
> inside a kernel CONFIG option so people can test out the patches fully
> before replacing the old model.

I'll likely OK that for merge into 2.6.16 without the Kconfig option.
Just replace the old scheduling with the newer stuff; no point in
trying to sort out two different sets of hard-to-track lowlevel bugs.

- Dave

2005-12-30 00:56:23

by Dan Streetman

[permalink] [raw]
Subject: Re: EHCI TT bandwidth (was Re: [PATCH] USB_BANDWIDTH documentation change)


On Thu, 29 Dec 2005, David Brownell wrote:

>Though I did notice you were using microframe "7" as an error code;
>best to have some #defined constant, and one that would later allow
>for use of FSTN nodes in the periodic schedule.

Sounds good, feel free to make any changes you want of course :)
or if you have other comments and/or want me to resend the patches let me
know.


--
Dan Streetman
[email protected]
---------------------
186,272 miles per second:
It isn't just a good idea, it's the law!

2005-12-30 19:13:23

by Dan Streetman

[permalink] [raw]
Subject: Re: [linux-usb-devel] Re: EHCI TT bandwidth (was Re: [PATCH] USB_BANDWIDTH documentation change)


On Thu, 29 Dec 2005, Lee Revell wrote:

>How do I test them? Should this make USB audio work with
>CONFIG_USB_BANDWIDTH?

It won't have any effect on CONFIG_USB_BANDWIDTH, as the EHCI transaction
translator scheudling code doesn't care about that config setting. This
also won't have any effect on USB 2.0 devices (e.g. a highspeed Audio
device).

The updates will only help in the situation where there are multiple
lowpseed or fullspeed devices with periodic endpoints, all connected to
the same USB 2.0 (highspeed) hub. In that situation it's possible to
"fill up" the USB 2.0 hub's transaction translator periodic schedule with
only a few devices. The updates allow many more devices to fit in the
TT's periodic schedule. The specific number of devices depends on how
many periodic endpoints, those endpoint's poll rates, and their max packet
sizes.



--
Dan Streetman
[email protected]
---------------------
186,272 miles per second:
It isn't just a good idea, it's the law!

2005-12-30 19:16:41

by Lee Revell

[permalink] [raw]
Subject: Re: [linux-usb-devel] Re: EHCI TT bandwidth (was Re: [PATCH] USB_BANDWIDTH documentation change)

On Fri, 2005-12-30 at 14:13 -0500, Dan Streetman wrote:
> On Thu, 29 Dec 2005, Lee Revell wrote:
>
> >How do I test them? Should this make USB audio work with
> >CONFIG_USB_BANDWIDTH?
>
> It won't have any effect on CONFIG_USB_BANDWIDTH, as the EHCI transaction
> translator scheudling code doesn't care about that config setting. This
> also won't have any effect on USB 2.0 devices (e.g. a highspeed Audio
> device).
>
> The updates will only help in the situation where there are multiple
> lowpseed or fullspeed devices with periodic endpoints, all connected to
> the same USB 2.0 (highspeed) hub. In that situation it's possible to
> "fill up" the USB 2.0 hub's transaction translator periodic schedule with
> only a few devices. The updates allow many more devices to fit in the
> TT's periodic schedule. The specific number of devices depends on how
> many periodic endpoints, those endpoint's poll rates, and their max packet
> sizes.
>

OK. For some reason I though that problem was fixed, I guess it was a
different issue. ALSA users previously reported that a full speed audio
device didn't work at all through a high speed hub at all but that it
was fixed a few months ago.

Lee