[Patch] Increase USBFS Bulk Transfer size

[Patch] Increase USBFS Bulk Transfer size

[Markus Rechberger]

[-- Attachment #1: Type: text/plain, Size: 993 bytes --]

We have 2 products which can perform better with increased Bulk transfers

Device No. 1:
According to the hardware spec of on of our product
Available Bulk Transfer Size are:
- 188 * n bytes, where n = 1 ~ 256.

Although we can drive that one with 15K as well when setting the HW
register down to it.

Device No. 2
only creates jitter video with Bulk transfer sizes which are below
24064 bytes, no such chipfeature is available
to decrease the bulk transfer size.
http://sundtek.de/images/dtvjitter2.jpg
with transfer size of 24064:
http://sundtek.de/images/gooddata.jpg

The patch takes the features of Device No. 1 into account allowing a
maximum buffer of 48128 bytes.

Those issues have been evaluated with MacOSX and a customized patched
Linux version.
Device No. 2 also corrupts on MacOSX with too small packet sizes,
Windows and Mac are using 24064 bytes.

Default Bulk Transfersize of device No. 1 is around 1-2k which leads
to very high cpu usage, updating it to 15k lowers that one.

[-- Attachment #2: devio.c.diff --]
[-- Type: text/x-patch, Size: 336 bytes --]

--- ./drivers/usb/core/devio.c_old	2011-10-12 14:23:34.000000000 +0200
+++ ./drivers/usb/core/devio.c	2011-10-12 14:25:30.000000000 +0200
@@ -107,7 +107,7 @@
 
 #define USB_DEVICE_DEV		MKDEV(USB_DEVICE_MAJOR, 0)
 
-#define	MAX_USBFS_BUFFER_SIZE	16384
+#define	MAX_USBFS_BUFFER_SIZE	48128
 
 
 static int connected(struct dev_state *ps)

Re: [Patch] Increase USBFS Bulk Transfer size

[Markus Rechberger]

On Wed, Oct 12, 2011 at 2:36 PM, Markus Rechberger
<mrechberger@gmail.com> wrote:
> We have 2 products which can perform better with increased Bulk transfers
>
> Device No. 1:
> According to the hardware spec of on of our product
> Available Bulk Transfer Size are:
> - 188 * n bytes, where n = 1 ~ 256.
>
> Although we can drive that one with 15K as well when setting the HW
> register down to it.
>
> Device No. 2
> only creates jitter video with Bulk transfer sizes which are below
> 24064 bytes, no such chipfeature is available
> to decrease the bulk transfer size.
> http://sundtek.de/images/dtvjitter2.jpg
> with transfer size of 24064:
> http://sundtek.de/images/gooddata.jpg
>
> The patch takes the features of Device No. 1 into account allowing a
> maximum buffer of 48128 bytes.
>
> Those issues have been evaluated with MacOSX and a customized patched
> Linux version.
> Device No. 2 also corrupts on MacOSX with too small packet sizes,
> Windows and Mac are using 24064 bytes.
>
> Default Bulk Transfersize of device No. 1 is around 1-2k which leads
> to very high cpu usage, updating it to 15k lowers that one.
>


Signed-off-by: Markus Rechberger <mrechberger@gmail.com>

Re: [Patch] Increase USBFS Bulk Transfer size

[Sergei Shtylyov]

On 10/12/2011 04:36 PM, Markus Rechberger wrote:

> We have 2 products which can perform better with increased Bulk transfers

> Device No. 1:
> According to the hardware spec of on of our product
> Available Bulk Transfer Size are:
> - 188 * n bytes, where n = 1 ~ 256.

> Although we can drive that one with 15K as well when setting the HW
> register down to it.

> Device No. 2
> only creates jitter video with Bulk transfer sizes which are below
> 24064 bytes, no such chipfeature is available
> to decrease the bulk transfer size.
> http://sundtek.de/images/dtvjitter2.jpg
> with transfer size of 24064:
> http://sundtek.de/images/gooddata.jpg

> The patch takes the features of Device No. 1 into account allowing a
> maximum buffer of 48128 bytes.

> Those issues have been evaluated with MacOSX and a customized patched
> Linux version.
> Device No. 2 also corrupts on MacOSX with too small packet sizes,
> Windows and Mac are using 24064 bytes.

    You are constantly mixing the packet size and the transfer size.

> Default Bulk Transfersize of device No. 1 is around 1-2k which leads
> to very high cpu usage, updating it to 15k lowers that one.

WBR, Sergei

Re: [Patch] Increase USBFS Bulk Transfer size

[Greg KH]

On Wed, Oct 12, 2011 at 02:36:59PM +0200, Markus Rechberger wrote:
> We have 2 products which can perform better with increased Bulk transfers

I don't believe you :)

As stated before, this patch is not acceptable.  Please work to figure
out the real reason for your device problems here, this is not the
correct solution at all.

sorry,

greg k-h

Re: [Patch] Increase USBFS Bulk Transfer size

[Markus Rechberger]

On Wed, Oct 12, 2011 at 4:17 PM, Greg KH <gregkh@suse.de> wrote:
> On Wed, Oct 12, 2011 at 02:36:59PM +0200, Markus Rechberger wrote:
>> We have 2 products which can perform better with increased Bulk transfers
>
> I don't believe you :)
>
> As stated before, this patch is not acceptable.  Please work to figure
> out the real reason for your device problems here, this is not the
> correct solution at all.
>

OK no device support for linux then. Windows and MacOSX are fine.

Let me know when you are in Berlin I can give you the device and you
can figure it out by yourself
I'm done with talking about those issues now since I spent too much
time on figuring out
how to get this device work. I guess after 3 years successfully
working with USBFS you can assume that I have
some experience with using this API. If Linux cannot offer support for
this not my fault.

Great achievement!

Markus
> sorry,
>
> greg k-h
>

Re: [Patch] Increase USBFS Bulk Transfer size

[Mihai Moldovan]

[-- Attachment #1: Type: text/plain, Size: 1149 bytes --]

Hi Greg,

* On 12.10.2011 04:17 PM, Greg KH wrote:
> As stated before, this patch is not acceptable. Please work to figure
> out the real reason for your device problems here, this is not the
> correct solution at all.

I've gone through this whole (and previous) thread, but couldn't find a
real argument why this is so wrong.
So far everybody has argued that it's 'wrong' and may break older user
code. The latter argument even is wrong, as drivers not requiring a
higher bulk transfer size just aren't affected.

This being said, I agree that allocating more memory than needed is
wasting memory and bad, if it can be avoided. On the other hand, we're
talking about very few devices here and not several ten of MB system
memory being wasted by all bulk transfers in total.
I basically see two cases:
  - systems with a few MB of RAM. I highly doubt those use usbfs anyway
(usually other stuff like usb-storage)
  - systems with 'enough' system memory. In this case the problem is
purely... let's call it 'academic'. You can discuss it lengthly in
theory, but won't even notice it in practice.

Best regards,


Mihai


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4369 bytes --]

Re: [Patch] Increase USBFS Bulk Transfer size

[Greg KH]

On Wed, Oct 12, 2011 at 06:59:01PM +0200, Markus Rechberger wrote:
> On Wed, Oct 12, 2011 at 4:17 PM, Greg KH <gregkh@suse.de> wrote:
> > On Wed, Oct 12, 2011 at 02:36:59PM +0200, Markus Rechberger wrote:
> >> We have 2 products which can perform better with increased Bulk transfers
> >
> > I don't believe you :)
> >
> > As stated before, this patch is not acceptable.  Please work to figure
> > out the real reason for your device problems here, this is not the
> > correct solution at all.
> >
> 
> OK no device support for linux then. Windows and MacOSX are fine.

That is your choice, not ours, as you are the one writing the closed
source code.  Without usbmon dumps, showing that the problem really is
in the kernel code, we can't do anything for you here.

Best of luck,

greg k-h

Re: [Patch] Increase USBFS Bulk Transfer size

[Greg KH]

On Wed, Oct 12, 2011 at 08:00:38PM +0200, Mihai Moldovan wrote:
> Hi Greg,
> 
> * On 12.10.2011 04:17 PM, Greg KH wrote:
> > As stated before, this patch is not acceptable. Please work to figure
> > out the real reason for your device problems here, this is not the
> > correct solution at all.
> 
> I've gone through this whole (and previous) thread, but couldn't find a
> real argument why this is so wrong.

Because it does nothing except increase kernel memory pressure.  Why not
increase it to 1Gb while we are at it, it realistically makes no
difference.

And so, because it makes no difference, we should not change the
existing value as there seems to be something else wrong with what is
going on here in the userspace code that can't handle smaller buffer
sizes.

Within the kernel, and the host controller driver, the memory is split
up into much smaller pieces anyway, so there should not be any
difference at all, if you write your userspace code correctly, that the
USB device would see anything different with a bigger (or even smaller)
usbfs buffer size.

> So far everybody has argued that it's 'wrong' and may break older user
> code. The latter argument even is wrong, as drivers not requiring a
> higher bulk transfer size just aren't affected.
> 
> This being said, I agree that allocating more memory than needed is
> wasting memory and bad, if it can be avoided. On the other hand, we're
> talking about very few devices here and not several ten of MB system
> memory being wasted by all bulk transfers in total.
> I basically see two cases:
>   - systems with a few MB of RAM. I highly doubt those use usbfs anyway
> (usually other stuff like usb-storage)

Not true at all, I know of some using usbfs quite well, and have been
since the 2.2 kernel days.

greg k-h

Re: [Patch] Increase USBFS Bulk Transfer size

[Markus Rechberger]

On Wed, Oct 12, 2011 at 10:33 PM, Greg KH <gregkh@suse.de> wrote:
> On Wed, Oct 12, 2011 at 06:59:01PM +0200, Markus Rechberger wrote:
>> On Wed, Oct 12, 2011 at 4:17 PM, Greg KH <gregkh@suse.de> wrote:
>> > On Wed, Oct 12, 2011 at 02:36:59PM +0200, Markus Rechberger wrote:
>> >> We have 2 products which can perform better with increased Bulk transfers
>> >
>> > I don't believe you :)
>> >
>> > As stated before, this patch is not acceptable.  Please work to figure
>> > out the real reason for your device problems here, this is not the
>> > correct solution at all.
>> >
>>
>> OK no device support for linux then. Windows and MacOSX are fine.
>
> That is your choice, not ours, as you are the one writing the closed
> source code.  Without usbmon dumps, showing that the problem really is
> in the kernel code, we can't do anything for you here.
>


here take your usbmon logs:
http://sundtek.de/support/working.log
http://sundtek.de/support/notworking.log (this is with your proposed
split up of 22k).

Isochronous already supports up to 190kb buffers which cause no
trouble anywhere.
Bulk is castrated to 15kb buffers and 50kb should be such a big issue
while applications
which do not request it are not affected at all.

I'm very angry that you do not really focus on what I write and just
try to write stories about
bogus things like my patch might break other applications while I
pointed out that this is not
possible with legacy applications which use this interface. It is not
even possible to read the
maximum buffer value from the kernelspace.

Your bogus message about 1Gig ram you can put it elsewhere, I checked
multiple windows drivers
in the past the buffer size varies between small and something around
50k usually.
I would fully like to avoid to patch this kernel to get those things
work because I don't want to
deal with people who just write around the actual issues.

Alsi the fact that bigger buffers are already being used plus the HW
specifications of our main device also points out to a certain
buffersize near 50k. But of course you are the smart knowitall without
the need of explanation why
those things can be affected by HW and some HW chips have options to
influence that buffer size.

I'm certainly angry,
Markus

> Best of luck,
>
> greg k-h
>

Re: [Patch] Increase USBFS Bulk Transfer size

[Markus Rechberger]

On Wed, Oct 12, 2011 at 11:48 PM, Markus Rechberger
<mrechberger@gmail.com> wrote:
> On Wed, Oct 12, 2011 at 10:33 PM, Greg KH <gregkh@suse.de> wrote:
>> On Wed, Oct 12, 2011 at 06:59:01PM +0200, Markus Rechberger wrote:
>>> On Wed, Oct 12, 2011 at 4:17 PM, Greg KH <gregkh@suse.de> wrote:
>>> > On Wed, Oct 12, 2011 at 02:36:59PM +0200, Markus Rechberger wrote:
>>> >> We have 2 products which can perform better with increased Bulk transfers
>>> >
>>> > I don't believe you :)
>>> >
>>> > As stated before, this patch is not acceptable.  Please work to figure
>>> > out the real reason for your device problems here, this is not the
>>> > correct solution at all.
>>> >
>>>
>>> OK no device support for linux then. Windows and MacOSX are fine.
>>
>> That is your choice, not ours, as you are the one writing the closed
>> source code.  Without usbmon dumps, showing that the problem really is
>> in the kernel code, we can't do anything for you here.
>>
>
>
> here take your usbmon logs:
> http://sundtek.de/support/working.log
> http://sundtek.de/support/notworking.log (this is with your proposed
> split up of 22k).
>
> Isochronous already supports up to 190kb buffers which cause no
> trouble anywhere.
> Bulk is castrated to 15kb buffers and 50kb should be such a big issue
> while applications
> which do not request it are not affected at all.
>
> I'm very angry that you do not really focus on what I write and just
> try to write stories about
> bogus things like my patch might break other applications while I
> pointed out that this is not
> possible with legacy applications which use this interface. It is not
> even possible to read the
> maximum buffer value from the kernelspace.
>
> Your bogus message about 1Gig ram you can put it elsewhere, I checked
> multiple windows drivers
> in the past the buffer size varies between small and something around
> 50k usually.
> I would fully like to avoid to patch this kernel to get those things
> work because I don't want to
> deal with people who just write around the actual issues.
>
> Alsi the fact that bigger buffers are already being used plus the HW
> specifications of our main device also points out to a certain
> buffersize near 50k. But of course you are the smart knowitall without
> the need of explanation why
> those things can be affected by HW and some HW chips have options to
> influence that buffer size.

And for those who are curious about the logfiles:
Not working one as proposed by Alan that the full buffer size should
be split into 2 requests:

ffff8800b38d9f00 1231540351 S Bi:2:013:1 -115 12288 <
ffff8800b38d96c0 1231540404 S Bi:2:013:1 -115 11776 <
ffff8800b38d9cc0 1231540440 S Bi:2:013:1 -115 12288 <
ffff880002ede600 1231540496 S Bi:2:013:1 -115 11776 <
ffff8800b38d9f00 1231545491 C Bi:2:013:1 0 12288 = 7a1a0840 1ca8353c
004b80ec 4de08401 2f0227f8 34005e7e 80181dfd 1a8f700a
ffff88007d51fb40 1231545875 S Bi:2:013:1 -115 12288 <
ffff8800b38d96c0 1231551861 C Bi:2:013:1 0 11776 = 5244e386 a7800000
010642ea 35bfbba5 373e738b cc035a73 c328a1ff 4da728ce
ffff880002ede0c0 1231556173 S Bi:2:013:1 -115 11776 <
ffff8800b38d9cc0 1231558618 C Bi:2:013:1 0 12288 = db91aae9 2d2532f3
2e37448a fb36c213 55dda2ad 243122b2 261edb06 875848ac


12288 = 7a1a0840 every Line with 12288 should start with 47 (MPEG-TS Sync byte)

The working one which allocate 7000 bytes more (oh really big memory
pressure now!)  than this castrated USBFS interface allows.

ffff88003ac37240 992178919 S Bi:2:013:1 -115 24064 <
ffff88003ac37000 992178953 S Bi:2:013:1 -115 24064 <
ffff88003ac37c00 992178980 S Bi:2:013:1 -115 24064 <
ffff88003ac37e40 992179003 S Bi:2:013:1 -115 24064 <
ffff88003ac37240 992190198 C Bi:2:013:1 0 24064 = 471fff10 00000000
00000000 00000000 00000000 00000000 00000000 00000000
ffff88000601f480 992194368 S Bi:2:013:1 -115 24064 <
ffff88003ac37000 992203209 C Bi:2:013:1 0 24064 = 4701b114 43867ee6
40790660 e898681c 9b1c7dca 08980d43 73181369 9be1bc67
ffff880067e38a80 992204642 S Bi:2:013:1 -115 24064 <
ffff88003ac37c00 992216318 C Bi:2:013:1 0 24064 = 4740001c 0000b01d
0305d500 000000e0 104015e1 504016e1 60401be1 b04022e2
ffff880067e383c0 992219978 S Bi:2:013:1 -115 24064 <
ffff88003ac37e40 992229340 C Bi:2:013:1 0 24064 = 471fff10 00000000
00000000 00000000 00000000 00000000 00000000 00000000

everything starts nicely with 0x47

And now everyone again is clueless how this can happen.. surprise surprise..

Markus

Re: [Patch] Increase USBFS Bulk Transfer size

[Manu Abraham]

Hello Markus,

