* [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: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
* 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
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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox