On Sun, 29 Sep 2013, Markus Rechberger wrote: > This patch adds memory mapping support to USBFS for isochronous and bulk > data transfers, it allows to pre-allocate usb transfer buffers. > > The CPU usage decreases 1-2% on my 1.3ghz U7300 notebook > The CPU usage decreases 6-8% on an Intel Atom n270 when > transferring 20mbyte/sec (isochronous), it should be more interesting to > see those > statistics on embedded systems where copying data is more expensive. > > Usage from userspace: > allocation: > rv = ioctl(priv->usbfd, USBDEVFS_ALLOC_MEMORY, &mem); > if (rv == 0) > buffer = mmap(NULL, size, PROT_READ|PROT_WRITE, > MAP_SHARED, priv->usbfd, mem.offset); > use the mapped buffer with urb->buffer. > release: > rv = munmap(buffer, size); > memset(&mem, 0x0, sizeof(struct usbdevfs_memory)); > mem.buffer = buffer; > rv = ioctl(priv->usbfd, USBDEVFS_RELEASE_MEMORY, &mem); > > non freed memory buffers are collected and will be released when closing > the node. > > I tested this patch with Bulk and Isochronous, with and without memory > mapping (applications which don't use mmap will just fall back to the > legacy mechanism). On the whole this seems reasonable. There are a few stylistic things that could be cleaned up (missing blank lines after variable declarations, for example, and other checkpatch issues), but they are minor. Why do you constantly compute (PAGE_SIZE<<get_order(usbm->size)) instead of just using usbm->size directly? (And why is the variable sometimes called usbm and sometimes called usbmem?) The biggest problem is that your proc_alloc_memory() routine doesn't call usbfs_increase_memory_usage(). Without that, there's nothing to prevent a user from allocating all the available kernel memory. > Version 0.3: > * Removed comment > * Renamed USBDEVFS_FREE_MEMORY to USBDEVFS_RELEASE_MEMORY > * Clearing allocated memory > > Version 0.4: > * overriding SG transfers once memory mapped buffers are allocated for > BULK > * adding pgprot_noncached to ensure that the IO memory is not cached > (thanks to Ming Lei for mentioning that) I don't understand this. Why shouldn't the buffer memory be cached? Won't uncached buffers be a lot slower? As I understand it, you wrote this in order to solve problems where memory couldn't be allocated for USB transfers. If the system is running short on memory, won't your USBDEVFS_ALLOC_MEMORY ioctl also fail? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
