Hi Greg, On Thu, Feb 18, 2010 at 7:54 AM, Greg KH <greg@xxxxxxxxx> wrote: > I'm not going to reject this patch, but are you sure about this being > needed? The code path for creating a new urb is very tiny, just a > memory allocation. Is that really noticable in any benchmarks or cpu > usage that you have found? You're definitely right, from a performance standpoint, allocating a fresh urb/buffer each transfer itself wouldn't be a problem. The big perf win here, over the older udlfb code, is the asynchronous dispatch and being able to have several urbs in flight at once, not the pre-allocation itself. I actually implemented it first with alloc/free for each transfer (http://git.plugable.com/gitphp/index.php?p=udlfb&a=commit&h=0a4fa3b22580fd1532d06292e9061b9b2057f6a6), but freeing the associated buffer during completion generated WARN_ONs during each transfer. Google background on the problem I hit (lots of others hitting, too): http://www.google.com/search?q=Linux+WARN_ON+dma_free_coherent I thought about working around by queuing up a deferred op to free buffers outside of interrupt context, but that raised overhead concerns and leak concerns. So that led to the current udlfb implementation, which is a pretty common and efficient pattern, in the drivers I've known. And it's well tested at this point. usb-skel needs to also deal with this same WARN_ON issue, as others are hitting this same problem -- something like the udlfb implementation (using internal list anchor, rather than externally allocated one) may be a good way to go. Best wishes, Bernie _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