On Thu, Oct 13, 2011 at 3:39 AM, Markus Rechberger
<mrechberger@gmail.com> wrote:
> On Wed, Oct 12, 2011 at 11:48 PM, Markus Rechberger
> <mrechberger@gmail.com> wrote:
>> On Wed, Oct 12, 2011 at 10:33 PM, Greg KH <gregkh@suse.de> wrote:
>>> On Wed, Oct 12, 2011 at 06:59:01PM +0200, Markus Rechberger wrote:
>>>> On Wed, Oct 12, 2011 at 4:17 PM, Greg KH <gregkh@suse.de> wrote:
>>>> > On Wed, Oct 12, 2011 at 02:36:59PM +0200, Markus Rechberger wrote:
>>>> >> We have 2 products which can perform better with increased Bulk transfers
>>>> >
>>>> > I don't believe you :)
>>>> >
>>>> > As stated before, this patch is not acceptable.  Please work to figure
>>>> > out the real reason for your device problems here, this is not the
>>>> > correct solution at all.
>>>> >
>>>>
>>>> OK no device support for linux then. Windows and MacOSX are fine.
>>>
>>> That is your choice, not ours, as you are the one writing the closed
>>> source code.  Without usbmon dumps, showing that the problem really is
>>> in the kernel code, we can't do anything for you here.
>>>
>>
>>
>> here take your usbmon logs:
>> http://sundtek.de/support/working.log
>> http://sundtek.de/support/notworking.log (this is with your proposed
>> split up of 22k).
>>
>> Isochronous already supports up to 190kb buffers which cause no
>> trouble anywhere.
>> Bulk is castrated to 15kb buffers and 50kb should be such a big issue
>> while applications
>> which do not request it are not affected at all.
>>
>> I'm very angry that you do not really focus on what I write and just
>> try to write stories about
>> bogus things like my patch might break other applications while I
>> pointed out that this is not
>> possible with legacy applications which use this interface. It is not
>> even possible to read the
>> maximum buffer value from the kernelspace.
>>
>> Your bogus message about 1Gig ram you can put it elsewhere, I checked
>> multiple windows drivers
>> in the past the buffer size varies between small and something around
>> 50k usually.
>> I would fully like to avoid to patch this kernel to get those things
>> work because I don't want to
>> deal with people who just write around the actual issues.
>>
>> Alsi the fact that bigger buffers are already being used plus the HW
>> specifications of our main device also points out to a certain
>> buffersize near 50k. But of course you are the smart knowitall without
>> the need of explanation why
>> those things can be affected by HW and some HW chips have options to
>> influence that buffer size.
>
> And for those who are curious about the logfiles:
> Not working one as proposed by Alan that the full buffer size should
> be split into 2 requests:
>
> ffff8800b38d9f00 1231540351 S Bi:2:013:1 -115 12288 <
> ffff8800b38d96c0 1231540404 S Bi:2:013:1 -115 11776 <
> ffff8800b38d9cc0 1231540440 S Bi:2:013:1 -115 12288 <
> ffff880002ede600 1231540496 S Bi:2:013:1 -115 11776 <
> ffff8800b38d9f00 1231545491 C Bi:2:013:1 0 12288 = 7a1a0840 1ca8353c
> 004b80ec 4de08401 2f0227f8 34005e7e 80181dfd 1a8f700a
> ffff88007d51fb40 1231545875 S Bi:2:013:1 -115 12288 <
> ffff8800b38d96c0 1231551861 C Bi:2:013:1 0 11776 = 5244e386 a7800000
> 010642ea 35bfbba5 373e738b cc035a73 c328a1ff 4da728ce
> ffff880002ede0c0 1231556173 S Bi:2:013:1 -115 11776 <
> ffff8800b38d9cc0 1231558618 C Bi:2:013:1 0 12288 = db91aae9 2d2532f3
> 2e37448a fb36c213 55dda2ad 243122b2 261edb06 875848ac
>
>
> 12288 = 7a1a0840 every Line with 12288 should start with 47 (MPEG-TS Sync byte)
>
> The working one which allocate 7000 bytes more (oh really big memory
> pressure now!)  than this castrated USBFS interface allows.
>
> ffff88003ac37240 992178919 S Bi:2:013:1 -115 24064 <
> ffff88003ac37000 992178953 S Bi:2:013:1 -115 24064 <
> ffff88003ac37c00 992178980 S Bi:2:013:1 -115 24064 <
> ffff88003ac37e40 992179003 S Bi:2:013:1 -115 24064 <
> ffff88003ac37240 992190198 C Bi:2:013:1 0 24064 = 471fff10 00000000
> 00000000 00000000 00000000 00000000 00000000 00000000
> ffff88000601f480 992194368 S Bi:2:013:1 -115 24064 <
> ffff88003ac37000 992203209 C Bi:2:013:1 0 24064 = 4701b114 43867ee6
> 40790660 e898681c 9b1c7dca 08980d43 73181369 9be1bc67
> ffff880067e38a80 992204642 S Bi:2:013:1 -115 24064 <
> ffff88003ac37c00 992216318 C Bi:2:013:1 0 24064 = 4740001c 0000b01d
> 0305d500 000000e0 104015e1 504016e1 60401be1 b04022e2
> ffff880067e383c0 992219978 S Bi:2:013:1 -115 24064 <
> ffff88003ac37e40 992229340 C Bi:2:013:1 0 24064 = 471fff10 00000000
> 00000000 00000000 00000000 00000000 00000000 00000000
>
> everything starts nicely with 0x47

Is the transfer size a multiple of 188 bytes in both cases ? In the
working case, it looks so, but in the broken case ? Many DTV bridges
have fixed DMA apertures and not variable really much, even though the
device specifications do state that it might support different block
sizes, some modes could be broken. I guess most likely, you would have
handled this situation.

Other issues that could possibly happen (wrt your jitter) is a high
latency (in the particular case of failures) where the received data
have unusable relative timestamps wrt DTS and or PTS.

The only areas where you can have a corrupted stream as you mentioned
"jitter" is either lost frames or the above two cases. I don't know
what's the issue in your case, but might help to track down the issue
altogether.

Regards,
Manu

Re: [Patch] Increase USBFS Bulk Transfer size

[Markus Rechberger]

On Thu, Oct 13, 2011 at 6:03 AM, Manu Abraham <abraham.manu@gmail.com> wrote:
> Hello Markus,
>
> On Thu, Oct 13, 2011 at 3:39 AM, Markus Rechberger
> <mrechberger@gmail.com> wrote:
>> On Wed, Oct 12, 2011 at 11:48 PM, Markus Rechberger
>> <mrechberger@gmail.com> wrote:
>>> On Wed, Oct 12, 2011 at 10:33 PM, Greg KH <gregkh@suse.de> wrote:
>>>> On Wed, Oct 12, 2011 at 06:59:01PM +0200, Markus Rechberger wrote:
>>>>> On Wed, Oct 12, 2011 at 4:17 PM, Greg KH <gregkh@suse.de> wrote:
>>>>> > On Wed, Oct 12, 2011 at 02:36:59PM +0200, Markus Rechberger wrote:
>>>>> >> We have 2 products which can perform better with increased Bulk transfers
>>>>> >
>>>>> > I don't believe you :)
>>>>> >
>>>>> > As stated before, this patch is not acceptable.  Please work to figure
>>>>> > out the real reason for your device problems here, this is not the
>>>>> > correct solution at all.
>>>>> >
>>>>>
>>>>> OK no device support for linux then. Windows and MacOSX are fine.
>>>>
>>>> That is your choice, not ours, as you are the one writing the closed
>>>> source code.  Without usbmon dumps, showing that the problem really is
>>>> in the kernel code, we can't do anything for you here.
>>>>
>>>
>>>
>>> here take your usbmon logs:
>>> http://sundtek.de/support/working.log
>>> http://sundtek.de/support/notworking.log (this is with your proposed
>>> split up of 22k).
>>>
>>> Isochronous already supports up to 190kb buffers which cause no
>>> trouble anywhere.
>>> Bulk is castrated to 15kb buffers and 50kb should be such a big issue
>>> while applications
>>> which do not request it are not affected at all.
>>>
>>> I'm very angry that you do not really focus on what I write and just
>>> try to write stories about
>>> bogus things like my patch might break other applications while I
>>> pointed out that this is not
>>> possible with legacy applications which use this interface. It is not
>>> even possible to read the
>>> maximum buffer value from the kernelspace.
>>>
>>> Your bogus message about 1Gig ram you can put it elsewhere, I checked
>>> multiple windows drivers
>>> in the past the buffer size varies between small and something around
>>> 50k usually.
>>> I would fully like to avoid to patch this kernel to get those things
>>> work because I don't want to
>>> deal with people who just write around the actual issues.
>>>
>>> Alsi the fact that bigger buffers are already being used plus the HW
>>> specifications of our main device also points out to a certain
>>> buffersize near 50k. But of course you are the smart knowitall without
>>> the need of explanation why
>>> those things can be affected by HW and some HW chips have options to
>>> influence that buffer size.
>>
>> And for those who are curious about the logfiles:
>> Not working one as proposed by Alan that the full buffer size should
>> be split into 2 requests:
>>
>> ffff8800b38d9f00 1231540351 S Bi:2:013:1 -115 12288 <
>> ffff8800b38d96c0 1231540404 S Bi:2:013:1 -115 11776 <
>> ffff8800b38d9cc0 1231540440 S Bi:2:013:1 -115 12288 <
>> ffff880002ede600 1231540496 S Bi:2:013:1 -115 11776 <
>> ffff8800b38d9f00 1231545491 C Bi:2:013:1 0 12288 = 7a1a0840 1ca8353c
>> 004b80ec 4de08401 2f0227f8 34005e7e 80181dfd 1a8f700a
>> ffff88007d51fb40 1231545875 S Bi:2:013:1 -115 12288 <
>> ffff8800b38d96c0 1231551861 C Bi:2:013:1 0 11776 = 5244e386 a7800000
>> 010642ea 35bfbba5 373e738b cc035a73 c328a1ff 4da728ce
>> ffff880002ede0c0 1231556173 S Bi:2:013:1 -115 11776 <
>> ffff8800b38d9cc0 1231558618 C Bi:2:013:1 0 12288 = db91aae9 2d2532f3
>> 2e37448a fb36c213 55dda2ad 243122b2 261edb06 875848ac
>>
>>
>> 12288 = 7a1a0840 every Line with 12288 should start with 47 (MPEG-TS Sync byte)
>>
>> The working one which allocate 7000 bytes more (oh really big memory
>> pressure now!)  than this castrated USBFS interface allows.
>>
>> ffff88003ac37240 992178919 S Bi:2:013:1 -115 24064 <
>> ffff88003ac37000 992178953 S Bi:2:013:1 -115 24064 <
>> ffff88003ac37c00 992178980 S Bi:2:013:1 -115 24064 <
>> ffff88003ac37e40 992179003 S Bi:2:013:1 -115 24064 <
>> ffff88003ac37240 992190198 C Bi:2:013:1 0 24064 = 471fff10 00000000
>> 00000000 00000000 00000000 00000000 00000000 00000000
>> ffff88000601f480 992194368 S Bi:2:013:1 -115 24064 <
>> ffff88003ac37000 992203209 C Bi:2:013:1 0 24064 = 4701b114 43867ee6
>> 40790660 e898681c 9b1c7dca 08980d43 73181369 9be1bc67
>> ffff880067e38a80 992204642 S Bi:2:013:1 -115 24064 <
>> ffff88003ac37c00 992216318 C Bi:2:013:1 0 24064 = 4740001c 0000b01d
>> 0305d500 000000e0 104015e1 504016e1 60401be1 b04022e2
>> ffff880067e383c0 992219978 S Bi:2:013:1 -115 24064 <
>> ffff88003ac37e40 992229340 C Bi:2:013:1 0 24064 = 471fff10 00000000
>> 00000000 00000000 00000000 00000000 00000000 00000000
>>
>> everything starts nicely with 0x47
>
> Is the transfer size a multiple of 188 bytes in both cases ? In the
> working case, it looks so, but in the broken case ? Many DTV bridges
> have fixed DMA apertures and not variable really much, even though the
> device specifications do state that it might support different block
> sizes, some modes could be broken. I guess most likely, you would have
> handled this situation.
>
> Other issues that could possibly happen (wrt your jitter) is a high
> latency (in the particular case of failures) where the received data
> have unusable relative timestamps wrt DTS and or PTS.
>
> The only areas where you can have a corrupted stream as you mentioned
> "jitter" is either lost frames or the above two cases. I don't know
> what's the issue in your case, but might help to track down the issue
> altogether.
>

