[PATCH] incrase usbdevfs bulk buffer size

[PATCH] incrase usbdevfs bulk buffer size

[Christopher Li]

Increase the usbdevfs buffer size limit.

I hit this limit with running ehci in the VM. The a single ehci
qTD transfer buffer can be 5 pages long, that is 20K. The current devio bulk
transfer limit is 16K.  I am not sure why the bulk transfer limit
is 16K. I can complicate the user space part to work around that,
but it seems much simpler just allow usbdevfs to accept bigger buffers.

I did some limited usage test with ehci controller. It seems works for me,
it did use a bigger buffer and nothing blow up.

Signed-Off-By: Christopher Li <chrisl@vmware.com>

Index: linux-2.6.13.2/drivers/usb/core/devio.c
===================================================================
--- linux-2.6.13.2.orig/drivers/usb/core/devio.c	2005-09-28 13:08:10.000000000 -0700
+++ linux-2.6.13.2/drivers/usb/core/devio.c	2005-10-01 04:58:47.000000000 -0700
@@ -72,7 +72,7 @@ MODULE_PARM_DESC (usbfs_snoop, "true to 
 	} while (0)
 
 
-#define	MAX_USBFS_BUFFER_SIZE	16384
+#define	MAX_USBFS_BUFFER_SIZE	(32*1024)
 
 static inline int connected (struct usb_device *dev)
 {

Re: PATCH] incrase usbdevfs bulk buffer size

[Pete Zaitcev]

On Sat, 1 Oct 2005 16:20:59 -0400, Christopher Li <chrisl@vmware.com> wrote:

> I hit this limit with running ehci in the VM. The a single ehci
> qTD transfer buffer can be 5 pages long, that is 20K. [...]

Even 16K is too much, IMHO.

> I can complicate the user space part to work around that,
> but it seems much simpler just allow usbdevfs to accept bigger buffers.

It seems, yes. However, I assure you that this is not going to
work for anyone who has anything reasonable swapped out, because
of the kmalloc().

16K is an order 2 allocation on systems with 4KB pages, such as
Opteron. It kinda sorta works, but not really.

This looks like a requirement to think about a better API. Also,
the things that Harald was trying to fix, with pids and signals,
just tell me that something was really wrong from the start.

-- Pete

Re: PATCH] incrase usbdevfs bulk buffer size

[Christopher Li]

On Sun, Oct 02, 2005 at 03:08:29PM -0700, Pete Zaitcev wrote:
> On Sat, 1 Oct 2005 16:20:59 -0400, Christopher Li <chrisl@vmware.com> wrote:
> 
> > I hit this limit with running ehci in the VM. The a single ehci
> > qTD transfer buffer can be 5 pages long, that is 20K. [...]
> 
> Even 16K is too much, IMHO.
> 
> > I can complicate the user space part to work around that,
> > but it seems much simpler just allow usbdevfs to accept bigger buffers.
> 
> It seems, yes. However, I assure you that this is not going to
> work for anyone who has anything reasonable swapped out, because
> of the kmalloc().
> 
> 16K is an order 2 allocation on systems with 4KB pages, such as
> Opteron. It kinda sorta works, but not really.
> 
> This looks like a requirement to think about a better API. Also,

I think the API is kind of fine in this aspect. The usbdevfs should be
able to take bigger than 16K, but the internal copy of the urb does not
have to use kmalloc on data buffers.

> the things that Harald was trying to fix, with pids and signals,
> just tell me that something was really wrong from the start.

More detail please?

Chris

Re: PATCH] incrase usbdevfs bulk buffer size

[Pete Zaitcev]

On Sun, 2 Oct 2005 15:34:22 -0400, Christopher Li <usb-devel@chrisli.org> wrote:

> > 16K is an order 2 allocation on systems with 4KB pages, such as
> > Opteron. It kinda sorta works, but not really.
> > 
> > This looks like a requirement to think about a better API. []
> 
> I think the API is kind of fine in this aspect. The usbdevfs should be
> able to take bigger than 16K, but the internal copy of the urb does not
> have to use kmalloc on data buffers.

You miss an important detail here, namely that single URBs do not have
a capability to transfer to a discotiguous buffer. As long as you try
to map one transfer insive VMware to one URB, one and only one kmalloc
has to be done. But if splitting the transfer is acceptable, there is
no reason to up the maximum buffer size in usbfs. But the above is IMHO
only, and perhaps I miss something as well.

-- Pete

Re: PATCH] incrase usbdevfs bulk buffer size

[Christopher Li]

On Sun, Oct 02, 2005 at 09:10:14PM -0700, Pete Zaitcev wrote:
> On Sun, 2 Oct 2005 15:34:22 -0400, Christopher Li <usb-devel@chrisli.org> wrote:
> > 
> > I think the API is kind of fine in this aspect. The usbdevfs should be
> > able to take bigger than 16K, but the internal copy of the urb does not
> > have to use kmalloc on data buffers.
> 
> You miss an important detail here, namely that single URBs do not have
> a capability to transfer to a discotiguous buffer. As long as you try

That is exactly my point that the kernel should not limit itself on only
using contiguous buffers. Every USB controller can handle discrete DMA
buffer why shouldn't the kernel? Obviously it should be nice to address
the contiguous buffer restriction before bump up the bulk transfer limit.

> to map one transfer insive VMware to one URB, one and only one kmalloc

The current usbdevfs does a extra copy between from the user space urb
to the kernel space urb. So it does not matter if the user space urb is
contiguous or not. If the kernel can handle discrete dma internally,
the usbdevfs could use it and maintain it's current API.

BTW, That is not VMware choice how the data buffer was arranged. It is the guest.

> has to be done. But if splitting the transfer is acceptable, there is

I still think fixing the kernel to allow address scatter-getter buffer and
allow bigger buffer size is the right thing to do.

Chris