On Thu, Dec 03, 2015 at 10:18:20PM +0100, Steinar H. Gunderson wrote: > But you still think usb_alloc_coherent() is the better way to go? Should be > easy enough to change, just let me know. I must be doing something wrong, because I don't seem to get any frames from my video capture, and when I close the application, I get a slowpath warning: [ 188.781392] ------------[ cut here ]------------ [ 188.781409] WARNING: CPU: 0 PID: 1747 at include/asm-generic/dma-mapping-common.h:274 hcd_buffer_free+0xe9/0x130 [usbcore]() [ 188.781422] Modules linked in: ctr ccm joydev nls_utf8 nls_cp437 vfat fat ext2 arc4 intel_rapl iosf_mbi x86_pkg_temp_thermal intel_powerclamp coretemp snd_hda_codec_realtek snd_hda_codec_generic kvm_intel snd_hda_codec_hdmi iTCO_wdt iTCO_vendor_support kvm crct10dif_pclmul crc32_pclmul crc32c_intel jitterentropy_rng sha256_generic hmac drbg evdev aesni_intel psmouse aes_x86_64 iwlmvm glue_helper lrw serio_raw gf128mul ablk_helper pcspkr cryptd mac80211 efivars i2c_i801 sg thinkpad_acpi lpc_ich iwlwifi nvram snd_hda_intel rtsx_pci_ms memstick snd_hda_codec snd_seq cfg80211 snd_hwdep mei_me snd_seq_device snd_hda_core i915 snd_pcm mei snd_timer wmi i2c_algo_bit drm_kms_helper snd ac drm rfkill tpm_tis tpm battery i2c_core soundcore video button processor autofs4 ext4 crc16 mbcache jbd2 dm_mod sd_mod [ 188.781526] rtsx_pci_sdmmc mmc_core ahci libahci libata scsi_mod ehci_pci ehci_hcd rtsx_pci mfd_core xhci_pci xhci_hcd e1000e ptp usbcore pps_core usb_common thermal [ 188.781550] CPU: 0 PID: 1747 Comm: QXcbEventReader Tainted: G W 4.3.0-usb+ #34 [ 188.781554] Hardware name: LENOVO 20ALCT01WW/20ALCT01WW, BIOS GIET83WW (2.33 ) 08/25/2015 [ 188.781557] ffffffffc003bb40 ffffffff812a5113 0000000000000000 ffffffff81068a2c [ 188.781563] 0000000000004000 ffff8802340c1098 ffffffff8189c400 ffff8802345d8400 [ 188.781570] 0000000000000282 ffffffffc002e279 ffff88020c968000 00000000ffeb0000 [ 188.781578] Call Trace: [ 188.781584] [<ffffffff812a5113>] ? dump_stack+0x40/0x5d [ 188.781596] [<ffffffff81068a2c>] ? warn_slowpath_common+0x7c/0xb0 [ 188.781613] [<ffffffffc002e279>] ? hcd_buffer_free+0xe9/0x130 [usbcore] [ 188.781630] [<ffffffffc002fbc6>] ? dec_usb_memory_use_count+0x56/0x80 [usbcore] [ 188.781647] [<ffffffffc0031962>] ? free_async+0xa2/0xf0 [usbcore] [ 188.781666] [<ffffffffc0031a8e>] ? usbdev_release+0xde/0x130 [usbcore] [ 188.781680] [<ffffffff811a4395>] ? __fput+0xc5/0x1d0 [ 188.781694] [<ffffffff81082cef>] ? task_work_run+0x6f/0x90 [ 188.781711] [<ffffffff8106b46e>] ? do_exit+0x36e/0xa80 [ 188.781724] [<ffffffff8108d2f4>] ? check_preempt_curr+0x74/0x80 [ 188.781737] [<ffffffff8108d30f>] ? ttwu_do_wakeup+0xf/0xc0 [ 188.781753] [<ffffffff8108dc7e>] ? try_to_wake_up+0x3e/0x330 [ 188.781767] [<ffffffff8106bbe4>] ? do_group_exit+0x34/0xb0 [ 188.781783] [<ffffffff81075de7>] ? get_signal+0x297/0x680 [ 188.781799] [<ffffffff810142ce>] ? do_signal+0x1e/0x670 [ 188.781812] [<ffffffff8108e010>] ? wake_up_q+0x60/0x60 [ 188.781828] [<ffffffff811a2d7e>] ? vfs_write+0x13e/0x180 [ 188.781834] [<ffffffff81003a03>] ? prepare_exit_to_usermode+0xa3/0xf0 [ 188.781840] [<ffffffff8151308c>] ? int_ret_from_sys_call+0x25/0x8f [ 188.781844] ---[ end trace 8fd705c87c4a8887 ]--- [ 188.781850] ------------[ cut here ]------------ I'm attaching the current patch for reference. I know I have comments of yours to attend to, but I'd like to try to figure out what's wrong before I embark on polish. /* Steinar */ -- Homepage: https://www.sesse.net/
>From e00443ca99bf5bb4871a7714ce244b63b35e8f83 Mon Sep 17 00:00:00 2001 From: "Steinar H. Gunderson" <sesse@xxxxxxxxxx> Date: Thu, 26 Nov 2015 01:19:13 +0100 Subject: [PATCH] Add support for usbfs zerocopy. Add a new interface for userspace to preallocate memory that can be used with usbfs. This gives two primary benefits: - Zerocopy; data no longer needs to be copied between the userspace and the kernel, but can instead be read directly by the driver from userspace's buffers. This works for both bulk and isochronous transfers; isochronous also no longer need to memset() the buffer to zero to avoid leaking kernel data. - Once the buffers are allocated, USB transfers can no longer fail due to memory fragmentation; previously, long-running programs could run into problems finding a large enough contiguous memory chunk, especially on embedded systems or at high rates. Memory is allocated by using mmap() against the usbfs file descriptor, and similarly deallocated by munmap(). Once memory has been allocated, using it as pointers to a bulk or isochronous operation means you will automatically get zerocopy behavior. Note that this also means you cannot modify outgoing data until the transfer is complete. The same holds for data on the same cache lines as incoming data; DMA modifying them at the same time could lead to your changes being overwritten. Largely based on a patch by Markus Rechberger with some updates. The original patch can be found at: http://sundtek.de/support/devio_mmap_v0.4.diff Signed-off-by: Steinar H. Gunderson <sesse@xxxxxxxxxx> Signed-off-by: Markus Rechberger <mrechberger@xxxxxxxxx> --- drivers/usb/core/devio.c | 201 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 191 insertions(+), 10 deletions(-) diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 38ae877c..389a27c 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -50,6 +50,7 @@ #include <linux/user_namespace.h> #include <linux/scatterlist.h> #include <linux/uaccess.h> +#include <linux/dma-mapping.h> #include <asm/byteorder.h> #include <linux/moduleparam.h> @@ -69,6 +70,7 @@ struct usb_dev_state { spinlock_t lock; /* protects the async urb lists */ struct list_head async_pending; struct list_head async_completed; + struct list_head memory_list; wait_queue_head_t wait; /* wake up if a request completed */ unsigned int discsignr; struct pid *disc_pid; @@ -79,6 +81,17 @@ struct usb_dev_state { u32 disabled_bulk_eps; }; +struct usb_memory { + struct list_head memlist; + int vma_use_count; + int urb_use_count; + u32 size; + void *mem; + dma_addr_t dma_handle; + unsigned long vm_start; + struct usb_dev_state *ps; +}; + struct async { struct list_head asynclist; struct usb_dev_state *ps; @@ -89,6 +102,7 @@ struct async { void __user *userbuffer; void __user *userurb; struct urb *urb; + struct usb_memory *usbm; unsigned int mem_usage; int status; u32 secid; @@ -157,6 +171,74 @@ static int connected(struct usb_dev_state *ps) ps->dev->state != USB_STATE_NOTATTACHED); } +static struct usb_memory * +alloc_usb_memory(struct usb_dev_state *ps, size_t size) +{ + struct usb_memory *usbm; + void *mem; + unsigned long flags; + dma_addr_t dma_handle; + + mem = usb_alloc_coherent(ps->dev, size, GFP_KERNEL, &dma_handle); + if (!mem) + return NULL; + + usbm = kzalloc(sizeof(struct usb_memory), GFP_KERNEL); + if (!usbm) { + usb_free_coherent(ps->dev, size, mem, dma_handle); + return NULL; + } + usbm->mem = mem; + usbm->dma_handle = dma_handle; + usbm->size = size; + usbm->ps = ps; + spin_lock_irqsave(&ps->lock, flags); + list_add_tail(&usbm->memlist, &ps->memory_list); + spin_unlock_irqrestore(&ps->lock, flags); + + return usbm; +} + +static void dec_usb_memory_use_count(struct usb_memory *usbm, int *count) +{ + struct usb_dev_state *ps = usbm->ps; + unsigned long flags; + + spin_lock_irqsave(&ps->lock, flags); + --*count; + if (usbm->urb_use_count == 0 && usbm->vma_use_count == 0) { + list_del_init(&usbm->memlist); + usb_free_coherent(ps->dev, usbm->size, usbm->mem, + usbm->dma_handle); + usbfs_decrease_memory_usage( + usbm->size + sizeof(struct usb_memory)); + kfree(usbm); + } + spin_unlock_irqrestore(&ps->lock, flags); +} + +static struct usb_memory * +find_memory_area(struct usb_dev_state *ps, const struct usbdevfs_urb *uurb) +{ + struct usb_memory *usbm = NULL, *iter; + unsigned long flags; + unsigned long uurb_start = (unsigned long)uurb->buffer; + + spin_lock_irqsave(&ps->lock, flags); + list_for_each_entry(iter, &ps->memory_list, memlist) { + if (uurb_start >= iter->vm_start && + uurb_start < iter->vm_start + iter->size && + uurb->buffer_length <= iter->vm_start + iter->size - + uurb_start) { + usbm = iter; + usbm->urb_use_count++; + break; + } + } + spin_unlock_irqrestore(&ps->lock, flags); + return usbm; +} + static loff_t usbdev_lseek(struct file *file, loff_t offset, int orig) { loff_t ret; @@ -297,8 +379,13 @@ static void free_async(struct async *as) if (sg_page(&as->urb->sg[i])) kfree(sg_virt(&as->urb->sg[i])); } + kfree(as->urb->sg); - kfree(as->urb->transfer_buffer); + if (as->usbm == NULL) + kfree(as->urb->transfer_buffer); + else + dec_usb_memory_use_count(as->usbm, &as->usbm->urb_use_count); + kfree(as->urb->setup_packet); usb_free_urb(as->urb); usbfs_decrease_memory_usage(as->mem_usage); @@ -910,6 +997,7 @@ static int usbdev_open(struct inode *inode, struct file *file) INIT_LIST_HEAD(&ps->list); INIT_LIST_HEAD(&ps->async_pending); INIT_LIST_HEAD(&ps->async_completed); + INIT_LIST_HEAD(&ps->memory_list); init_waitqueue_head(&ps->wait); ps->discsignr = 0; ps->disc_pid = get_pid(task_pid(current)); @@ -962,6 +1050,13 @@ static int usbdev_release(struct inode *inode, struct file *file) free_async(as); as = async_getcompleted(ps); } + + /* Any elements still left on this list are still in use and cannot + * be deleted here, but will be freed once the counters go to 0 + * (see dec_usb_memory_use_count()). + */ + list_del(&ps->memory_list); + kfree(ps); return 0; } @@ -1291,6 +1386,7 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb struct usb_host_endpoint *ep; struct async *as = NULL; struct usb_ctrlrequest *dr = NULL; + struct usb_memory *usbm = NULL; unsigned int u, totlen, isofrmlen; int i, ret, is_in, num_sgs = 0, ifnum = -1; int number_of_packets = 0; @@ -1372,9 +1468,17 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb uurb->type = USBDEVFS_URB_TYPE_INTERRUPT; goto interrupt_urb; } - num_sgs = DIV_ROUND_UP(uurb->buffer_length, USB_SG_SIZE); - if (num_sgs == 1 || num_sgs > ps->dev->bus->sg_tablesize) - num_sgs = 0; + /* do not use SG buffers when memory mapped segments + * are in use + */ + if (find_memory_area(ps, uurb) == NULL) { + num_sgs = DIV_ROUND_UP(uurb->buffer_length, + USB_SG_SIZE); + if (num_sgs == 1 || + num_sgs > ps->dev->bus->sg_tablesize) { + num_sgs = 0; + } + } if (ep->streams) stream_id = uurb->stream_id; break; @@ -1445,10 +1549,11 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb if (ret) goto error; as->mem_usage = u; + as->usbm = NULL; if (num_sgs) { as->urb->sg = kmalloc(num_sgs * sizeof(struct scatterlist), - GFP_KERNEL); + GFP_KERNEL); if (!as->urb->sg) { ret = -ENOMEM; goto error; @@ -1476,21 +1581,33 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb totlen -= u; } } else if (uurb->buffer_length > 0) { - as->urb->transfer_buffer = kmalloc(uurb->buffer_length, + if (!list_empty(&ps->memory_list)) { + usbm = find_memory_area(ps, uurb); + if (usbm) { + as->usbm = usbm; + as->urb->transfer_buffer = usbm->mem; + } else { + ret = -ENOMEM; + goto error; + } + } else { + as->urb->transfer_buffer = kmalloc(uurb->buffer_length, GFP_KERNEL); + } if (!as->urb->transfer_buffer) { ret = -ENOMEM; goto error; } - if (!is_in) { + if (!is_in && usbm == NULL) { if (copy_from_user(as->urb->transfer_buffer, uurb->buffer, uurb->buffer_length)) { ret = -EFAULT; goto error; } - } else if (uurb->type == USBDEVFS_URB_TYPE_ISO) { + } else if (uurb->type == USBDEVFS_URB_TYPE_ISO && + usbm == NULL) { /* * Isochronous input data may end up being * discontiguous if some of the packets are short. @@ -1545,10 +1662,16 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb isopkt = NULL; as->ps = ps; as->userurb = arg; - if (is_in && uurb->buffer_length > 0) + if (is_in && uurb->buffer_length > 0 && usbm == NULL) { as->userbuffer = uurb->buffer; - else + } else { as->userbuffer = NULL; + } + if (usbm != NULL) { + as->urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP; + as->urb->transfer_dma = usbm->dma_handle + + (uurb->buffer - usbm->mem); + } as->signr = uurb->signr; as->ifnum = ifnum; as->pid = get_pid(task_pid(current)); @@ -1604,6 +1727,8 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb return 0; error: + if (usbm) + dec_usb_memory_use_count(usbm, &usbm->urb_use_count); kfree(isopkt); kfree(dr); if (as) @@ -2337,6 +2462,61 @@ static long usbdev_ioctl(struct file *file, unsigned int cmd, return ret; } +static void usbdev_vm_open(struct vm_area_struct *vma) +{ + struct usb_memory *usbm = vma->vm_private_data; + unsigned long flags; + + spin_lock_irqsave(&usbm->ps->lock, flags); + ++usbm->vma_use_count; + spin_unlock_irqrestore(&usbm->ps->lock, flags); +} + +static void usbdev_vm_close(struct vm_area_struct *vma) +{ + struct usb_memory *usbm = vma->vm_private_data; + + dec_usb_memory_use_count(usbm, &usbm->vma_use_count); +} + +struct vm_operations_struct usbdev_vm_ops = { + .open = usbdev_vm_open, + .close = usbdev_vm_close +}; + +static int usbdev_mmap(struct file *file, struct vm_area_struct *vma) +{ + struct usb_memory *usbm = NULL; + struct usb_dev_state *ps = file->private_data; + size_t size = vma->vm_end - vma->vm_start; + int ret; + + ret = usbfs_increase_memory_usage(size + sizeof(struct usb_memory)); + if (ret) + return ret; + + usbm = alloc_usb_memory(ps, size); + if (!usbm) + return -ENOMEM; + + if (remap_pfn_range(vma, vma->vm_start, + virt_to_phys(usbm->mem) >> PAGE_SHIFT, + size, vma->vm_page_prot) < 0) { + int dummy_count = 1; + + dec_usb_memory_use_count(usbm, &dummy_count); + return -EAGAIN; + } + + usbm->vm_start = vma->vm_start; + vma->vm_flags |= VM_IO; + vma->vm_flags |= (VM_DONTEXPAND | VM_DONTDUMP); + vma->vm_ops = &usbdev_vm_ops; + vma->vm_private_data = usbm; + usbdev_vm_open(vma); + return 0; +} + #ifdef CONFIG_COMPAT static long usbdev_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg) @@ -2373,6 +2553,7 @@ const struct file_operations usbdev_file_operations = { #ifdef CONFIG_COMPAT .compat_ioctl = usbdev_compat_ioctl, #endif + .mmap = usbdev_mmap, .open = usbdev_open, .release = usbdev_release, }; -- 2.1.4