I am not so sure about that one but I guess that's it.
The 'black outs (0xff after the Sync byte)' are also related to the
demodulator (not perfect signal, it's still DVB-T) but
the alignment is just perfect
I also guess the misaligning problem could be related to the latency
and that the bridges have some
fixed or adjustable bulk transfer settings to work with that. In the
end the additional features
are done with software demodulation so there will be a certain minimal
system requirement anyway.

What would be against that theory would be that 2*that maximum
transfer size also results in a misaligned
video. Since I tried many other transfer sizes before I've been told
to use that one and I even thought the
device is broke but it just works that way... also with Windows and MacOSX.
As far as I can tell for user experience the device is okay with 24064 bytes.

TV has been running for several days now with no issues (I was also
testing the ts 'blackouts' where data
after the sync byte 0x47 is 0xff on weak signal, the TS sync byte
itself is still valid)

I have to do signal tests anyway next week in order to file some
specs, pattern generator and attenuator
are available for it (however that goes deeper into the functionality
and QA of those devices, also the
final device will have a tuner with better sensitivity onboard).

BR,
Markus

Re: [Patch] Increase USBFS Bulk Transfer size

[Manu Abraham]

On Thu, Oct 13, 2011 at 10:29 AM, Markus Rechberger
<mrechberger@gmail.com> wrote:
> On Thu, Oct 13, 2011 at 6:03 AM, Manu Abraham <abraham.manu@gmail.com> wrote:
>> Hello Markus,
>>
>> On Thu, Oct 13, 2011 at 3:39 AM, Markus Rechberger
>> <mrechberger@gmail.com> wrote:
>>> On Wed, Oct 12, 2011 at 11:48 PM, Markus Rechberger
>>> <mrechberger@gmail.com> wrote:
>>>> On Wed, Oct 12, 2011 at 10:33 PM, Greg KH <gregkh@suse.de> wrote:
>>>>> On Wed, Oct 12, 2011 at 06:59:01PM +0200, Markus Rechberger wrote:
>>>>>> On Wed, Oct 12, 2011 at 4:17 PM, Greg KH <gregkh@suse.de> wrote:
>>>>>> > On Wed, Oct 12, 2011 at 02:36:59PM +0200, Markus Rechberger wrote:
>>>>>> >> We have 2 products which can perform better with increased Bulk transfers
>>>>>> >
>>>>>> > I don't believe you :)
>>>>>> >
>>>>>> > As stated before, this patch is not acceptable.  Please work to figure
>>>>>> > out the real reason for your device problems here, this is not the
>>>>>> > correct solution at all.
>>>>>> >
>>>>>>
>>>>>> OK no device support for linux then. Windows and MacOSX are fine.
>>>>>
>>>>> That is your choice, not ours, as you are the one writing the closed
>>>>> source code.  Without usbmon dumps, showing that the problem really is
>>>>> in the kernel code, we can't do anything for you here.
>>>>>
>>>>
>>>>
>>>> here take your usbmon logs:
>>>> http://sundtek.de/support/working.log
>>>> http://sundtek.de/support/notworking.log (this is with your proposed
>>>> split up of 22k).
>>>>
>>>> Isochronous already supports up to 190kb buffers which cause no
>>>> trouble anywhere.
>>>> Bulk is castrated to 15kb buffers and 50kb should be such a big issue
>>>> while applications
>>>> which do not request it are not affected at all.
>>>>
>>>> I'm very angry that you do not really focus on what I write and just
>>>> try to write stories about
>>>> bogus things like my patch might break other applications while I
>>>> pointed out that this is not
>>>> possible with legacy applications which use this interface. It is not
>>>> even possible to read the
>>>> maximum buffer value from the kernelspace.
>>>>
>>>> Your bogus message about 1Gig ram you can put it elsewhere, I checked
>>>> multiple windows drivers
>>>> in the past the buffer size varies between small and something around
>>>> 50k usually.
>>>> I would fully like to avoid to patch this kernel to get those things
>>>> work because I don't want to
>>>> deal with people who just write around the actual issues.
>>>>
>>>> Alsi the fact that bigger buffers are already being used plus the HW
>>>> specifications of our main device also points out to a certain
>>>> buffersize near 50k. But of course you are the smart knowitall without
>>>> the need of explanation why
>>>> those things can be affected by HW and some HW chips have options to
>>>> influence that buffer size.
>>>
>>> And for those who are curious about the logfiles:
>>> Not working one as proposed by Alan that the full buffer size should
>>> be split into 2 requests:
>>>
>>> ffff8800b38d9f00 1231540351 S Bi:2:013:1 -115 12288 <
>>> ffff8800b38d96c0 1231540404 S Bi:2:013:1 -115 11776 <
>>> ffff8800b38d9cc0 1231540440 S Bi:2:013:1 -115 12288 <
>>> ffff880002ede600 1231540496 S Bi:2:013:1 -115 11776 <
>>> ffff8800b38d9f00 1231545491 C Bi:2:013:1 0 12288 = 7a1a0840 1ca8353c
>>> 004b80ec 4de08401 2f0227f8 34005e7e 80181dfd 1a8f700a
>>> ffff88007d51fb40 1231545875 S Bi:2:013:1 -115 12288 <
>>> ffff8800b38d96c0 1231551861 C Bi:2:013:1 0 11776 = 5244e386 a7800000
>>> 010642ea 35bfbba5 373e738b cc035a73 c328a1ff 4da728ce
>>> ffff880002ede0c0 1231556173 S Bi:2:013:1 -115 11776 <
>>> ffff8800b38d9cc0 1231558618 C Bi:2:013:1 0 12288 = db91aae9 2d2532f3
>>> 2e37448a fb36c213 55dda2ad 243122b2 261edb06 875848ac
>>>
>>>
>>> 12288 = 7a1a0840 every Line with 12288 should start with 47 (MPEG-TS Sync byte)
>>>
>>> The working one which allocate 7000 bytes more (oh really big memory
>>> pressure now!)  than this castrated USBFS interface allows.
>>>
>>> ffff88003ac37240 992178919 S Bi:2:013:1 -115 24064 <
>>> ffff88003ac37000 992178953 S Bi:2:013:1 -115 24064 <
>>> ffff88003ac37c00 992178980 S Bi:2:013:1 -115 24064 <
>>> ffff88003ac37e40 992179003 S Bi:2:013:1 -115 24064 <
>>> ffff88003ac37240 992190198 C Bi:2:013:1 0 24064 = 471fff10 00000000
>>> 00000000 00000000 00000000 00000000 00000000 00000000
>>> ffff88000601f480 992194368 S Bi:2:013:1 -115 24064 <
>>> ffff88003ac37000 992203209 C Bi:2:013:1 0 24064 = 4701b114 43867ee6
>>> 40790660 e898681c 9b1c7dca 08980d43 73181369 9be1bc67
>>> ffff880067e38a80 992204642 S Bi:2:013:1 -115 24064 <
>>> ffff88003ac37c00 992216318 C Bi:2:013:1 0 24064 = 4740001c 0000b01d
>>> 0305d500 000000e0 104015e1 504016e1 60401be1 b04022e2
>>> ffff880067e383c0 992219978 S Bi:2:013:1 -115 24064 <
>>> ffff88003ac37e40 992229340 C Bi:2:013:1 0 24064 = 471fff10 00000000
>>> 00000000 00000000 00000000 00000000 00000000 00000000
>>>
>>> everything starts nicely with 0x47
>>
>> Is the transfer size a multiple of 188 bytes in both cases ? In the
>> working case, it looks so, but in the broken case ? Many DTV bridges
>> have fixed DMA apertures and not variable really much, even though the
>> device specifications do state that it might support different block
>> sizes, some modes could be broken. I guess most likely, you would have
>> handled this situation.
>>
>> Other issues that could possibly happen (wrt your jitter) is a high
>> latency (in the particular case of failures) where the received data
>> have unusable relative timestamps wrt DTS and or PTS.
>>
>> The only areas where you can have a corrupted stream as you mentioned
>> "jitter" is either lost frames or the above two cases. I don't know
>> what's the issue in your case, but might help to track down the issue
>> altogether.
>>
>
> I am not so sure about that one but I guess that's it.
> The 'black outs (0xff after the Sync byte)' are also related to the
> demodulator (not perfect signal, it's still DVB-T) but
> the alignment is just perfect
> I also guess the misaligning problem could be related to the latency
> and that the bridges have some
> fixed or adjustable bulk transfer settings to work with that. In the
> end the additional features
> are done with software demodulation so there will be a certain minimal
> system requirement anyway.
>


I guess you meant to imply "software decoding" of the MPEG stream I
would say, rather than "demodulation" of the Baseband signal.


> What would be against that theory would be that 2*that maximum
> transfer size also results in a misaligned
> video. Since I tried many other transfer sizes before I've been told
> to use that one and I even thought the
> device is broke but it just works that way... also with Windows and MacOSX.
> As far as I can tell for user experience the device is okay with 24064 bytes.
>
> TV has been running for several days now with no issues (I was also
> testing the ts 'blackouts' where data
> after the sync byte 0x47 is 0xff on weak signal, the TS sync byte
> itself is still valid)

I have had such an issue earlier (0x47 followed 0xff. ie 0x47 0x1f 0xf
0x00 0x00 0x00 and so on) with a stream from the demodulator on a PCIe
device, but eventually it turned out to be that, it was handling the
device DMA in the reverse order, the device having 8 DMA apertures of
which all 8 had to be used for stream transfer.

RING(W) : Buf:2 Tail: 2 Count:0 Size:16
RING(R): Buf:2 Head: 2 Count:! Size: 16
47 If ff 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

Additionally, to be noted is that the ISO13818-1 specification allows
NULL padding of the TS by the channel modulator to achieve a CBR, from
VBR ES's. There exists an ETSI specification dealing with NULL padding
explicitly, don't remember the exact TR no.

After a search, EN 302755 v1.1.1 Section 5.1.5 deals with that, A
quick glance at least wrt to T2, I remember the same while I was
working with S2 as well.

All such issues mixed when together can be very nasty and painful,
making things difficult.

Regards,
Manu

Re: [Patch] Increase USBFS Bulk Transfer size

[Markus Rechberger]

On Thu, Oct 13, 2011 at 7:46 AM, Manu Abraham <abraham.manu@gmail.com> wrote:
> On Thu, Oct 13, 2011 at 10:29 AM, Markus Rechberger
> <mrechberger@gmail.com> wrote:
>> On Thu, Oct 13, 2011 at 6:03 AM, Manu Abraham <abraham.manu@gmail.com> wrote:
>>> Hello Markus,
>>>
>>> On Thu, Oct 13, 2011 at 3:39 AM, Markus Rechberger
>>> <mrechberger@gmail.com> wrote:
>>>> On Wed, Oct 12, 2011 at 11:48 PM, Markus Rechberger
>>>> <mrechberger@gmail.com> wrote:
>>>>> On Wed, Oct 12, 2011 at 10:33 PM, Greg KH <gregkh@suse.de> wrote:
>>>>>> On Wed, Oct 12, 2011 at 06:59:01PM +0200, Markus Rechberger wrote:
>>>>>>> On Wed, Oct 12, 2011 at 4:17 PM, Greg KH <gregkh@suse.de> wrote:
>>>>>>> > On Wed, Oct 12, 2011 at 02:36:59PM +0200, Markus Rechberger wrote:
>>>>>>> >> We have 2 products which can perform better with increased Bulk transfers
>>>>>>> >
>>>>>>> > I don't believe you :)
>>>>>>> >
>>>>>>> > As stated before, this patch is not acceptable.  Please work to figure
>>>>>>> > out the real reason for your device problems here, this is not the
>>>>>>> > correct solution at all.
>>>>>>> >
>>>>>>>
>>>>>>> OK no device support for linux then. Windows and MacOSX are fine.
>>>>>>
>>>>>> That is your choice, not ours, as you are the one writing the closed
>>>>>> source code.  Without usbmon dumps, showing that the problem really is
>>>>>> in the kernel code, we can't do anything for you here.
>>>>>>
>>>>>
>>>>>
>>>>> here take your usbmon logs:
>>>>> http://sundtek.de/support/working.log
>>>>> http://sundtek.de/support/notworking.log (this is with your proposed
>>>>> split up of 22k).
>>>>>
>>>>> Isochronous already supports up to 190kb buffers which cause no
>>>>> trouble anywhere.
>>>>> Bulk is castrated to 15kb buffers and 50kb should be such a big issue
>>>>> while applications
>>>>> which do not request it are not affected at all.
>>>>>
>>>>> I'm very angry that you do not really focus on what I write and just
>>>>> try to write stories about
>>>>> bogus things like my patch might break other applications while I
>>>>> pointed out that this is not
>>>>> possible with legacy applications which use this interface. It is not
>>>>> even possible to read the
>>>>> maximum buffer value from the kernelspace.
>>>>>
>>>>> Your bogus message about 1Gig ram you can put it elsewhere, I checked
>>>>> multiple windows drivers
>>>>> in the past the buffer size varies between small and something around
>>>>> 50k usually.
>>>>> I would fully like to avoid to patch this kernel to get those things
>>>>> work because I don't want to
>>>>> deal with people who just write around the actual issues.
>>>>>
>>>>> Alsi the fact that bigger buffers are already being used plus the HW
>>>>> specifications of our main device also points out to a certain
>>>>> buffersize near 50k. But of course you are the smart knowitall without
>>>>> the need of explanation why
>>>>> those things can be affected by HW and some HW chips have options to
>>>>> influence that buffer size.
>>>>
>>>> And for those who are curious about the logfiles:
>>>> Not working one as proposed by Alan that the full buffer size should
>>>> be split into 2 requests:
>>>>
>>>> ffff8800b38d9f00 1231540351 S Bi:2:013:1 -115 12288 <
>>>> ffff8800b38d96c0 1231540404 S Bi:2:013:1 -115 11776 <
>>>> ffff8800b38d9cc0 1231540440 S Bi:2:013:1 -115 12288 <
>>>> ffff880002ede600 1231540496 S Bi:2:013:1 -115 11776 <
>>>> ffff8800b38d9f00 1231545491 C Bi:2:013:1 0 12288 = 7a1a0840 1ca8353c
>>>> 004b80ec 4de08401 2f0227f8 34005e7e 80181dfd 1a8f700a
>>>> ffff88007d51fb40 1231545875 S Bi:2:013:1 -115 12288 <
>>>> ffff8800b38d96c0 1231551861 C Bi:2:013:1 0 11776 = 5244e386 a7800000
>>>> 010642ea 35bfbba5 373e738b cc035a73 c328a1ff 4da728ce
>>>> ffff880002ede0c0 1231556173 S Bi:2:013:1 -115 11776 <
>>>> ffff8800b38d9cc0 1231558618 C Bi:2:013:1 0 12288 = db91aae9 2d2532f3
>>>> 2e37448a fb36c213 55dda2ad 243122b2 261edb06 875848ac
>>>>
>>>>
>>>> 12288 = 7a1a0840 every Line with 12288 should start with 47 (MPEG-TS Sync byte)
>>>>
>>>> The working one which allocate 7000 bytes more (oh really big memory
>>>> pressure now!)  than this castrated USBFS interface allows.
>>>>
>>>> ffff88003ac37240 992178919 S Bi:2:013:1 -115 24064 <
>>>> ffff88003ac37000 992178953 S Bi:2:013:1 -115 24064 <
>>>> ffff88003ac37c00 992178980 S Bi:2:013:1 -115 24064 <
>>>> ffff88003ac37e40 992179003 S Bi:2:013:1 -115 24064 <
>>>> ffff88003ac37240 992190198 C Bi:2:013:1 0 24064 = 471fff10 00000000
>>>> 00000000 00000000 00000000 00000000 00000000 00000000
>>>> ffff88000601f480 992194368 S Bi:2:013:1 -115 24064 <
>>>> ffff88003ac37000 992203209 C Bi:2:013:1 0 24064 = 4701b114 43867ee6
>>>> 40790660 e898681c 9b1c7dca 08980d43 73181369 9be1bc67
>>>> ffff880067e38a80 992204642 S Bi:2:013:1 -115 24064 <
>>>> ffff88003ac37c00 992216318 C Bi:2:013:1 0 24064 = 4740001c 0000b01d
>>>> 0305d500 000000e0 104015e1 504016e1 60401be1 b04022e2
>>>> ffff880067e383c0 992219978 S Bi:2:013:1 -115 24064 <
>>>> ffff88003ac37e40 992229340 C Bi:2:013:1 0 24064 = 471fff10 00000000
>>>> 00000000 00000000 00000000 00000000 00000000 00000000
>>>>
>>>> everything starts nicely with 0x47
>>>
>>> Is the transfer size a multiple of 188 bytes in both cases ? In the
>>> working case, it looks so, but in the broken case ? Many DTV bridges
>>> have fixed DMA apertures and not variable really much, even though the
>>> device specifications do state that it might support different block
>>> sizes, some modes could be broken. I guess most likely, you would have
>>> handled this situation.
>>>
>>> Other issues that could possibly happen (wrt your jitter) is a high
>>> latency (in the particular case of failures) where the received data
>>> have unusable relative timestamps wrt DTS and or PTS.
>>>
>>> The only areas where you can have a corrupted stream as you mentioned
>>> "jitter" is either lost frames or the above two cases. I don't know
>>> what's the issue in your case, but might help to track down the issue
>>> altogether.
>>>
>>
>> I am not so sure about that one but I guess that's it.
>> The 'black outs (0xff after the Sync byte)' are also related to the
>> demodulator (not perfect signal, it's still DVB-T) but
>> the alignment is just perfect
>> I also guess the misaligning problem could be related to the latency
>> and that the bridges have some
>> fixed or adjustable bulk transfer settings to work with that. In the
>> end the additional features
>> are done with software demodulation so there will be a certain minimal
>> system requirement anyway.
>>
>
>
> I guess you meant to imply "software decoding" of the MPEG stream I
> would say, rather than "demodulation" of the Baseband signal.
>
>
>> What would be against that theory would be that 2*that maximum
>> transfer size also results in a misaligned
>> video. Since I tried many other transfer sizes before I've been told
>> to use that one and I even thought the
>> device is broke but it just works that way... also with Windows and MacOSX.
>> As far as I can tell for user experience the device is okay with 24064 bytes.
>>
>> TV has been running for several days now with no issues (I was also
>> testing the ts 'blackouts' where data
>> after the sync byte 0x47 is 0xff on weak signal, the TS sync byte
>> itself is still valid)
>
> I have had such an issue earlier (0x47 followed 0xff. ie 0x47 0x1f 0xf
> 0x00 0x00 0x00 and so on) with a stream from the demodulator on a PCIe
> device, but eventually it turned out to be that, it was handling the
> device DMA in the reverse order, the device having 8 DMA apertures of
> which all 8 had to be used for stream transfer.
>
> RING(W) : Buf:2 Tail: 2 Count:0 Size:16
> RING(R): Buf:2 Head: 2 Count:! Size: 16
> 47 If ff 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>
> Additionally, to be noted is that the ISO13818-1 specification allows
> NULL padding of the TS by the channel modulator to achieve a CBR, from
> VBR ES's. There exists an ETSI specification dealing with NULL padding
> explicitly, don't remember the exact TR no.
>
> After a search, EN 302755 v1.1.1 Section 5.1.5 deals with that, A
> quick glance at least wrt to T2, I remember the same while I was
> working with S2 as well.
>
> All such issues mixed when together can be very nasty and painful,
> making things difficult.
>

yes the PID is 0x1fff which are null packets over there, so those ones are okay.
Main point is it works and it would even comply with certain HW specs (the other
device points out about 256*188 bulk transfer buffer sizes.

Now it would be the time for some people to open their eyes and get
that patch in.
The maximum transfer buffer size was derived from our other device which can
do the flexible setting. According to the specification it is no magic value.

And seriously increasing that current limitation for a few more bytes
while isochronous
already allows up to 190k is just a joke, we have been using that for
3 years now on
x86, ARM, MIPS without any problem. The biggest problem is usually
when the memory
runs out in general by writing a log file to ram via /tmp or /var/log.
But even then we have system logs where other drivers fail first (eg.
ethernet) and complain
about not enough memory.

BR,
Markus

Null Packets 0x1fff (471fff...)
http://en.wikipedia.org/wiki/MPEG_transport_stream#Null_packets

> Regards,
> Manu
>

Re: [Patch] Increase USBFS Bulk Transfer size

[Markus Rechberger]

On Thu, Oct 13, 2011 at 10:37 AM, Markus Rechberger
<mrechberger@gmail.com> wrote:
> On Thu, Oct 13, 2011 at 7:46 AM, Manu Abraham <abraham.manu@gmail.com> wrote:
>> On Thu, Oct 13, 2011 at 10:29 AM, Markus Rechberger
>> <mrechberger@gmail.com> wrote:
>>> On Thu, Oct 13, 2011 at 6:03 AM, Manu Abraham <abraham.manu@gmail.com> wrote:
>>>> Hello Markus,
>>>>
>>>> On Thu, Oct 13, 2011 at 3:39 AM, Markus Rechberger
>>>> <mrechberger@gmail.com> wrote:
>>>>> On Wed, Oct 12, 2011 at 11:48 PM, Markus Rechberger
>>>>> <mrechberger@gmail.com> wrote:
>>>>>> On Wed, Oct 12, 2011 at 10:33 PM, Greg KH <gregkh@suse.de> wrote:
>>>>>>> On Wed, Oct 12, 2011 at 06:59:01PM +0200, Markus Rechberger wrote:
>>>>>>>> On Wed, Oct 12, 2011 at 4:17 PM, Greg KH <gregkh@suse.de> wrote:
>>>>>>>> > On Wed, Oct 12, 2011 at 02:36:59PM +0200, Markus Rechberger wrote:
>>>>>>>> >> We have 2 products which can perform better with increased Bulk transfers
>>>>>>>> >
>>>>>>>> > I don't believe you :)
>>>>>>>> >
>>>>>>>> > As stated before, this patch is not acceptable.  Please work to figure
>>>>>>>> > out the real reason for your device problems here, this is not the
>>>>>>>> > correct solution at all.
>>>>>>>> >
>>>>>>>>
>>>>>>>> OK no device support for linux then. Windows and MacOSX are fine.
>>>>>>>
>>>>>>> That is your choice, not ours, as you are the one writing the closed
>>>>>>> source code.  Without usbmon dumps, showing that the problem really is
>>>>>>> in the kernel code, we can't do anything for you here.
>>>>>>>
>>>>>>
>>>>>>
>>>>>> here take your usbmon logs:
>>>>>> http://sundtek.de/support/working.log
>>>>>> http://sundtek.de/support/notworking.log (this is with your proposed
>>>>>> split up of 22k).
>>>>>>
>>>>>> Isochronous already supports up to 190kb buffers which cause no
>>>>>> trouble anywhere.
>>>>>> Bulk is castrated to 15kb buffers and 50kb should be such a big issue
>>>>>> while applications
>>>>>> which do not request it are not affected at all.
>>>>>>
>>>>>> I'm very angry that you do not really focus on what I write and just
>>>>>> try to write stories about
>>>>>> bogus things like my patch might break other applications while I
>>>>>> pointed out that this is not
>>>>>> possible with legacy applications which use this interface. It is not
>>>>>> even possible to read the
>>>>>> maximum buffer value from the kernelspace.
>>>>>>
>>>>>> Your bogus message about 1Gig ram you can put it elsewhere, I checked
>>>>>> multiple windows drivers
>>>>>> in the past the buffer size varies between small and something around
>>>>>> 50k usually.
>>>>>> I would fully like to avoid to patch this kernel to get those things
>>>>>> work because I don't want to
>>>>>> deal with people who just write around the actual issues.
>>>>>>
>>>>>> Alsi the fact that bigger buffers are already being used plus the HW
>>>>>> specifications of our main device also points out to a certain
>>>>>> buffersize near 50k. But of course you are the smart knowitall without
>>>>>> the need of explanation why
>>>>>> those things can be affected by HW and some HW chips have options to
>>>>>> influence that buffer size.
>>>>>
>>>>> And for those who are curious about the logfiles:
>>>>> Not working one as proposed by Alan that the full buffer size should
>>>>> be split into 2 requests:
>>>>>
>>>>> ffff8800b38d9f00 1231540351 S Bi:2:013:1 -115 12288 <
>>>>> ffff8800b38d96c0 1231540404 S Bi:2:013:1 -115 11776 <
>>>>> ffff8800b38d9cc0 1231540440 S Bi:2:013:1 -115 12288 <
>>>>> ffff880002ede600 1231540496 S Bi:2:013:1 -115 11776 <
>>>>> ffff8800b38d9f00 1231545491 C Bi:2:013:1 0 12288 = 7a1a0840 1ca8353c
>>>>> 004b80ec 4de08401 2f0227f8 34005e7e 80181dfd 1a8f700a
>>>>> ffff88007d51fb40 1231545875 S Bi:2:013:1 -115 12288 <
>>>>> ffff8800b38d96c0 1231551861 C Bi:2:013:1 0 11776 = 5244e386 a7800000
>>>>> 010642ea 35bfbba5 373e738b cc035a73 c328a1ff 4da728ce
>>>>> ffff880002ede0c0 1231556173 S Bi:2:013:1 -115 11776 <
>>>>> ffff8800b38d9cc0 1231558618 C Bi:2:013:1 0 12288 = db91aae9 2d2532f3
>>>>> 2e37448a fb36c213 55dda2ad 243122b2 261edb06 875848ac
>>>>>
>>>>>
>>>>> 12288 = 7a1a0840 every Line with 12288 should start with 47 (MPEG-TS Sync byte)
>>>>>
>>>>> The working one which allocate 7000 bytes more (oh really big memory
>>>>> pressure now!)  than this castrated USBFS interface allows.
>>>>>
>>>>> ffff88003ac37240 992178919 S Bi:2:013:1 -115 24064 <
>>>>> ffff88003ac37000 992178953 S Bi:2:013:1 -115 24064 <
>>>>> ffff88003ac37c00 992178980 S Bi:2:013:1 -115 24064 <
>>>>> ffff88003ac37e40 992179003 S Bi:2:013:1 -115 24064 <
>>>>> ffff88003ac37240 992190198 C Bi:2:013:1 0 24064 = 471fff10 00000000
>>>>> 00000000 00000000 00000000 00000000 00000000 00000000
>>>>> ffff88000601f480 992194368 S Bi:2:013:1 -115 24064 <
>>>>> ffff88003ac37000 992203209 C Bi:2:013:1 0 24064 = 4701b114 43867ee6
>>>>> 40790660 e898681c 9b1c7dca 08980d43 73181369 9be1bc67
>>>>> ffff880067e38a80 992204642 S Bi:2:013:1 -115 24064 <
>>>>> ffff88003ac37c00 992216318 C Bi:2:013:1 0 24064 = 4740001c 0000b01d
>>>>> 0305d500 000000e0 104015e1 504016e1 60401be1 b04022e2
>>>>> ffff880067e383c0 992219978 S Bi:2:013:1 -115 24064 <
>>>>> ffff88003ac37e40 992229340 C Bi:2:013:1 0 24064 = 471fff10 00000000
>>>>> 00000000 00000000 00000000 00000000 00000000 00000000
>>>>>
>>>>> everything starts nicely with 0x47
>>>>
>>>> Is the transfer size a multiple of 188 bytes in both cases ? In the
>>>> working case, it looks so, but in the broken case ? Many DTV bridges
>>>> have fixed DMA apertures and not variable really much, even though the
>>>> device specifications do state that it might support different block
>>>> sizes, some modes could be broken. I guess most likely, you would have
>>>> handled this situation.
>>>>
>>>> Other issues that could possibly happen (wrt your jitter) is a high
>>>> latency (in the particular case of failures) where the received data
>>>> have unusable relative timestamps wrt DTS and or PTS.
>>>>
>>>> The only areas where you can have a corrupted stream as you mentioned
>>>> "jitter" is either lost frames or the above two cases. I don't know
>>>> what's the issue in your case, but might help to track down the issue
>>>> altogether.
>>>>
>>>
>>> I am not so sure about that one but I guess that's it.
>>> The 'black outs (0xff after the Sync byte)' are also related to the
>>> demodulator (not perfect signal, it's still DVB-T) but
>>> the alignment is just perfect
>>> I also guess the misaligning problem could be related to the latency
>>> and that the bridges have some
>>> fixed or adjustable bulk transfer settings to work with that. In the
>>> end the additional features
>>> are done with software demodulation so there will be a certain minimal
>>> system requirement anyway.
>>>
>>
>>
>> I guess you meant to imply "software decoding" of the MPEG stream I
>> would say, rather than "demodulation" of the Baseband signal.
>>
>>
>>> What would be against that theory would be that 2*that maximum
>>> transfer size also results in a misaligned
>>> video. Since I tried many other transfer sizes before I've been told
>>> to use that one and I even thought the
>>> device is broke but it just works that way... also with Windows and MacOSX.
>>> As far as I can tell for user experience the device is okay with 24064 bytes.
>>>
>>> TV has been running for several days now with no issues (I was also
>>> testing the ts 'blackouts' where data
>>> after the sync byte 0x47 is 0xff on weak signal, the TS sync byte
>>> itself is still valid)
>>
>> I have had such an issue earlier (0x47 followed 0xff. ie 0x47 0x1f 0xf
>> 0x00 0x00 0x00 and so on) with a stream from the demodulator on a PCIe
>> device, but eventually it turned out to be that, it was handling the
>> device DMA in the reverse order, the device having 8 DMA apertures of
>> which all 8 had to be used for stream transfer.
>>
>> RING(W) : Buf:2 Tail: 2 Count:0 Size:16
>> RING(R): Buf:2 Head: 2 Count:! Size: 16
>> 47 If ff 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>
>> Additionally, to be noted is that the ISO13818-1 specification allows
>> NULL padding of the TS by the channel modulator to achieve a CBR, from
>> VBR ES's. There exists an ETSI specification dealing with NULL padding
>> explicitly, don't remember the exact TR no.
>>
>> After a search, EN 302755 v1.1.1 Section 5.1.5 deals with that, A
>> quick glance at least wrt to T2, I remember the same while I was
>> working with S2 as well.
>>
>> All such issues mixed when together can be very nasty and painful,
>> making things difficult.
>>
>
> yes the PID is 0x1fff which are null packets over there, so those ones are okay.
> Main point is it works and it would even comply with certain HW specs (the other
> device points out about 256*188 bulk transfer buffer sizes.
>
> Now it would be the time for some people to open their eyes and get
> that patch in.
> The maximum transfer buffer size was derived from our other device which can
> do the flexible setting. According to the specification it is no magic value.
>
> And seriously increasing that current limitation for a few more bytes
> while isochronous
> already allows up to 190k is just a joke, we have been using that for
> 3 years now on
> x86, ARM, MIPS without any problem. The biggest problem is usually
> when the memory
> runs out in general by writing a log file to ram via /tmp or /var/log.
> But even then we have system logs where other drivers fail first (eg.
> ethernet) and complain
> about not enough memory.
>

Some demonstration video:
(with 24064 bytes, which uses the additional buffers which are allowed
by the introduced patch):
Flawlessly video:
http://www.sundtek.de/support/00011.MTS

(Alan's proposed buffer size 11776/12288)
Jerky video:
http://www.sundtek.de/support/00012.MTS

both videos are around 30 seconds made with a camera, video is still uploading.
this is my last post to that topic I still have some little hope that
this patch will get in to fix that issue.
As proposed for academic research you can pick up a Stick in Berlin
once the new cut is ready from manufacturing,

Markus

Re: [Patch] Increase USBFS Bulk Transfer size

[Manu Abraham]

> yes the PID is 0x1fff which are null packets over there, so those ones are okay.
> Main point is it works and it would even comply with certain HW specs (the other
> device points out about 256*188 bulk transfer buffer sizes.
>
> Now it would be the time for some people to open their eyes and get
> that patch in.


Call me dumb, but it still defies my logic why there would be stream
corruption with a smaller buffer size, if the driver is handling the
stream correctly.

I can probably imagine that there could be a likely chance, user space
would have to poll more often in case with smaller buffers, while
making it larger you increase the latency of the stream; that being a
double edged blade.


> The maximum transfer buffer size was derived from our other device which can
> do the flexible setting. According to the specification it is no magic value.


So, with your single device, if you were to make changes to some
"magical constants"; and tomorrow if someone wants to have
modifications again, what would you do ? If that change is a must,
maybe it should be configurable in some way, rather than having
another fixed magical number.


Regards,
Manu

Re: [Patch] Increase USBFS Bulk Transfer size

[Markus Rechberger]

On Thu, Oct 13, 2011 at 11:34 AM, Manu Abraham <abraham.manu@gmail.com> wrote:
>> yes the PID is 0x1fff which are null packets over there, so those ones are okay.
>> Main point is it works and it would even comply with certain HW specs (the other
>> device points out about 256*188 bulk transfer buffer sizes.
>>
>> Now it would be the time for some people to open their eyes and get
>> that patch in.
>
>
> Call me dumb, but it still defies my logic why there would be stream
> corruption with a smaller buffer size, if the driver is handling the
> stream correctly.
>

you can see it in the logfile that the TS packets don't start as they should
0x47 is not the first value which is wrong and so on. And this is a
fact which everyone
can see by looking at the logfile.
At least every second transfer would have to start with 0x47, at the
point of logging the userspace
application does not have access to the USB buffer.

> I can probably imagine that there could be a likely chance, user space
> would have to poll more often in case with smaller buffers, while
> making it larger you increase the latency of the stream; that being a
> double edged blade.
>
>
>> The maximum transfer buffer size was derived from our other device which can
>> do the flexible setting. According to the specification it is no magic value.
>
>
> So, with your single device, if you were to make changes to some
> "magical constants"; and tomorrow if someone wants to have
> modifications again, what would you do ?

please do some investigation about the patch, if the value is
increased it does not
affect any previous application. That constant is not readable from
the kernel so applications
need to hardcode it. Small buffers are totally unrelated to that.

That is what I don't like here that some people are talking without
doing their homework and reviewing
what it is about.

> If that change is a must,
> maybe it should be configurable in some way, rather than having
> another fixed magical number.
>

Although I agree with that one.

BR,
Markus

Re: [Patch] Increase USBFS Bulk Transfer size

[Alan Stern]

On Thu, 13 Oct 2011, Markus Rechberger wrote:

> And for those who are curious about the logfiles:
> Not working one as proposed by Alan that the full buffer size should
> be split into 2 requests:
> 
> ffff8800b38d9f00 1231540351 S Bi:2:013:1 -115 12288 <
> ffff8800b38d96c0 1231540404 S Bi:2:013:1 -115 11776 <
> ffff8800b38d9cc0 1231540440 S Bi:2:013:1 -115 12288 <
> ffff880002ede600 1231540496 S Bi:2:013:1 -115 11776 <
> ffff8800b38d9f00 1231545491 C Bi:2:013:1 0 12288 = 7a1a0840 1ca8353c
> 004b80ec 4de08401 2f0227f8 34005e7e 80181dfd 1a8f700a
> ffff88007d51fb40 1231545875 S Bi:2:013:1 -115 12288 <
> ffff8800b38d96c0 1231551861 C Bi:2:013:1 0 11776 = 5244e386 a7800000
> 010642ea 35bfbba5 373e738b cc035a73 c328a1ff 4da728ce
> ffff880002ede0c0 1231556173 S Bi:2:013:1 -115 11776 <
> ffff8800b38d9cc0 1231558618 C Bi:2:013:1 0 12288 = db91aae9 2d2532f3
> 2e37448a fb36c213 55dda2ad 243122b2 261edb06 875848ac
> 
> 
> 12288 = 7a1a0840 every Line with 12288 should start with 47 (MPEG-TS Sync byte)
> 
> The working one which allocate 7000 bytes more (oh really big memory
> pressure now!)  than this castrated USBFS interface allows.

Sarcasm won't help convince anybody to do anything.

> ffff88003ac37240 992178919 S Bi:2:013:1 -115 24064 <
> ffff88003ac37000 992178953 S Bi:2:013:1 -115 24064 <
> ffff88003ac37c00 992178980 S Bi:2:013:1 -115 24064 <
> ffff88003ac37e40 992179003 S Bi:2:013:1 -115 24064 <
> ffff88003ac37240 992190198 C Bi:2:013:1 0 24064 = 471fff10 00000000
> 00000000 00000000 00000000 00000000 00000000 00000000
> ffff88000601f480 992194368 S Bi:2:013:1 -115 24064 <
> ffff88003ac37000 992203209 C Bi:2:013:1 0 24064 = 4701b114 43867ee6
> 40790660 e898681c 9b1c7dca 08980d43 73181369 9be1bc67
> ffff880067e38a80 992204642 S Bi:2:013:1 -115 24064 <
> ffff88003ac37c00 992216318 C Bi:2:013:1 0 24064 = 4740001c 0000b01d
> 0305d500 000000e0 104015e1 504016e1 60401be1 b04022e2
> ffff880067e383c0 992219978 S Bi:2:013:1 -115 24064 <
> ffff88003ac37e40 992229340 C Bi:2:013:1 0 24064 = 471fff10 00000000
> 00000000 00000000 00000000 00000000 00000000 00000000
> 
> everything starts nicely with 0x47

This is very interesting.  There are only two things that could be
happening: Either the device sends different data during the two tests,
or there's a bug in the kernel.

Now, it is possible the device is sending bad data.  The initial parts
of the two logs do not agree exactly; there are numerous small
differences in the control data sent by the device and by the program.  
I don't know whether they are significant, but if they aren't, there's
no reason for the device to send different bulk data.  The transfer
size certainly cannot account for it.  Indeed, even if the transfer
size were only 512 bytes, the first data packet should still be the 
same.

You said you don't want to spend any more time working on this problem.  
But I'd still like to track it down.  Is it possible for you to loan me 
a device for testing?  I've got a USB bus analyzer, which could help to 
pin down what's going wrong.

Alan Stern

Re: [Patch] Increase USBFS Bulk Transfer size

[Markus Rechberger]

On Thu, Oct 13, 2011 at 4:58 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Thu, 13 Oct 2011, Markus Rechberger wrote:
>
>> And for those who are curious about the logfiles:
>> Not working one as proposed by Alan that the full buffer size should
>> be split into 2 requests:
>>
>> ffff8800b38d9f00 1231540351 S Bi:2:013:1 -115 12288 <
>> ffff8800b38d96c0 1231540404 S Bi:2:013:1 -115 11776 <
>> ffff8800b38d9cc0 1231540440 S Bi:2:013:1 -115 12288 <
>> ffff880002ede600 1231540496 S Bi:2:013:1 -115 11776 <
>> ffff8800b38d9f00 1231545491 C Bi:2:013:1 0 12288 = 7a1a0840 1ca8353c
>> 004b80ec 4de08401 2f0227f8 34005e7e 80181dfd 1a8f700a
>> ffff88007d51fb40 1231545875 S Bi:2:013:1 -115 12288 <
>> ffff8800b38d96c0 1231551861 C Bi:2:013:1 0 11776 = 5244e386 a7800000
>> 010642ea 35bfbba5 373e738b cc035a73 c328a1ff 4da728ce
>> ffff880002ede0c0 1231556173 S Bi:2:013:1 -115 11776 <
>> ffff8800b38d9cc0 1231558618 C Bi:2:013:1 0 12288 = db91aae9 2d2532f3
>> 2e37448a fb36c213 55dda2ad 243122b2 261edb06 875848ac
>>
>>
>> 12288 = 7a1a0840 every Line with 12288 should start with 47 (MPEG-TS Sync byte)
>>
>> The working one which allocate 7000 bytes more (oh really big memory
>> pressure now!)  than this castrated USBFS interface allows.
>
> Sarcasm won't help convince anybody to do anything.
>
>> ffff88003ac37240 992178919 S Bi:2:013:1 -115 24064 <
>> ffff88003ac37000 992178953 S Bi:2:013:1 -115 24064 <
>> ffff88003ac37c00 992178980 S Bi:2:013:1 -115 24064 <
>> ffff88003ac37e40 992179003 S Bi:2:013:1 -115 24064 <
>> ffff88003ac37240 992190198 C Bi:2:013:1 0 24064 = 471fff10 00000000
>> 00000000 00000000 00000000 00000000 00000000 00000000
>> ffff88000601f480 992194368 S Bi:2:013:1 -115 24064 <
>> ffff88003ac37000 992203209 C Bi:2:013:1 0 24064 = 4701b114 43867ee6
>> 40790660 e898681c 9b1c7dca 08980d43 73181369 9be1bc67
>> ffff880067e38a80 992204642 S Bi:2:013:1 -115 24064 <
>> ffff88003ac37c00 992216318 C Bi:2:013:1 0 24064 = 4740001c 0000b01d
>> 0305d500 000000e0 104015e1 504016e1 60401be1 b04022e2
>> ffff880067e383c0 992219978 S Bi:2:013:1 -115 24064 <
>> ffff88003ac37e40 992229340 C Bi:2:013:1 0 24064 = 471fff10 00000000
>> 00000000 00000000 00000000 00000000 00000000 00000000
>>
>> everything starts nicely with 0x47
>
> This is very interesting.  There are only two things that could be
> happening: Either the device sends different data during the two tests,
> or there's a bug in the kernel.
>
> Now, it is possible the device is sending bad data.  The initial parts
> of the two logs do not agree exactly; there are numerous small
> differences in the control data sent by the device and by the program.
> I don't know whether they are significant, but if they aren't, there's
> no reason for the device to send different bulk data.  The transfer
> size certainly cannot account for it.  Indeed, even if the transfer
> size were only 512 bytes, the first data packet should still be the
> same.
>
> You said you don't want to spend any more time working on this problem.
> But I'd still like to track it down.  Is it possible for you to loan me
> a device for testing?  I've got a USB bus analyzer, which could help to
> pin down what's going wrong.
>

The same issues happen with MacOSX and AGAIN windows is also using the same
buffer size.
Currently there's only one sample available, the production should be
ready by the end of this month.
I can certainly give you one device for playing around as soon as it
is ready but for that

Before that I insist that this patch will go into the kernel (I know
that sounds ridiculous but so far I did everything
you requested me to do), I clearly pointed out that we even have hardware
specs which can influence the transfer buffer. The patch does not hurt
and makes the device work.

Now it turns out that this issue suddenly is interesting because
there's no idea why that can happen.
First priority is to get this device work, second priority is
everything else if Linux support is wanted.

BR,
Markus

Re: [Patch] Increase USBFS Bulk Transfer size

[Chris Friesen]

On 10/13/2011 09:19 AM, Markus Rechberger wrote:
> On Thu, Oct 13, 2011 at 4:58 PM, Alan Stern<stern@rowland.harvard.edu>  wrote:

>> This is very interesting.  There are only two things that could be
>> happening: Either the device sends different data during the two tests,
>> or there's a bug in the kernel.

> Before that I insist that this patch will go into the kernel (I know
> that sounds ridiculous but so far I did everything
> you requested me to do), I clearly pointed out that we even have hardware
> specs which can influence the transfer buffer. The patch does not hurt
> and makes the device work.

That makes no sense.  If there's a bug in the hardware we likely don't 
want to make global changes to work around it--a more targeted fix may 
be more appropriate.

If there is a bug in the kernel, why would we want a bandaid patch that 
just happens to make it work rather than a true fix?

Chris

-- 
Chris Friesen
Software Developer
GENBAND
chris.friesen@genband.com
www.genband.com

Re: [Patch] Increase USBFS Bulk Transfer size

[Markus Rechberger]

On Thu, Oct 13, 2011 at 6:01 PM, Chris Friesen
<chris.friesen@genband.com> wrote:
> On 10/13/2011 09:19 AM, Markus Rechberger wrote:
>>
>> On Thu, Oct 13, 2011 at 4:58 PM, Alan Stern<stern@rowland.harvard.edu>
>>  wrote:
>
>>> This is very interesting.  There are only two things that could be
>>> happening: Either the device sends different data during the two tests,
>>> or there's a bug in the kernel.
>
>> Before that I insist that this patch will go into the kernel (I know
>> that sounds ridiculous but so far I did everything
>> you requested me to do), I clearly pointed out that we even have hardware
>> specs which can influence the transfer buffer. The patch does not hurt
>> and makes the device work.
>
> That makes no sense.  If there's a bug in the hardware we likely don't want
> to make global changes to work around it--a more targeted fix may be more
> appropriate.
>

Did you read the part about that we have some hardware which can do
some settings
in hardware to affect the transfer buffer? Even that part is not clear
how it can affect the
transfer for the usb subsystem.

Why on earth should things be done differently than with all other
operating systems?

> If there is a bug in the kernel, why would we want a bandaid patch that just
> happens to make it work rather than a true fix?
>

In silicon? Are you going to tape out another one for it?
First priority is that the device can work and be sold, my last
priority is that requested academic research (as we are not
the design house for the particular IC), I'm willing
to support it if my request is fulfilled, the demo videos are online
and show that it works with increasing the maximum allowed buffersize.

And about the particular device the boundaries are just around 7000
bytes off and people are screaming about that? Sorry this is just like
in kindergarden here.

Markus

Re: [Patch] Increase USBFS Bulk Transfer size

[Chris Friesen]

On 10/13/2011 10:12 AM, Markus Rechberger wrote:
> On Thu, Oct 13, 2011 at 6:01 PM, Chris Friesen

> Why on earth should things be done differently than with all other
> operating systems?

Linux does a lot of things differently than other operating systems.  In 
many cases it's because the other OS's do things wrong.  One could just 
as easily turn the question around and ask why on earth would your 
hardware be different than all the other USB hardware?

>> If there is a bug in the kernel, why would we want a bandaid patch that just
>> happens to make it work rather than a true fix?
>
> In silicon? Are you going to tape out another one for it?

If there's a bug in the kernel, the true fix would be in the kernel.

> And about the particular device the boundaries are just around 7000
> bytes off and people are screaming about that? Sorry this is just like
> in kindergarden here.

Actually I think that you're the one throwing a tantrum and yelling that 
it needs to be done your way or else.  We generally don't change global 
constants because one hardware device doesn't work.

Chris

-- 
Chris Friesen
Software Developer
GENBAND
chris.friesen@genband.com
www.genband.com

Re: [Patch] Increase USBFS Bulk Transfer size

[Alan Stern]

On Thu, 13 Oct 2011, Markus Rechberger wrote:

> The same issues happen with MacOSX and AGAIN windows is also using the same
> buffer size.
> Currently there's only one sample available, the production should be
> ready by the end of this month.
> I can certainly give you one device for playing around as soon as it
> is ready but for that
> 
> Before that I insist that this patch will go into the kernel (I know
> that sounds ridiculous but so far I did everything
> you requested me to do), I clearly pointed out that we even have hardware
> specs which can influence the transfer buffer. The patch does not hurt
> and makes the device work.
> 
> Now it turns out that this issue suddenly is interesting because
> there's no idea why that can happen.
> First priority is to get this device work, second priority is
> everything else if Linux support is wanted.

For my part, I don't much mind raising the limit from 16 KB to 32 KB.  
48 KB may be unnecessary, especially since one of the devices doesn't 
even work when the transfer buffer is that large.

However it's just a work-around, not a real solution to the underlying
problem (whatever that problem turns out to be).  Part of the reason
for keeping the limit low is that it shouldn't matter, since people
ought to be able to use a bunch of small transfers rather than one
large transfer.  Apparently that justification doesn't work with these
webcams, although there's no known reason why it shouldn't.

But I'm not the person in charge of these decisions...

Alan Stern

Re: [Patch] Increase USBFS Bulk Transfer size

[Markus Rechberger]

On Thu, Oct 13, 2011 at 6:25 PM, Chris Friesen
<chris.friesen@genband.com> wrote:
> On 10/13/2011 10:12 AM, Markus Rechberger wrote:
>>
>> On Thu, Oct 13, 2011 at 6:01 PM, Chris Friesen
>
>> Why on earth should things be done differently than with all other
>> operating systems?
>
> Linux does a lot of things differently than other operating systems.  In
> many cases it's because the other OS's do things wrong.  One could just as
> easily turn the question around and ask why on earth would your hardware be
> different than all the other USB hardware?
>
>>> If there is a bug in the kernel, why would we want a bandaid patch that
>>> just
>>> happens to make it work rather than a true fix?
>>
>> In silicon? Are you going to tape out another one for it?
>
> If there's a bug in the kernel, the true fix would be in the kernel.
>
>> And about the particular device the boundaries are just around 7000
>> bytes off and people are screaming about that? Sorry this is just like
>> in kindergarden here.
>
> Actually I think that you're the one throwing a tantrum and yelling that it
> needs to be done your way or else.  We generally don't change global
> constants because one hardware device doesn't work.
>

fair enough I have 2 devices which don't work properly with bulk.
Device A works by adjusting a hardware register
Device B is documented in this thread.

Device A causes corruptions when the hardwareregister is set to a
buffersize > 16k.
There is certainly something on the physical usb layer which is unknown to the
(software) USB experts here.
The logfiles in the previous email shows up enough and is clear enough.


Why do you think is this documented in the hardware specs of one of our devices?
The chip which has those problems unfortunately doesn't have such registers.

AGAIN:
According to the hardware spec of on of our product
Available Bulk Transfer Size are:
- 188 * n bytes, where n = 1 ~ 256.


Markus

> Chris
>
> --
> Chris Friesen
> Software Developer
> GENBAND
> chris.friesen@genband.com
> www.genband.com
>

Re: [Patch] Increase USBFS Bulk Transfer size

[Alan Cox]

> For my part, I don't much mind raising the limit from 16 KB to 32 KB.  
> 48 KB may be unnecessary, especially since one of the devices doesn't 
> even work when the transfer buffer is that large.
> 
> However it's just a work-around, not a real solution to the underlying
> problem (whatever that problem turns out to be).  Part of the reason
> for keeping the limit low is that it shouldn't matter, since people
> ought to be able to use a bunch of small transfers rather than one
> large transfer.  Apparently that justification doesn't work with these
> webcams, although there's no known reason why it shouldn't.
> 
> But I'm not the person in charge of these decisions...

Well if the underlying solution is crap hardware with no work around its
a bit hard to avoid. A more conservative approach would be to put the
'constant' in sysfs where it belongs so it can be adjusted and special
cased.

Alan

Re: [Patch] Increase USBFS Bulk Transfer size

[Alan Stern]

On Thu, 13 Oct 2011, Markus Rechberger wrote:

> fair enough I have 2 devices which don't work properly with bulk.
> Device A works by adjusting a hardware register
> Device B is documented in this thread.
> 
> Device A causes corruptions when the hardwareregister is set to a
> buffersize > 16k.
> There is certainly something on the physical usb layer which is unknown to the
> (software) USB experts here.
> The logfiles in the previous email shows up enough and is clear enough.

When you ran the test using the 12288/11776 sizes, did you change the 
hardware register buffersize value?  Or did you leave it set to the 
same 24064 as in the other test?

Alan Stern

Re: [Patch] Increase USBFS Bulk Transfer size

[Markus Rechberger]

On Thu, Oct 13, 2011 at 10:07 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Thu, 13 Oct 2011, Markus Rechberger wrote:
>
>> fair enough I have 2 devices which don't work properly with bulk.
>> Device A works by adjusting a hardware register
>> Device B is documented in this thread.
>>
>> Device A causes corruptions when the hardwareregister is set to a
>> buffersize > 16k.
>> There is certainly something on the physical usb layer which is unknown to the
>> (software) USB experts here.
>> The logfiles in the previous email shows up enough and is clear enough.
>
> When you ran the test using the 12288/11776 sizes, did you change the
> hardware register buffersize value?

sorry for confusing you here, this device does not have such a register.

Device A has the hardware register, the buffer is currently set to
~16kb for it and it is okay, it does not
require the patch. By increasing the hw register to >16kb the transfer
is also damaged.

Device B unfortunately not and only accepts 24064 bytes, before
getting that constant I tried
several other smaller buffer sizes and it did not succeed (result is
just as indicated by the video and logfiles in the earlier
post). CPU usage is also okay 0.9% for the entire streaming application.

Markus

> Or did you leave it set to the same 24064 as in the other test?
>
> Alan Stern
>
>

Re: [Patch] Increase USBFS Bulk Transfer size

[Markus Rechberger]

On Fri, Oct 14, 2011 at 12:19 AM, James Courtier-Dutton
<james.dutton@gmail.com> wrote:
> Why don't you try a bulk size of 12032 instead of 24064 and not try 12288 as
> you appear to be doing in the logs. Post the logs for that.

I tried that earlier already of course it fails. If I could pick a
smaller transfer size I would be happy since
the device would work with all 2.6.x kernels out of the box and I
wouldn't have to waste my time with it.
Unfortunately it requires the 24064 bytes.
The more flexible device A which is mentioned here confirms that there
can be some impact on the
bulk transfer size.
However to learn about this it's needed to look at the bottom line of
USB on the physical layer.

And I disagree with Alan Cox it's not about being a crappy device or
not, it's more like about something
that is not well understood here. Most people are familiar with
Software only here and not with the physical
USB bottom Layer, otherwise the fact that the devices can have an
impact on this wouldn't be such a surprise.

Markus

Re: [Patch] Increase USBFS Bulk Transfer size

[Markus Rechberger]

On Fri, Oct 14, 2011 at 4:47 AM, Markus Rechberger
<mrechberger@gmail.com> wrote:
> On Fri, Oct 14, 2011 at 12:19 AM, James Courtier-Dutton
> <james.dutton@gmail.com> wrote:
>> Why don't you try a bulk size of 12032 instead of 24064 and not try 12288 as
>> you appear to be doing in the logs. Post the logs for that.
>
> I tried that earlier already of course it fails. If I could pick a
> smaller transfer size I would be happy since
> the device would work with all 2.6.x kernels out of the box and I
> wouldn't have to waste my time with it.
> Unfortunately it requires the 24064 bytes.
> The more flexible device A which is mentioned here confirms that there
> can be some impact on the
> bulk transfer size.
> However to learn about this it's needed to look at the bottom line of
> USB on the physical layer.
>
> And I disagree with Alan Cox it's not about being a crappy device or
> not, it's more like about something
> that is not well understood here. Most people are familiar with
> Software only here and not with the physical
> USB bottom Layer, otherwise the fact that the devices can have an
> impact on this wouldn't be such a surprise.
>

however to not say that I'm unwilling to do that and that is the
reason for not accepting this patch
http://www.sundtek.de/support/notworking2.log

even if the value is exposed to sysfs, it still requires the static
value in the kernel. The current value
used in the patch is based on what is in the HW specs of device A
which has the flexible bulk transfer setting.
The inflexible device which uses 24064 bytes works with all other
Operating systems by using that value
and gives exactly the same results with other transfer sizes than that.

BR,
Markus

Re: [Patch] Increase USBFS Bulk Transfer size

[Markus Rechberger]

On Fri, Oct 14, 2011 at 5:42 AM, Markus Rechberger
<mrechberger@gmail.com> wrote:
> On Fri, Oct 14, 2011 at 4:47 AM, Markus Rechberger
> <mrechberger@gmail.com> wrote:
>> On Fri, Oct 14, 2011 at 12:19 AM, James Courtier-Dutton
>> <james.dutton@gmail.com> wrote:
>>> Why don't you try a bulk size of 12032 instead of 24064 and not try 12288 as
>>> you appear to be doing in the logs. Post the logs for that.
>>
>> I tried that earlier already of course it fails. If I could pick a
>> smaller transfer size I would be happy since
>> the device would work with all 2.6.x kernels out of the box and I
>> wouldn't have to waste my time with it.
>> Unfortunately it requires the 24064 bytes.
>> The more flexible device A which is mentioned here confirms that there
>> can be some impact on the
>> bulk transfer size.
>> However to learn about this it's needed to look at the bottom line of
>> USB on the physical layer.
>>
>> And I disagree with Alan Cox it's not about being a crappy device or
>> not, it's more like about something
>> that is not well understood here. Most people are familiar with
>> Software only here and not with the physical
>> USB bottom Layer, otherwise the fact that the devices can have an
>> impact on this wouldn't be such a surprise.
>>
>
> however to not say that I'm unwilling to do that and that is the
> reason for not accepting this patch
> http://www.sundtek.de/support/notworking2.log
>
> even if the value is exposed to sysfs, it still requires the static
> value in the kernel. The current value
> used in the patch is based on what is in the HW specs of device A
> which has the flexible bulk transfer setting.
> The inflexible device which uses 24064 bytes works with all other
> Operating systems by using that value
> and gives exactly the same results with other transfer sizes than that.
>

I didn't check this before the half of it is of course not a multiple
of 512 so the
logfile only shows up 11776 of course.

24064 is the smallest common multiple of 188 and 512.

BR,
Markus

Re: [Patch] Increase USBFS Bulk Transfer size

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 424 bytes --]

On Fri, 14 Oct 2011 05:42:44 +0200, Markus Rechberger said:

> The inflexible device which uses 24064 bytes works with all other
> Operating systems by using that value
> and gives exactly the same results with other transfer sizes than that.

-ENOPARSE.  If it's inflexible, how did you get results for other transfer
sizes? And if other transfer sizes give exactly the same results, why can't you
just use those instead?


[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

Re: [Patch] Increase USBFS Bulk Transfer size

[Markus Rechberger]

On Fri, Oct 14, 2011 at 7:47 AM,  <Valdis.Kletnieks@vt.edu> wrote:
> On Fri, 14 Oct 2011 05:42:44 +0200, Markus Rechberger said:
>
>> The inflexible device which uses 24064 bytes works with all other
>> Operating systems by using that value
>> and gives exactly the same results with other transfer sizes than that.
>
> -ENOPARSE.  If it's inflexible,

the particular device in question is inflexible yes.

> how did you get results for other transfer
> sizes?

other transfer sizes are related to another more flexible device, but that
device has some hardware register to modify the needed transfer buffer size.

> And if other transfer sizes give exactly the same results, why can't you
> just use those instead?
>

other transfer sizes for the device with the fixed transfer buffer
size give exactly the same result with OSX, meaning that it also
does not work on MacOSX with another size than 24064 bytes.

Final conclusion is still:
1. those people who used to deal with USB with linux in the past never
heard about such an option it seems, fact is that the stream corrupts
the logfiles show up damaged data before it arrives to the application
so on that side bugs in the application would be unrelated. As a
crosscheck I also posted a logfile with valid data.
2. the more flexible device even has hardware registers to modify the
bulk transfer size (maybe it's a timing related issue on the physical
usb bus).
3. if the more flexible device is set to buffers > 16k the data also corrupts
4. the fixed buffer size device does not accept any other values, even
twice that requested buffersize corrupts the data.
5. the patch does not hurt, since devices which request buffer sizes
below 16k will still be handled the same way as they were handled
during the last 10 years, it just allows applications to request
bigger buffers if needed.
6. BOTH of our devices (the flexible one device A, can work with
bigger buffers AND a bigger HW register configuration, device B which
is not so flexible also works flawlessly as seen in the video).

I'm repeating myself over and over here.

Markus

Re: [Patch] Increase USBFS Bulk Transfer size

[James Courtier-Dutton]

On 14 October 2011 07:23, Markus Rechberger <mrechberger@gmail.com> wrote:
> On Fri, Oct 14, 2011 at 7:47 AM,  <Valdis.Kletnieks@vt.edu> wrote:
>> On Fri, 14 Oct 2011 05:42:44 +0200, Markus Rechberger said:
>>
>>> The inflexible device which uses 24064 bytes works with all other
>>> Operating systems by using that value
>>> and gives exactly the same results with other transfer sizes than that.
>>
>> -ENOPARSE.  If it's inflexible,
>
> the particular device in question is inflexible yes.
>
Here is what I think the actual situation is.
Your transfers over the USB are done in 512 Bytes chunks.
If the device wishes to send 512 bytes, it sends one chunk.
If the device wishes to send 513 bytes, it sends only one chunk,
missing one byte.
if the device wishes to send 1023 bytes, it sends only one chunk,
missing 511 bytes.
if the device wishes to send 1024 bytes, it sends two chunks, missing 0 bytes.
So, the device is sending 1 too few chunks unless the bytes size
exactly matches the chunk size * n.
Another constraint is the device sends multiples of 188 bytes.
So, unless we can find a lowest common multiple of 188 and 512, there
is no transfer size that will work with this device. The LCM is 24064
and this is the only value that will work with this device.

Conclusion:
The hardware on the Linux PC and the kernel on the Linux PC are
working correctly.
Your external USB device has an off-by-one error in its
hardware/embedded software and the hardware/embedded software
manufacturer is not prepared to fix it.

Workaround in software: Increase MAX from 16384 to 24064 or above.

My own feeling, throw away this faulty bit of hardware and use a different part.
The hardware is not compatible with USB standards.

Kind Regards

James

Re: [Patch] Increase USBFS Bulk Transfer size

[Alan Stern]

On Thu, 13 Oct 2011, Alan Cox wrote:

> Well if the underlying solution is crap hardware with no work around its
> a bit hard to avoid. A more conservative approach would be to put the   
> 'constant' in sysfs where it belongs so it can be adjusted and special  
> cased.

On Fri, 14 Oct 2011, Markus Rechberger wrote:

> even if the value is exposed to sysfs, it still requires the static
> value in the kernel. The current value
> used in the patch is based on what is in the HW specs of device A
> which has the flexible bulk transfer setting.
> The inflexible device which uses 24064 bytes works with all other
> Operating systems by using that value
> and gives exactly the same results with other transfer sizes than that.

When you think about it, what's the real reason for limiting transfer
sizes?  Part of it has to do with avoiding large contiguous memory
allocations, of course, but that can't be the real reason.  After all,
if a memory allocation fails, there's no damage done except to the
program submitting the transfer -- and then it's clearly the program's
own fault for submitting a transfer that's too large.

A little thought shows the only reason for having this sort of limit is
to avoid denial-of-service attacks caused by dedicating too much kernel
memory to URB transfer buffers.  But limiting the size of individual
transfer buffers isn't the right way to do this!  There's nothing to
prevent a program from submitting many, many transfers, each one under
the size limit but all together exhausting available memory.

No, a much better approach is to remove all limits on individual
transfer sizes and instead have a global limit on the total amount of
all usbfs buffers in use at any time.  Maybe something like 16 MB; at 
SuperSpeed, that's about about 30 ms worth of data.

Greg, what do you think?

Alan Stern

Re: [Patch] Increase USBFS Bulk Transfer size

[Greg KH]

On Fri, Oct 14, 2011 at 10:05:41AM -0400, Alan Stern wrote:
> No, a much better approach is to remove all limits on individual
> transfer sizes and instead have a global limit on the total amount of
> all usbfs buffers in use at any time.  Maybe something like 16 MB; at 
> SuperSpeed, that's about about 30 ms worth of data.

That sounds quite reasonable.

greg k-h

Re: [Patch] Increase USBFS Bulk Transfer size

[Markus Rechberger]

On Fri, Oct 14, 2011 at 10:51 AM, James Courtier-Dutton
<james.dutton@gmail.com> wrote:
> On 14 October 2011 07:23, Markus Rechberger <mrechberger@gmail.com> wrote:
>> On Fri, Oct 14, 2011 at 7:47 AM,  <Valdis.Kletnieks@vt.edu> wrote:
>>> On Fri, 14 Oct 2011 05:42:44 +0200, Markus Rechberger said:
>>>
>>>> The inflexible device which uses 24064 bytes works with all other
>>>> Operating systems by using that value
>>>> and gives exactly the same results with other transfer sizes than that.
>>>
>>> -ENOPARSE.  If it's inflexible,
>>
>> the particular device in question is inflexible yes.
>>
> Here is what I think the actual situation is.
> Your transfers over the USB are done in 512 Bytes chunks.
> If the device wishes to send 512 bytes, it sends one chunk.
> If the device wishes to send 513 bytes, it sends only one chunk,
> missing one byte.
> if the device wishes to send 1023 bytes, it sends only one chunk,
> missing 511 bytes.
> if the device wishes to send 1024 bytes, it sends two chunks, missing 0 bytes.
> So, the device is sending 1 too few chunks unless the bytes size
> exactly matches the chunk size * n.
> Another constraint is the device sends multiples of 188 bytes.
> So, unless we can find a lowest common multiple of 188 and 512, there
> is no transfer size that will work with this device. The LCM is 24064
> and this is the only value that will work with this device.
>
> Conclusion:
> The hardware on the Linux PC and the kernel on the Linux PC are
> working correctly.
> Your external USB device has an off-by-one error in its
> hardware/embedded software and the hardware/embedded software
> manufacturer is not prepared to fix it.
>
> Workaround in software: Increase MAX from 16384 to 24064 or above.
>
> My own feeling, throw away this faulty bit of hardware and use a different part.
> The hardware is not compatible with USB standards.
>

I agree with that one, and think this conclusion comes closest to the
real issue.
I can imagine that the chip fifo has some issues with that one.
However the device has been tested for a long time without turning to
corrupted data so
the only issue remaining is the buffer alignment.

Thanks for having a look at all that, I wish all requests would have
that kind of review, instead
of "I don't believe you, not acceptable, not right solution at all"

BR,
Markus

Re: [Patch] Increase USBFS Bulk Transfer size

[Johannes Stezenbach]

On Thu, Oct 13, 2011 at 10:58:39AM -0400, Alan Stern wrote:
> On Thu, 13 Oct 2011, Markus Rechberger wrote:
> 
> > And for those who are curious about the logfiles:
> > Not working one as proposed by Alan that the full buffer size should
> > be split into 2 requests:
> > 
> > ffff8800b38d9f00 1231540351 S Bi:2:013:1 -115 12288 <
> > ffff8800b38d96c0 1231540404 S Bi:2:013:1 -115 11776 <
... 
> This is very interesting.  There are only two things that could be
> happening: Either the device sends different data during the two tests,
> or there's a bug in the kernel.
> 
> Now, it is possible the device is sending bad data.  The initial parts
> of the two logs do not agree exactly; there are numerous small
> differences in the control data sent by the device and by the program.  
> I don't know whether they are significant, but if they aren't, there's
> no reason for the device to send different bulk data.  The transfer
> size certainly cannot account for it.  Indeed, even if the transfer
> size were only 512 bytes, the first data packet should still be the 
> same.

I don't really want to help Markus with his proprietary, binary-only
userspace driver crap, but I wonder why nobody seems to remember
how the USB protocol works on the wire?  The transfer size is
never seen by the device, thus it cannot matter if two small URBs
or one large URB are queued.  What matters is the packet size.
Apparently the device can only handle fixed size packets
of either 188 or 2*188 byte, thus it breaks with 12288 or 11776.
The endpoint's wMaxPacketSize might reflect this.
I guess a transfer size of e.g. 188*60=11280 would work.
See the first mail of this thread.

See also Sergei's comment in
http://lkml.org/lkml/2011/10/12/183


Johannes

Re: [Patch] Increase USBFS Bulk Transfer size

[Alan Stern]

On Fri, 14 Oct 2011, Johannes Stezenbach wrote:

> I don't really want to help Markus with his proprietary, binary-only
> userspace driver crap, but I wonder why nobody seems to remember
> how the USB protocol works on the wire?

I remember it perfectly well.

>  The transfer size is
> never seen by the device, thus it cannot matter if two small URBs
> or one large URB are queued.  What matters is the packet size.

That's what I have been saying.  Markus's experience contradicts this,
however.

> Apparently the device can only handle fixed size packets
> of either 188 or 2*188 byte, thus it breaks with 12288 or 11776.

No.  The device expects 512-byte packets because it uses a bulk 
endpoint.

> The endpoint's wMaxPacketSize might reflect this.

For high-speed devices, a bulk endpoint's wMaxPacketSize must always be 
512.

> I guess a transfer size of e.g. 188*60=11280 would work.
> See the first mail of this thread.

According to Markus, with this particular device nothing but 24064 
works.  The discussion is a little difficult to follow because he 
talked about two different devices without always being clear about 
which was which.

> See also Sergei's comment in
> http://lkml.org/lkml/2011/10/12/183

Alan Stern

Re: [Patch] Increase USBFS Bulk Transfer size

[Johannes Stezenbach]

On Fri, Oct 14, 2011 at 04:19:34PM -0400, Alan Stern wrote:
> On Fri, 14 Oct 2011, Johannes Stezenbach wrote:
> 
> > I don't really want to help Markus with his proprietary, binary-only
> > userspace driver crap, but I wonder why nobody seems to remember
> > how the USB protocol works on the wire?
> 
> I remember it perfectly well.

My bad, I'm sorry for hitting the wrong tone.

What I meant to say is Markus' statement that the device only
works at a certain transfer size cannot be true since
this size is not visible to the device via the USB bus.

> > Apparently the device can only handle fixed size packets
> > of either 188 or 2*188 byte, thus it breaks with 12288 or 11776.
> 
> No.  The device expects 512-byte packets because it uses a bulk 
> endpoint.
> 
> > The endpoint's wMaxPacketSize might reflect this.
> 
> For high-speed devices, a bulk endpoint's wMaxPacketSize must always be 
> 512.

OK, after re-reading the USB spec I see you are right
and I stand corrected.

> > I guess a transfer size of e.g. 188*60=11280 would work.
> > See the first mail of this thread.
> 
> According to Markus, with this particular device nothing but 24064 
> works.  The discussion is a little difficult to follow because he 
> talked about two different devices without always being clear about 
> which was which.

If you queue two URBs, one 12288 and 11776 bytes, the device
does not see any difference to one URB with 24064.  It's just not
in the USB wire protocol.  It would make a difference if the
device violated the spec and sent 188 byte packets. However, the
spec says a short packet terminates the transfer.  But I wonder
if this is really the case?


Johannes

Re: [Patch] Increase USBFS Bulk Transfer size

[Markus Rechberger]

On Sat, Oct 15, 2011 at 12:45 AM, Johannes Stezenbach <js@sig21.net> wrote:
> On Fri, Oct 14, 2011 at 04:19:34PM -0400, Alan Stern wrote:
>> On Fri, 14 Oct 2011, Johannes Stezenbach wrote:
>>
>> > I don't really want to help Markus with his proprietary, binary-only
>> > userspace driver crap, but I wonder why nobody seems to remember
>> > how the USB protocol works on the wire?
>>
>> I remember it perfectly well.
>
> My bad, I'm sorry for hitting the wrong tone.
>
> What I meant to say is Markus' statement that the device only
> works at a certain transfer size cannot be true since
> this size is not visible to the device via the USB bus.
>

The log files and demonstration video say something different. Plus
the same happens with MacOSX. And
Windows is using the same. So that means that something has been
overlooked so far.
The "off-by-one error" must be caused by something. In practice it
doesn't seem to be relevant as all
other USB stacks can handle this.

>> > Apparently the device can only handle fixed size packets
>> > of either 188 or 2*188 byte, thus it breaks with 12288 or 11776.
>>
>> No.  The device expects 512-byte packets because it uses a bulk
>> endpoint.
>>
>> > The endpoint's wMaxPacketSize might reflect this.
>>
>> For high-speed devices, a bulk endpoint's wMaxPacketSize must always be
>> 512.
>
> OK, after re-reading the USB spec I see you are right
> and I stand corrected.
>
>> > I guess a transfer size of e.g. 188*60=11280 would work.
>> > See the first mail of this thread.
>>
>> According to Markus, with this particular device nothing but 24064
>> works.  The discussion is a little difficult to follow because he
>> talked about two different devices without always being clear about
>> which was which.
>
> If you queue two URBs, one 12288 and 11776 bytes, the device
> does not see any difference to one URB with 24064.  It's just not
> in the USB wire protocol.  It would make a difference if the
> device violated the spec and sent 188 byte packets. However, the
> spec says a short packet terminates the transfer.  But I wonder
> if this is really the case?
>

I can send a sample device in around one month to Alan so he can have a look
at it with the bus analyzer. However I would be happy to see Alan's
proposal about the
16 MB limitations included since that one also solves the issue and even USB 3.0
would benefit from this as it seems.

BR,
Markus

Re: [Patch] Increase USBFS Bulk Transfer size

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 524 bytes --]

On Sat, 15 Oct 2011 13:45:27 +0200, Markus Rechberger said:

> The log files and demonstration video say something different. Plus
> the same happens with MacOSX. And
> Windows is using the same. So that means that something has been
> overlooked so far.
> The "off-by-one error" must be caused by something. In practice it
> doesn't seem to be relevant as all
> other USB stacks can handle this.

It's quite possible that what's being overlooked is that the device
only accidentally works with OSX and Windows?

[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

Re: [Patch] Increase USBFS Bulk Transfer size

[Alan Stern]

On Sat, 15 Oct 2011, Johannes Stezenbach wrote:

> What I meant to say is Markus' statement that the device only
> works at a certain transfer size cannot be true since
> this size is not visible to the device via the USB bus.

That's what I would expect, too.  But did you take a look at the usbmon 
traces Markus acquired?

	http://marc.info/?l=linux-usb&m=131845614819045&w=2

They aren't completely definitive because the communications between
the computer and the device _before_ the bulk transfers started were
different.  However they do clearly show the device working with
24064-byte transfers and not working with 12288+11776-byte transfers.

> If you queue two URBs, one 12288 and 11776 bytes, the device
> does not see any difference to one URB with 24064.  It's just not
> in the USB wire protocol.

You can argue until you're blue in the face.  It won't affect the 
results that Markus got in real life.

>  It would make a difference if the
> device violated the spec and sent 188 byte packets. However, the
> spec says a short packet terminates the transfer.  But I wonder
> if this is really the case?

The device does not send short packets.  If it did, the 24064-byte 
transfers would end early.

Alan Stern

Re: [Patch] Increase USBFS Bulk Transfer size

[Alan Stern]

On Sat, 15 Oct 2011, Markus Rechberger wrote:

> I can send a sample device in around one month to Alan so he can have a look
> at it with the bus analyzer. However I would be happy to see Alan's
> proposal about the
> 16 MB limitations included since that one also solves the issue and even USB 3.0
> would benefit from this as it seems.

I'll post a pair of patches in a few days, after I finish testing them.

Alan Stern

Re: [Patch] Increase USBFS Bulk Transfer size

[Johannes Stezenbach]

On Sat, Oct 15, 2011 at 03:04:28PM -0400, Alan Stern wrote:
> On Sat, 15 Oct 2011, Johannes Stezenbach wrote:
> 
> > What I meant to say is Markus' statement that the device only
> > works at a certain transfer size cannot be true since
> > this size is not visible to the device via the USB bus.
> 
> That's what I would expect, too.  But did you take a look at the usbmon 
> traces Markus acquired?
> 
> 	http://marc.info/?l=linux-usb&m=131845614819045&w=2

I glanced at them for 3 seconds, but I cannot be bothered
to analyze them in detail.  The ASCII usbmon traces don't
show full USB packet contents anyway so you can't see if
partial MPEG TS packets are missing.

> They aren't completely definitive because the communications between
> the computer and the device _before_ the bulk transfers started were
> different.  However they do clearly show the device working with
> 24064-byte transfers and not working with 12288+11776-byte transfers.

The device transfers data in the "not working" case, it's just that
the MPEG TS sync byte is not where Markus expects it, which could
be explained by a partial MPEG TS packet left in the device's FIFO
from previous interaction.


Johannes

Re: [Patch] Increase USBFS Bulk Transfer size

[James Courtier-Dutton]

On 13 October 2011 10:29, Markus Rechberger <mrechberger@gmail.com> wrote:
>
> Some demonstration video:
> (with 24064 bytes, which uses the additional buffers which are allowed
> by the introduced patch):
> Flawlessly video:
> http://www.sundtek.de/support/00011.MTS
>
> (Alan's proposed buffer size 11776/12288)
> Jerky video:
> http://www.sundtek.de/support/00012.MTS
>
> both videos are around 30 seconds made with a camera, video is still uploading.
> this is my last post to that topic I still have some little hope that
> this patch will get in to fix that issue.
> As proposed for academic research you can pick up a Stick in Berlin
> once the new cut is ready from manufacturing,
>

Can you post the actual captured stream, and not a video camera copy
of a media player playing the stream.

Re: [Patch] Increase USBFS Bulk Transfer size

[Alan Stern]

On Sun, 16 Oct 2011, Johannes Stezenbach wrote:

> On Sat, Oct 15, 2011 at 03:04:28PM -0400, Alan Stern wrote:
> > On Sat, 15 Oct 2011, Johannes Stezenbach wrote:
> > 
> > > What I meant to say is Markus' statement that the device only
> > > works at a certain transfer size cannot be true since
> > > this size is not visible to the device via the USB bus.
> > 
> > That's what I would expect, too.  But did you take a look at the usbmon 
> > traces Markus acquired?
> > 
> > 	http://marc.info/?l=linux-usb&m=131845614819045&w=2
> 
> I glanced at them for 3 seconds, but I cannot be bothered
> to analyze them in detail.  The ASCII usbmon traces don't
> show full USB packet contents anyway so you can't see if
> partial MPEG TS packets are missing.

> The device transfers data in the "not working" case, it's just that
> the MPEG TS sync byte is not where Markus expects it, which could
> be explained by a partial MPEG TS packet left in the device's FIFO
> from previous interaction.

But why does this happen only when the transfer size is different from
24064?  Why doesn't it also happen when the transfer size _is_ 24064?

Alan Stern

Re: [Patch] Increase USBFS Bulk Transfer size

[Johannes Stezenbach]

On Sat, Oct 15, 2011 at 03:04:28PM -0400, Alan Stern wrote:
> On Sat, 15 Oct 2011, Johannes Stezenbach wrote:
> 
> >  It would make a difference if the
> > device violated the spec and sent 188 byte packets. However, the
> > spec says a short packet terminates the transfer.  But I wonder
> > if this is really the case?
> 
> The device does not send short packets.  If it did, the 24064-byte 
> transfers would end early.

Re-reading the USB-2.0 standard, a short packet which terminates
the transfer is defined by packet_size < wMaxPacketSize,
not by packet_size < 512.
Thus wMaxPacketSize == 188 (or 2*188) might be possible.

There is a comment in linux/drivers/usb/host/ehci-q.c:

	/* The USB spec says that high speed bulk endpoints
	 * always use 512 byte maxpacket.  But some device
	 * vendors decided to ignore that, and MSFT is happy
	 * to help them do so.  So now people expect to use
	 * such nonconformant devices with Linux too; sigh.
	 */

Maybe we should look at the descriptors?


Johannes

Re: [Patch] Increase USBFS Bulk Transfer size

[Alan Stern]

On Mon, 17 Oct 2011, Johannes Stezenbach wrote:

> On Sat, Oct 15, 2011 at 03:04:28PM -0400, Alan Stern wrote:
> > On Sat, 15 Oct 2011, Johannes Stezenbach wrote:
> > 
> > >  It would make a difference if the
> > > device violated the spec and sent 188 byte packets. However, the
> > > spec says a short packet terminates the transfer.  But I wonder
> > > if this is really the case?
> > 
> > The device does not send short packets.  If it did, the 24064-byte 
> > transfers would end early.
> 
> Re-reading the USB-2.0 standard, a short packet which terminates
> the transfer is defined by packet_size < wMaxPacketSize,
> not by packet_size < 512.
> Thus wMaxPacketSize == 188 (or 2*188) might be possible.
> 
> There is a comment in linux/drivers/usb/host/ehci-q.c:
> 
> 	/* The USB spec says that high speed bulk endpoints
> 	 * always use 512 byte maxpacket.  But some device
> 	 * vendors decided to ignore that, and MSFT is happy
> 	 * to help them do so.  So now people expect to use
> 	 * such nonconformant devices with Linux too; sigh.
> 	 */

That comment referred to devices with wMaxPacketSize = 1024, which has
been seen in the wild.  But other bizarre sizes are possible.

> Maybe we should look at the descriptors?

It won't hurt, although I would be quite surprised to see anything
other than 512.

Markus, can you post the output from "lsusb -v" for this inflexible 
webcam?

Alan Stern

Re: [Patch] Increase USBFS Bulk Transfer size

[Alan Stern]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 673 bytes --]

Markus:

Attached is a sequence of two patches for you to try out.  The second
is meant to be applied on top of the first.  I based them on 3.1-rc4
(more or less) so they may need a little tweaking for your kernel, but
hopefully not very much.

The first patch simplifies the error pathways for the USB submission
routines in usbfs.  The second patch removes the limits on the sizes of
individual URBs and replaces them with a global limit on the total
amount of memory allocated by usbfs.

They work okay on my system but I don't have any good way to give them
a thorough test.  Please try them out and see if they allow your webcam
programs to work properly.

Alan Stern

[-- Attachment #2: Type: TEXT/PLAIN, Size: 6852 bytes --]

 drivers/usb/core/devio.c |   96 ++++++++++++++++++++++++-----------------------
 1 file changed, 50 insertions(+), 46 deletions(-)

Index: usb-3.1/drivers/usb/core/devio.c
===================================================================
--- usb-3.1.orig/drivers/usb/core/devio.c
+++ usb-3.1/drivers/usb/core/devio.c
@@ -796,8 +796,8 @@ static int proc_control(struct dev_state
 	if (ctrl.bRequestType & 0x80) {
 		if (ctrl.wLength && !access_ok(VERIFY_WRITE, ctrl.data,
 					       ctrl.wLength)) {
-			free_page((unsigned long)tbuf);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto done;
 		}
 		pipe = usb_rcvctrlpipe(dev, 0);
 		snoop_urb(dev, NULL, pipe, ctrl.wLength, tmo, SUBMIT, NULL, 0);
@@ -811,15 +811,15 @@ static int proc_control(struct dev_state
 			  tbuf, max(i, 0));
 		if ((i > 0) && ctrl.wLength) {
 			if (copy_to_user(ctrl.data, tbuf, i)) {
-				free_page((unsigned long)tbuf);
-				return -EFAULT;
+				ret = -EFAULT;
+				goto done;
 			}
 		}
 	} else {
 		if (ctrl.wLength) {
 			if (copy_from_user(tbuf, ctrl.data, ctrl.wLength)) {
-				free_page((unsigned long)tbuf);
-				return -EFAULT;
+				ret = -EFAULT;
+				goto done;
 			}
 		}
 		pipe = usb_sndctrlpipe(dev, 0);
@@ -833,14 +833,16 @@ static int proc_control(struct dev_state
 		usb_lock_device(dev);
 		snoop_urb(dev, NULL, pipe, max(i, 0), min(i, 0), COMPLETE, NULL, 0);
 	}
-	free_page((unsigned long)tbuf);
 	if (i < 0 && i != -EPIPE) {
 		dev_printk(KERN_DEBUG, &dev->dev, "usbfs: USBDEVFS_CONTROL "
 			   "failed cmd %s rqt %u rq %u len %u ret %d\n",
 			   current->comm, ctrl.bRequestType, ctrl.bRequest,
 			   ctrl.wLength, i);
 	}
-	return i;
+	ret = i;
+ done:
+	free_page((unsigned long) tbuf);
+	return ret;
 }
 
 static int proc_bulk(struct dev_state *ps, void __user *arg)
@@ -874,8 +876,8 @@ static int proc_bulk(struct dev_state *p
 	tmo = bulk.timeout;
 	if (bulk.ep & 0x80) {
 		if (len1 && !access_ok(VERIFY_WRITE, bulk.data, len1)) {
-			kfree(tbuf);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto done;
 		}
 		snoop_urb(dev, NULL, pipe, len1, tmo, SUBMIT, NULL, 0);
 
@@ -886,15 +888,15 @@ static int proc_bulk(struct dev_state *p
 
 		if (!i && len2) {
 			if (copy_to_user(bulk.data, tbuf, len2)) {
-				kfree(tbuf);
-				return -EFAULT;
+				ret = -EFAULT;
+				goto done;
 			}
 		}
 	} else {
 		if (len1) {
 			if (copy_from_user(tbuf, bulk.data, len1)) {
-				kfree(tbuf);
-				return -EFAULT;
+				ret = -EFAULT;
+				goto done;
 			}
 		}
 		snoop_urb(dev, NULL, pipe, len1, tmo, SUBMIT, tbuf, len1);
@@ -904,10 +906,10 @@ static int proc_bulk(struct dev_state *p
 		usb_lock_device(dev);
 		snoop_urb(dev, NULL, pipe, len2, i, COMPLETE, NULL, 0);
 	}
+	ret = (i < 0 ? i : len2);
+ done:
 	kfree(tbuf);
-	if (i < 0)
-		return i;
-	return len2;
+	return ret;
 }
 
 static int proc_resetep(struct dev_state *ps, void __user *arg)
@@ -1052,7 +1054,7 @@ static int proc_do_submiturb(struct dev_
 {
 	struct usbdevfs_iso_packet_desc *isopkt = NULL;
 	struct usb_host_endpoint *ep;
-	struct async *as;
+	struct async *as = NULL;
 	struct usb_ctrlrequest *dr = NULL;
 	const struct cred *cred = current_cred();
 	unsigned int u, totlen, isofrmlen;
@@ -1099,19 +1101,17 @@ static int proc_do_submiturb(struct dev_
 		if (!dr)
 			return -ENOMEM;
 		if (copy_from_user(dr, uurb->buffer, 8)) {
-			kfree(dr);
-			return -EFAULT;
+			ret = -EFAULT;
+			goto error;
 		}
 		if (uurb->buffer_length < (le16_to_cpup(&dr->wLength) + 8)) {
-			kfree(dr);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto error;
 		}
 		ret = check_ctrlrecip(ps, dr->bRequestType,
 				      le16_to_cpup(&dr->wIndex));
-		if (ret) {
-			kfree(dr);
-			return ret;
-		}
+		if (ret)
+			goto error;
 		uurb->number_of_packets = 0;
 		uurb->buffer_length = le16_to_cpup(&dr->wLength);
 		uurb->buffer += 8;
@@ -1167,22 +1167,22 @@ static int proc_do_submiturb(struct dev_
 		if (!(isopkt = kmalloc(isofrmlen, GFP_KERNEL)))
 			return -ENOMEM;
 		if (copy_from_user(isopkt, iso_frame_desc, isofrmlen)) {
-			kfree(isopkt);
-			return -EFAULT;
+			ret = -EFAULT;
+			goto error;
 		}
 		for (totlen = u = 0; u < uurb->number_of_packets; u++) {
 			/* arbitrary limit,
 			 * sufficient for USB 2.0 high-bandwidth iso */
 			if (isopkt[u].length > 8192) {
-				kfree(isopkt);
-				return -EINVAL;
+				ret = -EINVAL;
+				goto error;
 			}
 			totlen += isopkt[u].length;
 		}
 		/* 3072 * 64 microframes */
 		if (totlen > 196608) {
-			kfree(isopkt);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto error;
 		}
 		uurb->buffer_length = totlen;
 		break;
@@ -1193,24 +1193,20 @@ static int proc_do_submiturb(struct dev_
 	if (uurb->buffer_length > 0 &&
 			!access_ok(is_in ? VERIFY_WRITE : VERIFY_READ,
 				uurb->buffer, uurb->buffer_length)) {
-		kfree(isopkt);
-		kfree(dr);
-		return -EFAULT;
+		ret = -EFAULT;
+		goto error;
 	}
 	as = alloc_async(uurb->number_of_packets);
 	if (!as) {
-		kfree(isopkt);
-		kfree(dr);
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto error;
 	}
 	if (uurb->buffer_length > 0) {
 		as->urb->transfer_buffer = kmalloc(uurb->buffer_length,
 				GFP_KERNEL);
 		if (!as->urb->transfer_buffer) {
-			kfree(isopkt);
-			kfree(dr);
-			free_async(as);
-			return -ENOMEM;
+			ret = -ENOMEM;
+			goto error;
 		}
 		/* Isochronous input data may end up being discontiguous
 		 * if some of the packets are short.  Clear the buffer so
@@ -1244,6 +1240,7 @@ static int proc_do_submiturb(struct dev_
 
 	as->urb->transfer_buffer_length = uurb->buffer_length;
 	as->urb->setup_packet = (unsigned char *)dr;
+	dr = NULL;
 	as->urb->start_frame = uurb->start_frame;
 	as->urb->number_of_packets = uurb->number_of_packets;
 	if (uurb->type == USBDEVFS_URB_TYPE_ISO ||
@@ -1259,6 +1256,7 @@ static int proc_do_submiturb(struct dev_
 		totlen += isopkt[u].length;
 	}
 	kfree(isopkt);
+	isopkt = NULL;
 	as->ps = ps;
 	as->userurb = arg;
 	if (is_in && uurb->buffer_length > 0)
@@ -1274,8 +1272,8 @@ static int proc_do_submiturb(struct dev_
 	if (!is_in && uurb->buffer_length > 0) {
 		if (copy_from_user(as->urb->transfer_buffer, uurb->buffer,
 				uurb->buffer_length)) {
-			free_async(as);
-			return -EFAULT;
+			ret = -EFAULT;
+			goto error;
 		}
 	}
 	snoop_urb(ps->dev, as->userurb, as->urb->pipe,
@@ -1321,10 +1319,16 @@ static int proc_do_submiturb(struct dev_
 		snoop_urb(ps->dev, as->userurb, as->urb->pipe,
 				0, ret, COMPLETE, NULL, 0);
 		async_removepending(as);
-		free_async(as);
-		return ret;
+		goto error;
 	}
 	return 0;
+
+ error:
+	kfree(isopkt);
+	kfree(dr);
+	if (as)
+		free_async(as);
+	return ret;
 }
 
 static int proc_submiturb(struct dev_state *ps, void __user *arg)

[-- Attachment #3: Type: TEXT/PLAIN, Size: 5554 bytes --]

 drivers/usb/core/devio.c |   76 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 57 insertions(+), 19 deletions(-)

Index: usb-3.1/drivers/usb/core/devio.c
===================================================================
--- usb-3.1.orig/drivers/usb/core/devio.c
+++ usb-3.1/drivers/usb/core/devio.c
@@ -85,6 +85,7 @@ struct async {
 	void __user *userbuffer;
 	void __user *userurb;
 	struct urb *urb;
+	unsigned int mem_usage;
 	int status;
 	u32 secid;
 	u8 bulk_addr;
@@ -107,8 +108,26 @@ enum snoop_when {
 
 #define USB_DEVICE_DEV		MKDEV(USB_DEVICE_MAJOR, 0)
 
-#define	MAX_USBFS_BUFFER_SIZE	16384
+/* Limit on the total amount of memory we can allocate for transfers */
+#define MAX_USBFS_MEMORY_USAGE	16777216	/* 16 MB */
 
+static atomic_t usbfs_memory_usage;	/* Total memory currently allocated */
+
+/* Check whether it's okay to allocate more memory for a transfer */
+static int usbfs_increase_memory_usage(unsigned amount)
+{
+	atomic_add(amount, &usbfs_memory_usage);
+	if (atomic_read(&usbfs_memory_usage) <= MAX_USBFS_MEMORY_USAGE)
+		return 0;
+	atomic_sub(amount, &usbfs_memory_usage);
+	return -ENOMEM;
+}
+
+/* Memory for a transfer is being deallocated */
+static void usbfs_decrease_memory_usage(unsigned amount)
+{
+	atomic_sub(amount, &usbfs_memory_usage);
+}
 
 static int connected(struct dev_state *ps)
 {
@@ -251,6 +270,7 @@ static void free_async(struct async *as)
 	kfree(as->urb->transfer_buffer);
 	kfree(as->urb->setup_packet);
 	usb_free_urb(as->urb);
+	usbfs_decrease_memory_usage(as->mem_usage);
 	kfree(as);
 }
 
@@ -782,9 +802,15 @@ static int proc_control(struct dev_state
 	wLength = ctrl.wLength;		/* To suppress 64k PAGE_SIZE warning */
 	if (wLength > PAGE_SIZE)
 		return -EINVAL;
+	ret = usbfs_increase_memory_usage(PAGE_SIZE + sizeof(struct urb) +
+			sizeof(struct usb_ctrlrequest));
+	if (ret)
+		return ret;
 	tbuf = (unsigned char *)__get_free_page(GFP_KERNEL);
-	if (!tbuf)
-		return -ENOMEM;
+	if (!tbuf) {
+		ret = -ENOMEM;
+		goto done;
+	}
 	tmo = ctrl.timeout;
 	snoop(&dev->dev, "control urb: bRequestType=%02x "
 		"bRequest=%02x wValue=%04x "
@@ -842,6 +868,8 @@ static int proc_control(struct dev_state
 	ret = i;
  done:
 	free_page((unsigned long) tbuf);
+	usbfs_decrease_memory_usage(PAGE_SIZE + sizeof(struct urb) +
+			sizeof(struct usb_ctrlrequest));
 	return ret;
 }
 
@@ -869,10 +897,15 @@ static int proc_bulk(struct dev_state *p
 	if (!usb_maxpacket(dev, pipe, !(bulk.ep & USB_DIR_IN)))
 		return -EINVAL;
 	len1 = bulk.len;
-	if (len1 > MAX_USBFS_BUFFER_SIZE)
+	if (len1 > MAX_USBFS_MEMORY_USAGE)
 		return -EINVAL;
-	if (!(tbuf = kmalloc(len1, GFP_KERNEL)))
-		return -ENOMEM;
+	ret = usbfs_increase_memory_usage(len1 + sizeof(struct urb));
+	if (ret)
+		return ret;
+	if (!(tbuf = kmalloc(len1, GFP_KERNEL))) {
+		ret = -ENOMEM;
+		goto done;
+	}
 	tmo = bulk.timeout;
 	if (bulk.ep & 0x80) {
 		if (len1 && !access_ok(VERIFY_WRITE, bulk.data, len1)) {
@@ -909,6 +942,7 @@ static int proc_bulk(struct dev_state *p
 	ret = (i < 0 ? i : len2);
  done:
 	kfree(tbuf);
+	usbfs_decrease_memory_usage(len1 + sizeof(struct urb));
 	return ret;
 }
 
@@ -1088,14 +1122,14 @@ static int proc_do_submiturb(struct dev_
 	}
 	if (!ep)
 		return -ENOENT;
+
+	u = 0;
 	switch(uurb->type) {
 	case USBDEVFS_URB_TYPE_CONTROL:
 		if (!usb_endpoint_xfer_control(&ep->desc))
 			return -EINVAL;
-		/* min 8 byte setup packet,
-		 * max 8 byte setup plus an arbitrary data stage */
-		if (uurb->buffer_length < 8 ||
-		    uurb->buffer_length > (8 + MAX_USBFS_BUFFER_SIZE))
+		/* min 8 byte setup packet */
+		if (uurb->buffer_length < 8)
 			return -EINVAL;
 		dr = kmalloc(sizeof(struct usb_ctrlrequest), GFP_KERNEL);
 		if (!dr)
@@ -1129,6 +1163,7 @@ static int proc_do_submiturb(struct dev_
 			__le16_to_cpup(&dr->wValue),
 			__le16_to_cpup(&dr->wIndex),
 			__le16_to_cpup(&dr->wLength));
+		u = sizeof(struct usb_ctrlrequest);
 		break;
 
 	case USBDEVFS_URB_TYPE_BULK:
@@ -1142,8 +1177,6 @@ static int proc_do_submiturb(struct dev_
 			goto interrupt_urb;
 		}
 		uurb->number_of_packets = 0;
-		if (uurb->buffer_length > MAX_USBFS_BUFFER_SIZE)
-			return -EINVAL;
 		break;
 
 	case USBDEVFS_URB_TYPE_INTERRUPT:
@@ -1151,8 +1184,6 @@ static int proc_do_submiturb(struct dev_
 			return -EINVAL;
  interrupt_urb:
 		uurb->number_of_packets = 0;
-		if (uurb->buffer_length > MAX_USBFS_BUFFER_SIZE)
-			return -EINVAL;
 		break;
 
 	case USBDEVFS_URB_TYPE_ISO:
@@ -1179,17 +1210,18 @@ static int proc_do_submiturb(struct dev_
 			}
 			totlen += isopkt[u].length;
 		}
-		/* 3072 * 64 microframes */
-		if (totlen > 196608) {
-			ret = -EINVAL;
-			goto error;
-		}
+		u *= sizeof(struct usb_iso_packet_descriptor);
 		uurb->buffer_length = totlen;
 		break;
 
 	default:
 		return -EINVAL;
 	}
+
+	if (uurb->buffer_length > MAX_USBFS_MEMORY_USAGE) {
+		ret = -EINVAL;
+		goto error;
+	}
 	if (uurb->buffer_length > 0 &&
 			!access_ok(is_in ? VERIFY_WRITE : VERIFY_READ,
 				uurb->buffer, uurb->buffer_length)) {
@@ -1201,6 +1233,12 @@ static int proc_do_submiturb(struct dev_
 		ret = -ENOMEM;
 		goto error;
 	}
+	u += sizeof(struct async) + sizeof(struct urb) + uurb->buffer_length;
+	ret = usbfs_increase_memory_usage(u);
+	if (ret)
+		goto error;
+	as->mem_usage = u;
+
 	if (uurb->buffer_length > 0) {
 		as->urb->transfer_buffer = kmalloc(uurb->buffer_length,
 				GFP_KERNEL);

Re: [Patch] Increase USBFS Bulk Transfer size

[Markus Rechberger]

On Mon, Oct 17, 2011 at 8:38 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> Markus:
>
> Attached is a sequence of two patches for you to try out.  The second
> is meant to be applied on top of the first.  I based them on 3.1-rc4
> (more or less) so they may need a little tweaking for your kernel, but
> hopefully not very much.
>
> The first patch simplifies the error pathways for the USB submission
> routines in usbfs.  The second patch removes the limits on the sizes of
> individual URBs and replaces them with a global limit on the total
> amount of memory allocated by usbfs.
>
> They work okay on my system but I don't have any good way to give them
> a thorough test.  Please try them out and see if they allow your webcam
> programs to work properly.
>

will go through it within the next 1-2 days, just a bit busy with
other things now..

thanks,
Markus
> Alan Stern
>

Re: [Patch] Increase USBFS Bulk Transfer size

[Sarah Sharp]

On Fri, Oct 14, 2011 at 08:33:29AM -0600, Greg KH wrote:
> On Fri, Oct 14, 2011 at 10:05:41AM -0400, Alan Stern wrote:
> > No, a much better approach is to remove all limits on individual
> > transfer sizes and instead have a global limit on the total amount of
> > all usbfs buffers in use at any time.  Maybe something like 16 MB; at 
> > SuperSpeed, that's about about 30 ms worth of data.
> 
> That sounds quite reasonable.

Alan, won't this global limit on the usbfs URB buffer size effect
userspace drivers that are currently allocating large amounts of
buffers, but still respecting individual buffer limit of 16KB?  It seems
like the patch has the potential to break userspace drivers.

I think that Point Grey's USB 3.0 webcam will be attempting to queue a
series of bulk URBs that will be bigger than your 16MB global limit.

Tim, what is the total size of buffers that will be in flight at any one
time for your device?

Sarah Sharp

Re: [Patch] Increase USBFS Bulk Transfer size

[Alan Stern]

On Mon, 7 Nov 2011, Sarah Sharp wrote:

> On Fri, Oct 14, 2011 at 08:33:29AM -0600, Greg KH wrote:
> > On Fri, Oct 14, 2011 at 10:05:41AM -0400, Alan Stern wrote:
> > > No, a much better approach is to remove all limits on individual
> > > transfer sizes and instead have a global limit on the total amount of
> > > all usbfs buffers in use at any time.  Maybe something like 16 MB; at 
> > > SuperSpeed, that's about about 30 ms worth of data.
> > 
> > That sounds quite reasonable.
> 
> Alan, won't this global limit on the usbfs URB buffer size effect
> userspace drivers that are currently allocating large amounts of
> buffers, but still respecting individual buffer limit of 16KB?  It seems
> like the patch has the potential to break userspace drivers.

It might indeed.  A further enhancement would replace that 16-MB global
constant with a sysfs attribute (a writable module parameter for
usbcore).  Do you have any better suggestions?

> I think that Point Grey's USB 3.0 webcam will be attempting to queue a
> series of bulk URBs that will be bigger than your 16MB global limit.

For SuperSpeed, 16 MB is rather on the low side.  For high speed it
amounts to about 1/3-second worth of data, which arguably is also a bit
low.  Increasing the default is easy enough, but the best choice isn't
obvious.

Alan Stern

RE: [Patch] Increase USBFS Bulk Transfer size

[Tim Vlaar]

Hi Sarah,

One of our cameras can produce 60MB images.  We usually queue up 10 images.  In Windows we have had customers queue up 100s of images at 2-3MB/image, but not at 60MB at a time ... yet.  That could easily amount to 60x10 = 600MB of in flight data or more.  Should there be a limit as long as there is memory available?

Thanks
Tim

-----Original Message-----
From: Sarah Sharp [mailto:sarah.a.sharp@linux.intel.com] 
Sent: Monday, November 07, 2011 10:53 AM
To: Alan Stern; Tim Vlaar
Cc: Greg KH; Markus Rechberger; Alan Cox; USB list; LKML
Subject: Re: [Patch] Increase USBFS Bulk Transfer size

On Fri, Oct 14, 2011 at 08:33:29AM -0600, Greg KH wrote:
> On Fri, Oct 14, 2011 at 10:05:41AM -0400, Alan Stern wrote:
> > No, a much better approach is to remove all limits on individual 
> > transfer sizes and instead have a global limit on the total amount 
> > of all usbfs buffers in use at any time.  Maybe something like 16 
> > MB; at SuperSpeed, that's about about 30 ms worth of data.
> 
> That sounds quite reasonable.

Alan, won't this global limit on the usbfs URB buffer size effect userspace drivers that are currently allocating large amounts of buffers, but still respecting individual buffer limit of 16KB?  It seems like the patch has the potential to break userspace drivers.

I think that Point Grey's USB 3.0 webcam will be attempting to queue a series of bulk URBs that will be bigger than your 16MB global limit.

Tim, what is the total size of buffers that will be in flight at any one time for your device?

Sarah Sharp

RE: [Patch] Increase USBFS Bulk Transfer size

[Alan Stern]

On Mon, 7 Nov 2011, Tim Vlaar wrote:

> Hi Sarah,
> 
> One of our cameras can produce 60MB images.  We usually queue up 10
> images.  In Windows we have had customers queue up 100s of images at
> 2-3MB/image, but not at 60MB at a time ... yet.  That could easily
> amount to 60x10 = 600MB of in flight data or more.  Should there be a
> limit as long as there is memory available?

This is debatable.  There are limits on how much memory a single 
process can allocate, but those limits don't apply to usbfs.  Even the 
patch Sarah referred to doesn't impose a per-process limit, but rather 
an overall global limit.

If we make the usbfs limit adjustable, one of the settings could be "no 
limit".  Then nothing would prevent you from using up more and more 
memory for your USB transfers until the machine runs out.  :-)

Alan Stern

RE: [Patch] Increase USBFS Bulk Transfer size

[Tim Vlaar]

Having a setting that we could adjust or even set to "no limit"  would work for us :)

Thanks
Tim


-----Original Message-----
From: Alan Stern [mailto:stern@rowland.harvard.edu] 
Sent: Monday, November 07, 2011 11:56 AM
To: Tim Vlaar
Cc: Sarah Sharp; Greg KH; Markus Rechberger; Alan Cox; USB list; LKML
Subject: RE: [Patch] Increase USBFS Bulk Transfer size

On Mon, 7 Nov 2011, Tim Vlaar wrote:

> Hi Sarah,
> 
> One of our cameras can produce 60MB images.  We usually queue up 10 
> images.  In Windows we have had customers queue up 100s of images at 
> 2-3MB/image, but not at 60MB at a time ... yet.  That could easily 
> amount to 60x10 = 600MB of in flight data or more.  Should there be a 
> limit as long as there is memory available?

This is debatable.  There are limits on how much memory a single process can allocate, but those limits don't apply to usbfs.  Even the patch Sarah referred to doesn't impose a per-process limit, but rather an overall global limit.

If we make the usbfs limit adjustable, one of the settings could be "no limit".  Then nothing would prevent you from using up more and more memory for your USB transfers until the machine runs out.  :-)

Alan Stern

Re: [Patch] Increase USBFS Bulk Transfer size

[Sarah Sharp]

On Mon, Nov 07, 2011 at 02:12:16PM -0500, Alan Stern wrote:
> On Mon, 7 Nov 2011, Sarah Sharp wrote:
> 
> > On Fri, Oct 14, 2011 at 08:33:29AM -0600, Greg KH wrote:
> > > On Fri, Oct 14, 2011 at 10:05:41AM -0400, Alan Stern wrote:
> > > > No, a much better approach is to remove all limits on individual
> > > > transfer sizes and instead have a global limit on the total amount of
> > > > all usbfs buffers in use at any time.  Maybe something like 16 MB; at 
> > > > SuperSpeed, that's about about 30 ms worth of data.
> > > 
> > > That sounds quite reasonable.
> > 
> > Alan, won't this global limit on the usbfs URB buffer size effect
> > userspace drivers that are currently allocating large amounts of
> > buffers, but still respecting individual buffer limit of 16KB?  It seems
> > like the patch has the potential to break userspace drivers.
> 
> It might indeed.  A further enhancement would replace that 16-MB global
> constant with a sysfs attribute (a writable module parameter for
> usbcore).  Do you have any better suggestions?

No, I don't have any better suggestions, except take out the limit. ;)

I do understand why we don't want userspace to DoS the system by using
up too much DMA'able memory.  However, as I understand it, the usbfs
files are created by udev with root access only by default, and distros
may choose to install rules that have more permissive privileges.  A
device vendor may not be ensured that a udev rule with permissive access
will be present for their device, so I think they're likely to write
programs that require root access.  Or require root privileges to
install said udev rule.

At that point, the same userspace program that has root privileges in
order to access usbfs or create the udev rule can just load and unload
the usbcore module with an arbitrarily large global limit, and the
global limit doesn't really add any security.  So why add the extra
barrier?

> > I think that Point Grey's USB 3.0 webcam will be attempting to queue a
> > series of bulk URBs that will be bigger than your 16MB global limit.
> 
> For SuperSpeed, 16 MB is rather on the low side.  For high speed it
> amounts to about 1/3-second worth of data, which arguably is also a bit
> low.  Increasing the default is easy enough, but the best choice isn't
> obvious.

Yeah, the choice is not obvious and we're probably going to get it
wrong, but as Tim said, he does need ~600MB in flight at once, so I knew
16MB was too small.  I guess the question really should be not "What is
the smallest limit we need?" but "When will the system start breaking
down due to memory pressure?" and set the limit somewhere pretty close
to there.

Do other subsystems have these issues as well?  Does the layer SCSI ever
limit the number of outstanding READ requests (aside from hardware
limitations)?  Or does the networking layer have a limit to the buffers
it keeps pending transfers for userspace to read?

Sarah Sharp

RE: [Patch] Increase USBFS Bulk Transfer size

[Brink, Peter]

Beyond hardware, I cannot tell you if there is a limitation, but both the SATA and SAS protocol layers have a command buffering system called NCQ (Native Command Queuing) for SATA or CQ (Command Queuing) for SAS.  Each of these is related specifically to the drive itself, and, from my observation, the file system tracks to the number of commands that the drive will support.

There are also queues for each set of commands that will direct to different drives, but in practice, even in high stress environments, the file system was hard pressed to keep up with the command-processing of the individual drives, depending, of course, on the size of the reads. This was true on both Windows and Linux and across RAIDed systems.

Pete

-----Original Message-----
From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-owner@vger.kernel.org] On Behalf Of Sarah Sharp
Sent: Monday, November 07, 2011 1:18 PM
To: Alan Stern
Cc: Tim Vlaar; Greg KH; Markus Rechberger; Alan Cox; USB list; LKML
Subject: Re: [Patch] Increase USBFS Bulk Transfer size

On Mon, Nov 07, 2011 at 02:12:16PM -0500, Alan Stern wrote:
> On Mon, 7 Nov 2011, Sarah Sharp wrote:
> 
> > On Fri, Oct 14, 2011 at 08:33:29AM -0600, Greg KH wrote:
> > > On Fri, Oct 14, 2011 at 10:05:41AM -0400, Alan Stern wrote:
> > > > No, a much better approach is to remove all limits on individual
> > > > transfer sizes and instead have a global limit on the total amount of
> > > > all usbfs buffers in use at any time.  Maybe something like 16 MB; at 
> > > > SuperSpeed, that's about about 30 ms worth of data.
> > > 
> > > That sounds quite reasonable.
> > 
> > Alan, won't this global limit on the usbfs URB buffer size effect
> > userspace drivers that are currently allocating large amounts of
> > buffers, but still respecting individual buffer limit of 16KB?  It seems
> > like the patch has the potential to break userspace drivers.
> 
> It might indeed.  A further enhancement would replace that 16-MB global
> constant with a sysfs attribute (a writable module parameter for
> usbcore).  Do you have any better suggestions?

No, I don't have any better suggestions, except take out the limit. ;)

I do understand why we don't want userspace to DoS the system by using
up too much DMA'able memory.  However, as I understand it, the usbfs
files are created by udev with root access only by default, and distros
may choose to install rules that have more permissive privileges.  A
device vendor may not be ensured that a udev rule with permissive access
will be present for their device, so I think they're likely to write
programs that require root access.  Or require root privileges to
install said udev rule.

At that point, the same userspace program that has root privileges in
order to access usbfs or create the udev rule can just load and unload
the usbcore module with an arbitrarily large global limit, and the
global limit doesn't really add any security.  So why add the extra
barrier?

> > I think that Point Grey's USB 3.0 webcam will be attempting to queue a
> > series of bulk URBs that will be bigger than your 16MB global limit.
> 
> For SuperSpeed, 16 MB is rather on the low side.  For high speed it
> amounts to about 1/3-second worth of data, which arguably is also a bit
> low.  Increasing the default is easy enough, but the best choice isn't
> obvious.

Yeah, the choice is not obvious and we're probably going to get it
wrong, but as Tim said, he does need ~600MB in flight at once, so I knew
16MB was too small.  I guess the question really should be not "What is
the smallest limit we need?" but "When will the system start breaking
down due to memory pressure?" and set the limit somewhere pretty close
to there.

Do other subsystems have these issues as well?  Does the layer SCSI ever
limit the number of outstanding READ requests (aside from hardware
limitations)?  Or does the networking layer have a limit to the buffers
it keeps pending transfers for userspace to read?

Sarah Sharp
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Patch] Increase USBFS Bulk Transfer size

[Alan Stern]

On Mon, 7 Nov 2011, Sarah Sharp wrote:

> > > Alan, won't this global limit on the usbfs URB buffer size effect
> > > userspace drivers that are currently allocating large amounts of
> > > buffers, but still respecting individual buffer limit of 16KB?  It seems
> > > like the patch has the potential to break userspace drivers.
> > 
> > It might indeed.  A further enhancement would replace that 16-MB global
> > constant with a sysfs attribute (a writable module parameter for
> > usbcore).  Do you have any better suggestions?
> 
> No, I don't have any better suggestions, except take out the limit. ;)
> 
> I do understand why we don't want userspace to DoS the system by using
> up too much DMA'able memory.  However, as I understand it, the usbfs
> files are created by udev with root access only by default, and distros
> may choose to install rules that have more permissive privileges.  A
> device vendor may not be ensured that a udev rule with permissive access
> will be present for their device, so I think they're likely to write
> programs that require root access.  Or require root privileges to
> install said udev rule.
> 
> At that point, the same userspace program that has root privileges in
> order to access usbfs or create the udev rule can just load and unload
> the usbcore module with an arbitrarily large global limit, and the
> global limit doesn't really add any security.  So why add the extra
> barrier?

This is a question of kernel policy, and I don't know what is the
generally accepted approach to this sort of thing.  Maybe Greg or Alan
Cox can comment.


> > > I think that Point Grey's USB 3.0 webcam will be attempting to queue a
> > > series of bulk URBs that will be bigger than your 16MB global limit.
> > 
> > For SuperSpeed, 16 MB is rather on the low side.  For high speed it
> > amounts to about 1/3-second worth of data, which arguably is also a bit
> > low.  Increasing the default is easy enough, but the best choice isn't
> > obvious.
> 
> Yeah, the choice is not obvious and we're probably going to get it
> wrong, but as Tim said, he does need ~600MB in flight at once, so I knew
> 16MB was too small.  I guess the question really should be not "What is
> the smallest limit we need?" but "When will the system start breaking
> down due to memory pressure?" and set the limit somewhere pretty close
> to there.

It might not be so easy to identify that value.  I wouldn't know how to
do it.

> Do other subsystems have these issues as well?  Does the layer SCSI ever
> limit the number of outstanding READ requests (aside from hardware
> limitations)?

Not as far as I know.  Perhaps the block layer tries to slow things 
down if too many I/O operations are pending (or maybe not -- I'm not 
at all familiar with the details), but that's different from returning 
an error.

>  Or does the networking layer have a limit to the buffers
> it keeps pending transfers for userspace to read?

Again, I don't know.  Those subsystems are a lot more complicated than
usbfs, and they probably have arrangements to allocate intermediate
buffers a piece at a time.  We could do something like that, but the
end result would be the same as our current limit on URB sizes -- the
only difference being that transfers would be split into multiple URBs
by the usbfs driver instead of by the user program.

In fact, it's not all that easy for a program to generate many I/O 
requests concurrently.  The old async I/O mechanism is one way, and you 
spent a lot of time working on it.  Do you remember if it had any 
limits?

Alan Stern

Re: [Patch] Increase USBFS Bulk Transfer size

[Greg KH]

On Mon, Nov 07, 2011 at 03:53:59PM -0500, Alan Stern wrote:
> On Mon, 7 Nov 2011, Sarah Sharp wrote:
> 
> > > > Alan, won't this global limit on the usbfs URB buffer size effect
> > > > userspace drivers that are currently allocating large amounts of
> > > > buffers, but still respecting individual buffer limit of 16KB?  It seems
> > > > like the patch has the potential to break userspace drivers.
> > > 
> > > It might indeed.  A further enhancement would replace that 16-MB global
> > > constant with a sysfs attribute (a writable module parameter for
> > > usbcore).  Do you have any better suggestions?
> > 
> > No, I don't have any better suggestions, except take out the limit. ;)
> > 
> > I do understand why we don't want userspace to DoS the system by using
> > up too much DMA'able memory.  However, as I understand it, the usbfs
> > files are created by udev with root access only by default, and distros
> > may choose to install rules that have more permissive privileges.  A
> > device vendor may not be ensured that a udev rule with permissive access
> > will be present for their device, so I think they're likely to write
> > programs that require root access.  Or require root privileges to
> > install said udev rule.
> > 
> > At that point, the same userspace program that has root privileges in
> > order to access usbfs or create the udev rule can just load and unload
> > the usbcore module with an arbitrarily large global limit, and the
> > global limit doesn't really add any security.  So why add the extra
> > barrier?
> 
> This is a question of kernel policy, and I don't know what is the
> generally accepted approach to this sort of thing.  Maybe Greg or Alan
> Cox can comment.

You have to set the limit to something, otherwise a non-root user can
kill the box quite easily.  If the admin wants to set the system up so
that any user can use up all of the memory, they are free to do that,
but we can't have it be the default.

thanks,

greg k-h

Re: [Patch] Increase USBFS Bulk Transfer size

[Sarah Sharp]

(Question about usbfs and aio limits at the bottom.)

On Mon, Nov 07, 2011 at 03:53:59PM -0500, Alan Stern wrote:
> On Mon, 7 Nov 2011, Sarah Sharp wrote:
> 
> > > > Alan, won't this global limit on the usbfs URB buffer size effect
> > > > userspace drivers that are currently allocating large amounts of
> > > > buffers, but still respecting individual buffer limit of 16KB?  It seems
> > > > like the patch has the potential to break userspace drivers.
> > > 
> > > It might indeed.  A further enhancement would replace that 16-MB global
> > > constant with a sysfs attribute (a writable module parameter for
> > > usbcore).  Do you have any better suggestions?
> > 
> > No, I don't have any better suggestions, except take out the limit. ;)
> > 
> > I do understand why we don't want userspace to DoS the system by using
> > up too much DMA'able memory.  However, as I understand it, the usbfs
> > files are created by udev with root access only by default, and distros
> > may choose to install rules that have more permissive privileges.  A
> > device vendor may not be ensured that a udev rule with permissive access
> > will be present for their device, so I think they're likely to write
> > programs that require root access.  Or require root privileges to
> > install said udev rule.
> > 
> > At that point, the same userspace program that has root privileges in
> > order to access usbfs or create the udev rule can just load and unload
> > the usbcore module with an arbitrarily large global limit, and the
> > global limit doesn't really add any security.  So why add the extra
> > barrier?
> 
> This is a question of kernel policy, and I don't know what is the
> generally accepted approach to this sort of thing.  Maybe Greg or Alan
> Cox can comment.

Ok, after thinking about it, Greg is right that we do need to do
something for the case where any userspace program can write to a usbfs
file.

> > > > I think that Point Grey's USB 3.0 webcam will be attempting to queue a
> > > > series of bulk URBs that will be bigger than your 16MB global limit.
> > > 
> > > For SuperSpeed, 16 MB is rather on the low side.  For high speed it
> > > amounts to about 1/3-second worth of data, which arguably is also a bit
> > > low.  Increasing the default is easy enough, but the best choice isn't
> > > obvious.
> > 
> > Yeah, the choice is not obvious and we're probably going to get it
> > wrong, but as Tim said, he does need ~600MB in flight at once, so I knew
> > 16MB was too small.  I guess the question really should be not "What is
> > the smallest limit we need?" but "When will the system start breaking
> > down due to memory pressure?" and set the limit somewhere pretty close
> > to there.
> 
> It might not be so easy to identify that value.  I wouldn't know how to
> do it.

I wouldn't know how to test it either, and I suspect it would be
system-specific.  Are you at least OK with setting the limit to 600MB so
that Tim's userspace driver will work by default?  I think we still need
the modparam for the usb core to set the limit as well.

> > Do other subsystems have these issues as well?  Does the layer SCSI ever
> > limit the number of outstanding READ requests (aside from hardware
> > limitations)?
> 
> Not as far as I know.  Perhaps the block layer tries to slow things 
> down if too many I/O operations are pending (or maybe not -- I'm not 
> at all familiar with the details), but that's different from returning 
> an error.
> 
> >  Or does the networking layer have a limit to the buffers
> > it keeps pending transfers for userspace to read?
> 
> Again, I don't know.  Those subsystems are a lot more complicated than
> usbfs, and they probably have arrangements to allocate intermediate
> buffers a piece at a time.  We could do something like that, but the
> end result would be the same as our current limit on URB sizes -- the
> only difference being that transfers would be split into multiple URBs
> by the usbfs driver instead of by the user program.

Or splitting the transfer into multiple scatter-gather list entries,
right?  There's no need to submit multiple URBs if you can just use
sglists instead.  If the host can't handle sglists, the core can just
split it up into multiple URBs.  I'd like to push the biggest data
chucks we can as far down in the stack as possible to avoid performance
hits from completing many URBs.  But that's a separate issue...

> In fact, it's not all that easy for a program to generate many I/O 
> requests concurrently.  The old async I/O mechanism is one way, and you 
> spent a lot of time working on it.  Do you remember if it had any 
> limits?

I haven't looked at that in about four years, so if it did I don't
remember it.  Maybe someone from the aio list knows?

Sarah Sharp

Re: [Patch] Increase USBFS Bulk Transfer size

[Alan Stern]

On Mon, 7 Nov 2011, Sarah Sharp wrote:

> > > Yeah, the choice is not obvious and we're probably going to get it
> > > wrong, but as Tim said, he does need ~600MB in flight at once, so I knew
> > > 16MB was too small.  I guess the question really should be not "What is
> > > the smallest limit we need?" but "When will the system start breaking
> > > down due to memory pressure?" and set the limit somewhere pretty close
> > > to there.
> > 
> > It might not be so easy to identify that value.  I wouldn't know how to
> > do it.
> 
> I wouldn't know how to test it either, and I suspect it would be
> system-specific.  Are you at least OK with setting the limit to 600MB so
> that Tim's userspace driver will work by default?  I think we still need
> the modparam for the usb core to set the limit as well.

I don't know; 600 MB sounds kind of high for some systems.  It's
entirely possible to run Linux on a machine with < 1 GB of RAM.  I'd
feel a lot safer making the default value small, like 16 MB, and asking
Tim's clients to increase it.

> > Again, I don't know.  Those subsystems are a lot more complicated than
> > usbfs, and they probably have arrangements to allocate intermediate
> > buffers a piece at a time.  We could do something like that, but the
> > end result would be the same as our current limit on URB sizes -- the
> > only difference being that transfers would be split into multiple URBs
> > by the usbfs driver instead of by the user program.
> 
> Or splitting the transfer into multiple scatter-gather list entries,
> right?

Not really, because the problem is to avoid loading all the buffers
into kernel space at the same time.  That means using multiple URBs --
although each URB could use SG to handle more than 16 KB at once.

>  There's no need to submit multiple URBs if you can just use
> sglists instead.  If the host can't handle sglists, the core can just
> split it up into multiple URBs.  I'd like to push the biggest data
> chucks we can as far down in the stack as possible to avoid performance
> hits from completing many URBs.  But that's a separate issue...

Right.  For example, usbfs could support an iov sort of interface.  
That would translate naturally into an SG list.

> > In fact, it's not all that easy for a program to generate many I/O 
> > requests concurrently.  The old async I/O mechanism is one way, and you 
> > spent a lot of time working on it.  Do you remember if it had any 
> > limits?
> 
> I haven't looked at that in about four years, so if it did I don't
> remember it.  Maybe someone from the aio list knows?

One point that hasn't been emphasized very strongly (although it was
mentioned once) is that our _current_ kernel allows programs to submit
many USB transfers and thus eat up all available kernel memory.  That
definitely could be considered a bug.

Alan Stern