All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Increase usbfs bulk buffer size
@ 2011-08-17 17:07 Markus Rechberger
  2011-08-17 17:14 ` Oliver Neukum
  2011-08-18 18:37 ` Greg KH
  0 siblings, 2 replies; 14+ messages in thread
From: Markus Rechberger @ 2011-08-17 17:07 UTC (permalink / raw)
  To: Greg Kroah-Hartman, linux-usb, linux-kernel

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

Hi,

this patch increases the maximum buffersize for bulk transfers, our
devices support at least up to 46k bytes for bulk transfers.
This patch allows us to lower the iterations between kernel and
userspace and lower the system pressure.

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

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 37518df..ad193d0 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -107,7 +107,7 @@ enum snoop_when {

 #define USB_DEVICE_DEV         MKDEV(USB_DEVICE_MAJOR, 0)

-#define        MAX_USBFS_BUFFER_SIZE   16384
+#define        MAX_USBFS_BUFFER_SIZE   46080


 static int connected(struct dev_state *ps)

[-- Attachment #2: bulk_patch.diff --]
[-- Type: text/x-patch, Size: 373 bytes --]

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 37518df..ad193d0 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -107,7 +107,7 @@ enum snoop_when {
 
 #define USB_DEVICE_DEV		MKDEV(USB_DEVICE_MAJOR, 0)
 
-#define	MAX_USBFS_BUFFER_SIZE	16384
+#define	MAX_USBFS_BUFFER_SIZE	46080
 
 
 static int connected(struct dev_state *ps)

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] Increase usbfs bulk buffer size
  2011-08-17 17:07 [PATCH] Increase usbfs bulk buffer size Markus Rechberger
@ 2011-08-17 17:14 ` Oliver Neukum
  2011-08-17 17:44   ` Markus Rechberger
  2011-08-18 18:37 ` Greg KH
  1 sibling, 1 reply; 14+ messages in thread
From: Oliver Neukum @ 2011-08-17 17:14 UTC (permalink / raw)
  To: Markus Rechberger; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel

Am Mittwoch, 17. August 2011, 19:07:01 schrieb Markus Rechberger:
> Hi,
> 
> this patch increases the maximum buffersize for bulk transfers, our
> devices support at least up to 46k bytes for bulk transfers.
> This patch allows us to lower the iterations between kernel and
> userspace and lower the system pressure.

You must not do this. It increases memory pressure too much
by forcing very high order allocations. If your problem is really
the number of syscalls, you must implement scatter/gather.

	Regards
		Ôliver

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Increase usbfs bulk buffer size
  2011-08-17 17:14 ` Oliver Neukum
@ 2011-08-17 17:44   ` Markus Rechberger
  2011-08-19  6:23     ` Oliver Neukum
  0 siblings, 1 reply; 14+ messages in thread
From: Markus Rechberger @ 2011-08-17 17:44 UTC (permalink / raw)
  To: Oliver Neukum; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel

Hi,

On Wed, Aug 17, 2011 at 7:14 PM, Oliver Neukum <oliver@neukum.org> wrote:
> Am Mittwoch, 17. August 2011, 19:07:01 schrieb Markus Rechberger:
>> Hi,
>>
>> this patch increases the maximum buffersize for bulk transfers, our
>> devices support at least up to 46k bytes for bulk transfers.
>> This patch allows us to lower the iterations between kernel and
>> userspace and lower the system pressure.
>
> You must not do this. It increases memory pressure too much
> by forcing very high order allocations. If your problem is really
> the number of syscalls, you must implement scatter/gather.
>

I understand that USBFS is not 100% optimized, although we are already
using 3-4 times that much buffer with isochronous and it was working
fine for years so far as long as the system doesn't run out of memory
.. but even kerneldrivers would have issues with it in such a
situation.

An example analog video frames 170 mbit per second
~3000 bytes * 64 microframes = 192000

The bulk buffers we are asking for are for encoded videostreams, which
is much smaller than analog TV frames.
So for bulk we are just asking for around 1/4 of that buffersize.
Our application works fine with USBFS in Isochronous mode, however
some embedded systems don't support Isochronous so we are switching to
Bulk. I submitted the Isochronous patch for accepting bigger buffers
around 1-2 years ago the allocations and iterations went down from 600
to around 125 per second. For the enduser exerience since that time
it's possible to start heavy IO/memory applications without hurting
the video process.

BR,
Markus

>        Regards
>                Ôliver
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Increase usbfs bulk buffer size
  2011-08-17 17:07 [PATCH] Increase usbfs bulk buffer size Markus Rechberger
  2011-08-17 17:14 ` Oliver Neukum
@ 2011-08-18 18:37 ` Greg KH
  2011-08-18 19:13   ` Markus Rechberger
  1 sibling, 1 reply; 14+ messages in thread
From: Greg KH @ 2011-08-18 18:37 UTC (permalink / raw)
  To: Markus Rechberger; +Cc: linux-usb, linux-kernel

On Wed, Aug 17, 2011 at 07:07:01PM +0200, Markus Rechberger wrote:
> Hi,
> 
> this patch increases the maximum buffersize for bulk transfers, our
> devices support at least up to 46k bytes for bulk transfers.
> This patch allows us to lower the iterations between kernel and
> userspace and lower the system pressure.

But as Oliver pointed out, this increases the memory pressure.  I'm
curious why you say there is "system pressure" with the current buffer
size.  What is the problems here, you are just passing the same logic
issues down to the kernel, that you should be handling in userspace if
you want to use usbfs.

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

Also note that this is a user/kernel API that you are changing, are you
_sure_ you didn't just break some user code with this change that was
assuming that the buffer size was 16Kb, as it's been that way for the
past 10 years?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Increase usbfs bulk buffer size
  2011-08-18 18:37 ` Greg KH
@ 2011-08-18 19:13   ` Markus Rechberger
  2011-08-19  5:26     ` Greg KH
  0 siblings, 1 reply; 14+ messages in thread
From: Markus Rechberger @ 2011-08-18 19:13 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-usb, linux-kernel

On Thu, Aug 18, 2011 at 8:37 PM, Greg KH <gregkh@suse.de> wrote:
> On Wed, Aug 17, 2011 at 07:07:01PM +0200, Markus Rechberger wrote:
>> Hi,
>>
>> this patch increases the maximum buffersize for bulk transfers, our
>> devices support at least up to 46k bytes for bulk transfers.
>> This patch allows us to lower the iterations between kernel and
>> userspace and lower the system pressure.
>
> But as Oliver pointed out, this increases the memory pressure.  I'm
> curious why you say there is "system pressure" with the current buffer
> size.  What is the problems here, you are just passing the same logic
> issues down to the kernel, that you should be handling in userspace if
> you want to use usbfs.
>

userspace applications are not realtime applications, the bigger the buffer
the less buffers will be required.
As noted this is just 1/4th the buffersize we are already using with
Isochronous.
Back then with small isochronous buffers even moving the mozilla GUI
window could
cause incomplete data (and I'm talking about 170mbit per second
transfers here), with increased buffers this issue is gone (and it's
gone
since I submitted a similar patch for isochronous ~1-2 years ago).
In case of bulk allocations can go down for 2/3 of the current
pressure, of course it goes up on another side but that side has a
better performance.

>> Signed-off-by: Markus Rechberger <mrechberger@gmail.com>
>
> Also note that this is a user/kernel API that you are changing, are you
> _sure_ you didn't just break some user code with this change that was
> assuming that the buffer size was 16Kb, as it's been that way for the
> past 10 years?
>

just the check about the maximum allowed buffer size will be
increased, nothing else.
Existing applications will still go for 16k.

BR,
Markus


> thanks,
>
> greg k-h
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Increase usbfs bulk buffer size
  2011-08-18 19:13   ` Markus Rechberger
@ 2011-08-19  5:26     ` Greg KH
  0 siblings, 0 replies; 14+ messages in thread
From: Greg KH @ 2011-08-19  5:26 UTC (permalink / raw)
  To: Markus Rechberger; +Cc: linux-usb, linux-kernel

On Thu, Aug 18, 2011 at 09:13:04PM +0200, Markus Rechberger wrote:
> On Thu, Aug 18, 2011 at 8:37 PM, Greg KH <gregkh@suse.de> wrote:
> > On Wed, Aug 17, 2011 at 07:07:01PM +0200, Markus Rechberger wrote:
> >> Hi,
> >>
> >> this patch increases the maximum buffersize for bulk transfers, our
> >> devices support at least up to 46k bytes for bulk transfers.
> >> This patch allows us to lower the iterations between kernel and
> >> userspace and lower the system pressure.
> >
> > But as Oliver pointed out, this increases the memory pressure.  I'm
> > curious why you say there is "system pressure" with the current buffer
> > size.  What is the problems here, you are just passing the same logic
> > issues down to the kernel, that you should be handling in userspace if
> > you want to use usbfs.
> >
> 
> userspace applications are not realtime applications, the bigger the buffer
> the less buffers will be required.

In userspace, yes, but it's not "free", you still end up allocating the
same amount of memory in the kernel instead.  And larger memory
allocations like this, provide more memory pressure on the system
overall, which can cause problems.

> As noted this is just 1/4th the buffersize we are already using with
> Isochronous.  Back then with small isochronous buffers even moving the
> mozilla GUI window could cause incomplete data (and I'm talking about
> 170mbit per second transfers here), with increased buffers this issue
> is gone (and it's gone since I submitted a similar patch for
> isochronous ~1-2 years ago).  In case of bulk allocations can go down
> for 2/3 of the current pressure, of course it goes up on another side
> but that side has a better performance.

Do you have real numbers and example programs that show the difference
of this kernel change making a real difference?

> >> Signed-off-by: Markus Rechberger <mrechberger@gmail.com>
> >
> > Also note that this is a user/kernel API that you are changing, are you
> > _sure_ you didn't just break some user code with this change that was
> > assuming that the buffer size was 16Kb, as it's been that way for the
> > past 10 years?
> >
> 
> just the check about the maximum allowed buffer size will be
> increased, nothing else.
> Existing applications will still go for 16k.

Yes, but you just changed the value the kernel can accept, which could
cause problems.

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Increase usbfs bulk buffer size
  2011-08-17 17:44   ` Markus Rechberger
@ 2011-08-19  6:23     ` Oliver Neukum
  2011-08-19 10:45       ` Markus Rechberger
  0 siblings, 1 reply; 14+ messages in thread
From: Oliver Neukum @ 2011-08-19  6:23 UTC (permalink / raw)
  To: Markus Rechberger; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel

Am Mittwoch, 17. August 2011, 19:44:16 schrieb Markus Rechberger:
> I understand that USBFS is not 100% optimized, although we are already
> using 3-4 times that much buffer with isochronous and it was working
> fine for years so far as long as the system doesn't run out of memory
> .. but even kerneldrivers would have issues with it in such a
> situation.

The problem is not what happens on your test system dedicated to this use.
There it'll work. But you allowed everybody else to make the kernel request
the second largest chunk of memory there is. On a otherwise busy system
with fragmented memory this is not good.

As I said, you want scatter/gather if you need this.
There still would be the problem with the API, but if this is really a problem,
you could define a new ioctl()

	Regards
		Oliver

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Increase usbfs bulk buffer size
  2011-08-19  6:23     ` Oliver Neukum
@ 2011-08-19 10:45       ` Markus Rechberger
  2011-08-31  6:57         ` Markus Rechberger
  0 siblings, 1 reply; 14+ messages in thread
From: Markus Rechberger @ 2011-08-19 10:45 UTC (permalink / raw)
  To: Oliver Neukum; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel

On Fri, Aug 19, 2011 at 8:23 AM, Oliver Neukum <oliver@neukum.org> wrote:
> Am Mittwoch, 17. August 2011, 19:44:16 schrieb Markus Rechberger:
>> I understand that USBFS is not 100% optimized, although we are already
>> using 3-4 times that much buffer with isochronous and it was working
>> fine for years so far as long as the system doesn't run out of memory
>> .. but even kerneldrivers would have issues with it in such a
>> situation.
>
> The problem is not what happens on your test system dedicated to this use.
> There it'll work. But you allowed everybody else to make the kernel request
> the second largest chunk of memory there is. On a otherwise busy system
> with fragmented memory this is not good.
>
> As I said, you want scatter/gather if you need this.
> There still would be the problem with the API, but if this is really a problem,
> you could define a new ioctl()
>

let's say there are a few thousand users of the 190k isochronous implementation.
I don't want to add more complex code to the kernel, if it's not
accepted I'm okay with it,
since I know the performance impact I can recommend it to those who
are interested in it.
Our software works without it anyway, and Bulk is just for special use
in our case.

Best Regards,
Markus

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Increase usbfs bulk buffer size
  2011-08-19 10:45       ` Markus Rechberger
@ 2011-08-31  6:57         ` Markus Rechberger
  2011-09-12 10:18           ` Markus Rechberger
  0 siblings, 1 reply; 14+ messages in thread
From: Markus Rechberger @ 2011-08-31  6:57 UTC (permalink / raw)
  To: Oliver Neukum; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel

Greg, Oliver,

can you justify why 190K is acceptable for Isochronous (which has been
in the kernel for a long long time now) but less than 50k is suddenly
not acceptable for bulk when using USBFS? Especially while the wild
out there already approved that 190K is okay even on embedded systems?
That's the only thing I would like to know :-)

We made many tests here (and have many more customers using Isoc 190k
buffers) and memory allocation has a much better performance instead
of pushing ioctls forward/backward for reading those small packets.
I do understand your concern but from my point and experience with it
(we've been using it for 3 years so far) it does not cause any
problem. We reached the 1k amount of users using 190k Isoc buffers a
long time ago, those are pushing 170 Mbit a second. The bulk purpose
is for 80mbit a second only.
I would just like to have all supported modes with performance and not
too many changes. Currently we only get good performance with
isochronous, bulk USBFS performance could just be better with Linux,
our software works well enough with Mac, Solaris and FreeBSD with Bulk
and Iso.

BR,
Markus

On Fri, Aug 19, 2011 at 12:45 PM, Markus Rechberger
<mrechberger@gmail.com> wrote:
> On Fri, Aug 19, 2011 at 8:23 AM, Oliver Neukum <oliver@neukum.org> wrote:
>> Am Mittwoch, 17. August 2011, 19:44:16 schrieb Markus Rechberger:
>>> I understand that USBFS is not 100% optimized, although we are already
>>> using 3-4 times that much buffer with isochronous and it was working
>>> fine for years so far as long as the system doesn't run out of memory
>>> .. but even kerneldrivers would have issues with it in such a
>>> situation.
>>
>> The problem is not what happens on your test system dedicated to this use.
>> There it'll work. But you allowed everybody else to make the kernel request
>> the second largest chunk of memory there is. On a otherwise busy system
>> with fragmented memory this is not good.
>>
>> As I said, you want scatter/gather if you need this.
>> There still would be the problem with the API, but if this is really a problem,
>> you could define a new ioctl()
>>
>
> let's say there are a few thousand users of the 190k isochronous implementation.
> I don't want to add more complex code to the kernel, if it's not
> accepted I'm okay with it,
> since I know the performance impact I can recommend it to those who
> are interested in it.
> Our software works without it anyway, and Bulk is just for special use
> in our case.
>
> Best Regards,
> Markus
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Increase usbfs bulk buffer size
  2011-08-31  6:57         ` Markus Rechberger
@ 2011-09-12 10:18           ` Markus Rechberger
  2011-09-12 10:25             ` Markus Rechberger
  2011-09-12 11:33             ` Greg KH
  0 siblings, 2 replies; 14+ messages in thread
From: Markus Rechberger @ 2011-09-12 10:18 UTC (permalink / raw)
  To: Oliver Neukum; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel

Hi,

I'm a little bit curious why you guys are just ignoring this. We will
do some performance tests within the next days and submit the results.
Isochronous uses way bigger allocations using kmalloc and you seem to
be okay with it, but for bulk 1/3rd of it is not okay?! It doesn't
make sense to me.
The impact of changing it from

> Do you have real numbers and example programs that show the difference
of this kernel change making a real difference?

we do have we've been providing our applications for 3 years, the
device support is directly implemented into our streaming application.
Our customers were even the first ones who recognized when USBFS was
broken in the past.
http://code.google.com/p/wl500g/source/browse/trunk/kernel/247-usb-devio-max-isoc.patch?spec=svn1907&r=1907

this patch introduced some significant performance improvement for
Isochronous transfers in the past, now it would be nice to have the
same one (note 1/3rd that max bufsize only!) for Bulk.
Our isochronous devices are already running day and night with that
maximum for a very long period of time.

Markus

On Wed, Aug 31, 2011 at 8:57 AM, Markus Rechberger
<mrechberger@gmail.com> wrote:
> Greg, Oliver,
>
> can you justify why 190K is acceptable for Isochronous (which has been
> in the kernel for a long long time now) but less than 50k is suddenly
> not acceptable for bulk when using USBFS? Especially while the wild
> out there already approved that 190K is okay even on embedded systems?
> That's the only thing I would like to know :-)
>
> We made many tests here (and have many more customers using Isoc 190k
> buffers) and memory allocation has a much better performance instead
> of pushing ioctls forward/backward for reading those small packets.
> I do understand your concern but from my point and experience with it
> (we've been using it for 3 years so far) it does not cause any
> problem. We reached the 1k amount of users using 190k Isoc buffers a
> long time ago, those are pushing 170 Mbit a second. The bulk purpose
> is for 80mbit a second only.
> I would just like to have all supported modes with performance and not
> too many changes. Currently we only get good performance with
> isochronous, bulk USBFS performance could just be better with Linux,
> our software works well enough with Mac, Solaris and FreeBSD with Bulk
> and Iso.
>
> BR,
> Markus
>
> On Fri, Aug 19, 2011 at 12:45 PM, Markus Rechberger
> <mrechberger@gmail.com> wrote:
>> On Fri, Aug 19, 2011 at 8:23 AM, Oliver Neukum <oliver@neukum.org> wrote:
>>> Am Mittwoch, 17. August 2011, 19:44:16 schrieb Markus Rechberger:
>>>> I understand that USBFS is not 100% optimized, although we are already
>>>> using 3-4 times that much buffer with isochronous and it was working
>>>> fine for years so far as long as the system doesn't run out of memory
>>>> .. but even kerneldrivers would have issues with it in such a
>>>> situation.
>>>
>>> The problem is not what happens on your test system dedicated to this use.
>>> There it'll work. But you allowed everybody else to make the kernel request
>>> the second largest chunk of memory there is. On a otherwise busy system
>>> with fragmented memory this is not good.
>>>
>>> As I said, you want scatter/gather if you need this.
>>> There still would be the problem with the API, but if this is really a problem,
>>> you could define a new ioctl()
>>>
>>
>> let's say there are a few thousand users of the 190k isochronous implementation.
>> I don't want to add more complex code to the kernel, if it's not
>> accepted I'm okay with it,
>> since I know the performance impact I can recommend it to those who
>> are interested in it.
>> Our software works without it anyway, and Bulk is just for special use
>> in our case.
>>
>> Best Regards,
>> Markus
>>
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Increase usbfs bulk buffer size
  2011-09-12 10:18           ` Markus Rechberger
@ 2011-09-12 10:25             ` Markus Rechberger
  2011-09-12 11:33             ` Greg KH
  1 sibling, 0 replies; 14+ messages in thread
From: Markus Rechberger @ 2011-09-12 10:25 UTC (permalink / raw)
  To: Oliver Neukum; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel

On Mon, Sep 12, 2011 at 12:18 PM, Markus Rechberger
<mrechberger@gmail.com> wrote:
> Hi,
>
> I'm a little bit curious why you guys are just ignoring this. We will
> do some performance tests within the next days and submit the results.
> Isochronous uses way bigger allocations using kmalloc and you seem to
> be okay with it, but for bulk 1/3rd of it is not okay?! It doesn't
> make sense to me.
> The impact of changing it from
>
>> Do you have real numbers and example programs that show the difference
> of this kernel change making a real difference?
>
> we do have we've been providing our applications for 3 years, the
> device support is directly implemented into our streaming application.
> Our customers were even the first ones who recognized when USBFS was
> broken in the past.

As a reference for the USBFS bug last year which was introduced by
suse (it's fixed already):
http://support.sundtek.com/index.php/topic,237.msg1126.html#msg1126

This is normal operation with 190k buffers, transfering 20 mbyte/sec:
http://www.sundtek.de/images/ubuntu_lucid.png

> http://code.google.com/p/wl500g/source/browse/trunk/kernel/247-usb-devio-max-isoc.patch?spec=svn1907&r=1907
>
> this patch introduced some significant performance improvement for
> Isochronous transfers in the past, now it would be nice to have the
> same one (note 1/3rd that max bufsize only!) for Bulk.
> Our isochronous devices are already running day and night with that
> maximum for a very long period of time.
>
> Markus
>
> On Wed, Aug 31, 2011 at 8:57 AM, Markus Rechberger
> <mrechberger@gmail.com> wrote:
>> Greg, Oliver,
>>
>> can you justify why 190K is acceptable for Isochronous (which has been
>> in the kernel for a long long time now) but less than 50k is suddenly
>> not acceptable for bulk when using USBFS? Especially while the wild
>> out there already approved that 190K is okay even on embedded systems?
>> That's the only thing I would like to know :-)
>>
>> We made many tests here (and have many more customers using Isoc 190k
>> buffers) and memory allocation has a much better performance instead
>> of pushing ioctls forward/backward for reading those small packets.
>> I do understand your concern but from my point and experience with it
>> (we've been using it for 3 years so far) it does not cause any
>> problem. We reached the 1k amount of users using 190k Isoc buffers a
>> long time ago, those are pushing 170 Mbit a second. The bulk purpose
>> is for 80mbit a second only.
>> I would just like to have all supported modes with performance and not
>> too many changes. Currently we only get good performance with
>> isochronous, bulk USBFS performance could just be better with Linux,
>> our software works well enough with Mac, Solaris and FreeBSD with Bulk
>> and Iso.
>>
>> BR,
>> Markus
>>
>> On Fri, Aug 19, 2011 at 12:45 PM, Markus Rechberger
>> <mrechberger@gmail.com> wrote:
>>> On Fri, Aug 19, 2011 at 8:23 AM, Oliver Neukum <oliver@neukum.org> wrote:
>>>> Am Mittwoch, 17. August 2011, 19:44:16 schrieb Markus Rechberger:
>>>>> I understand that USBFS is not 100% optimized, although we are already
>>>>> using 3-4 times that much buffer with isochronous and it was working
>>>>> fine for years so far as long as the system doesn't run out of memory
>>>>> .. but even kerneldrivers would have issues with it in such a
>>>>> situation.
>>>>
>>>> The problem is not what happens on your test system dedicated to this use.
>>>> There it'll work. But you allowed everybody else to make the kernel request
>>>> the second largest chunk of memory there is. On a otherwise busy system
>>>> with fragmented memory this is not good.
>>>>
>>>> As I said, you want scatter/gather if you need this.
>>>> There still would be the problem with the API, but if this is really a problem,
>>>> you could define a new ioctl()
>>>>
>>>
>>> let's say there are a few thousand users of the 190k isochronous implementation.
>>> I don't want to add more complex code to the kernel, if it's not
>>> accepted I'm okay with it,
>>> since I know the performance impact I can recommend it to those who
>>> are interested in it.
>>> Our software works without it anyway, and Bulk is just for special use
>>> in our case.
>>>
>>> Best Regards,
>>> Markus
>>>
>>
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Increase usbfs bulk buffer size
  2011-09-12 10:18           ` Markus Rechberger
  2011-09-12 10:25             ` Markus Rechberger
@ 2011-09-12 11:33             ` Greg KH
  2011-09-12 12:14               ` Markus Rechberger
  1 sibling, 1 reply; 14+ messages in thread
From: Greg KH @ 2011-09-12 11:33 UTC (permalink / raw)
  To: Markus Rechberger
  Cc: Oliver Neukum, Greg Kroah-Hartman, linux-usb, linux-kernel

On Mon, Sep 12, 2011 at 12:18:02PM +0200, Markus Rechberger wrote:
> Hi,
> 
> I'm a little bit curious why you guys are just ignoring this.

Because you seem to be ignoring the fact that changing this can cause
problems on systems.

And because it really doesn't solve any problems, you are only pushing
the processing to the kernel, when it can be done in userspace just as
easily.

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Increase usbfs bulk buffer size
  2011-09-12 11:33             ` Greg KH
@ 2011-09-12 12:14               ` Markus Rechberger
  2011-09-12 14:07                 ` Alan Stern
  0 siblings, 1 reply; 14+ messages in thread
From: Markus Rechberger @ 2011-09-12 12:14 UTC (permalink / raw)
  To: Greg KH; +Cc: Oliver Neukum, Greg Kroah-Hartman, linux-usb, linux-kernel

On Mon, Sep 12, 2011 at 1:33 PM, Greg KH <greg@kroah.com> wrote:
> On Mon, Sep 12, 2011 at 12:18:02PM +0200, Markus Rechberger wrote:
>> Hi,
>>
>> I'm a little bit curious why you guys are just ignoring this.
>
> Because you seem to be ignoring the fact that changing this can cause
> problems on systems.
>

Can you explain which problems? We are even using analogTV on
Freescale ARM Systems.
There is no way to read the maximum allowed buffersize from the kernel
right now,
if an application wants to use it they roughly need to know the
current maximum packet size.

Since this is about a product I'm not eager to introduce trouble to
our customers frankly speaking
those things are well tested at our side.

> And because it really doesn't solve any problems,

wrong, it does solve problems, and it also solved it in the past for
isochronous. Would it help if we ship a sample
device to you?
The easiest way to reproduce this is to take 2 different Ubuntu
versions, an old one with lower Isochronous packet size eg. Ubuntu
9.04
and another new one Ubuntu 10.04.
Simply dragging the player with Ubuntu 9.04 can cause framedrops,
while it works smoothly with 10.04.
Next step update the buffer with 9.04 and the video is also smoothly
with the old version - and there are no framedrops (=incomplete data).
I can imagine because the application just gets a certain timeslice
and cannot reliable requeue many packets.
The problem with Bulk is that we need to submit many Bulk URBs in
order to get it work at all, aside of that it shows up the
same issue as with isochronous.

> you are only pushing
> the processing to the kernel, when it can be done in userspace just as
> easily.
>

Your assumption is wrong, even though I understand your point.
All I want is to sort this out.

Markus

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Increase usbfs bulk buffer size
  2011-09-12 12:14               ` Markus Rechberger
@ 2011-09-12 14:07                 ` Alan Stern
  0 siblings, 0 replies; 14+ messages in thread
From: Alan Stern @ 2011-09-12 14:07 UTC (permalink / raw)
  To: Markus Rechberger
  Cc: Greg KH, Oliver Neukum, USB list, Kernel development list

On Mon, 12 Sep 2011, Markus Rechberger wrote:

> On Mon, Sep 12, 2011 at 1:33 PM, Greg KH <greg@kroah.com> wrote:
> > On Mon, Sep 12, 2011 at 12:18:02PM +0200, Markus Rechberger wrote:
> >> Hi,
> >>
> >> I'm a little bit curious why you guys are just ignoring this.
> >
> > Because you seem to be ignoring the fact that changing this can cause
> > problems on systems.
> >
> 
> Can you explain which problems? We are even using analogTV on
> Freescale ARM Systems.
> There is no way to read the maximum allowed buffersize from the kernel
> right now,
> if an application wants to use it they roughly need to know the
> current maximum packet size.

I don't understand.  Surely applications need to know the maximum 
packet size exactly (not just roughly!) in any case.  They can get that 
value easily by looking at the endpoint descriptor.

Do you mean applications need to know the maximum buffer size?  Why do 
they need to know it?

> Since this is about a product I'm not eager to introduce trouble to
> our customers frankly speaking
> those things are well tested at our side.
> 
> > And because it really doesn't solve any problems,
> 
> wrong, it does solve problems, and it also solved it in the past for
> isochronous.

I can understand that increasing the maximum isochronous buffer size 
fixed a problem.  It's not so clear why increasing the maximum bulk 
buffer size would fix anything, though.  You haven't explained this.

>  Would it help if we ship a sample
> device to you?
> The easiest way to reproduce this is to take 2 different Ubuntu
> versions, an old one with lower Isochronous packet size eg. Ubuntu
> 9.04
> and another new one Ubuntu 10.04.
> Simply dragging the player with Ubuntu 9.04 can cause framedrops,
> while it works smoothly with 10.04.
> Next step update the buffer with 9.04 and the video is also smoothly
> with the old version - and there are no framedrops (=incomplete data).
> I can imagine because the application just gets a certain timeslice
> and cannot reliable requeue many packets.
> The problem with Bulk is that we need to submit many Bulk URBs in
> order to get it work at all, aside of that it shows up the
> same issue as with isochronous.

What's wrong with submitting many bulk URBs?

Besides, if you use libusb then the library will automatically break up
the transfer into as many URBs as necessary.  The application doesn't
have to worry about it at all.

> > you are only pushing
> > the processing to the kernel, when it can be done in userspace just as
> > easily.
> >
> 
> Your assumption is wrong, even though I understand your point.

What's wrong with this assumption?

> All I want is to sort this out.

Alan Stern


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2011-09-12 14:07 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-17 17:07 [PATCH] Increase usbfs bulk buffer size Markus Rechberger
2011-08-17 17:14 ` Oliver Neukum
2011-08-17 17:44   ` Markus Rechberger
2011-08-19  6:23     ` Oliver Neukum
2011-08-19 10:45       ` Markus Rechberger
2011-08-31  6:57         ` Markus Rechberger
2011-09-12 10:18           ` Markus Rechberger
2011-09-12 10:25             ` Markus Rechberger
2011-09-12 11:33             ` Greg KH
2011-09-12 12:14               ` Markus Rechberger
2011-09-12 14:07                 ` Alan Stern
2011-08-18 18:37 ` Greg KH
2011-08-18 19:13   ` Markus Rechberger
2011-08-19  5:26     ` Greg KH

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.