[PATCH 3/10] udlfb: pre-allocated urb list helpers
bernie at plugable.com
Thu Feb 18 22:32:35 UTC 2010
On Thu, Feb 18, 2010 at 7:54 AM, Greg KH <greg at kroah.com> 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
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):
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.
More information about the devel