[PATCH] Add support for usbfs zerocopy.

[PATCH] Add support for usbfs zerocopy.

[Steinar H. Gunderson]

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 all kinds of transfers (even if
   nonsensical for control and interrupt 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.

There's a new capability USBDEVFS_CAP_MMAP that userspace can query to see
if the running kernel supports this functionality, if just trying mmap() is
not acceptable.

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@google.com>
Signed-off-by: Markus Rechberger <mrechberger@gmail.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
---
 drivers/usb/core/devio.c          | 227 +++++++++++++++++++++++++++++++++-----
 include/uapi/linux/usbdevice_fs.h |   1 +
 2 files changed, 203 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 38ae877c..0238c78 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,111 @@ static int connected(struct usb_dev_state *ps)
 			ps->dev->state != USB_STATE_NOTATTACHED);
 }
 
+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(&usbm->memlist);
+		spin_unlock_irqrestore(&ps->lock, flags);
+
+		usb_free_coherent(ps->dev, usbm->size, usbm->mem,
+				usbm->dma_handle);
+		usbfs_decrease_memory_usage(
+			usbm->size + sizeof(struct usb_memory));
+		kfree(usbm);
+	} else {
+		spin_unlock_irqrestore(&ps->lock, flags);
+	}
+}
+
+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;
+	void *mem;
+	unsigned long flags;
+	dma_addr_t dma_handle;
+	int ret;
+
+	ret = usbfs_increase_memory_usage(size + sizeof(struct usb_memory));
+	if (ret)
+		goto error;
+
+	usbm = kzalloc(sizeof(struct usb_memory), GFP_KERNEL);
+	if (!usbm) {
+		ret = -ENOMEM;
+		goto error_decrease_mem;
+	}
+
+	mem = usb_alloc_coherent(ps->dev, size, GFP_USER, &dma_handle);
+	if (!mem) {
+		ret = -ENOMEM;
+		goto error_free_usbm;
+	}
+
+	memset(mem, 0, size);
+
+	usbm->mem = mem;
+	usbm->dma_handle = dma_handle;
+	usbm->size = size;
+	usbm->ps = ps;
+	usbm->vm_start = vma->vm_start;
+	usbm->vma_use_count = 1;
+	INIT_LIST_HEAD(&usbm->memlist);
+
+	if (remap_pfn_range(vma, vma->vm_start,
+			virt_to_phys(usbm->mem) >> PAGE_SHIFT,
+			size, vma->vm_page_prot) < 0) {
+		dec_usb_memory_use_count(usbm, &usbm->vma_use_count);
+		return -EAGAIN;
+	}
+
+	vma->vm_flags |= VM_IO;
+	vma->vm_flags |= (VM_DONTEXPAND | VM_DONTDUMP);
+	vma->vm_ops = &usbdev_vm_ops;
+	vma->vm_private_data = usbm;
+
+	spin_lock_irqsave(&ps->lock, flags);
+	list_add_tail(&usbm->memlist, &ps->memory_list);
+	spin_unlock_irqrestore(&ps->lock, flags);
+
+	return 0;
+
+error_free_usbm:
+	kfree(usbm);
+error_decrease_mem:
+	usbfs_decrease_memory_usage(size + sizeof(struct usb_memory));
+error:
+	return ret;
+}
+
 static loff_t usbdev_lseek(struct file *file, loff_t offset, int orig)
 {
 	loff_t ret;
@@ -297,8 +416,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 +1034,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 +1087,7 @@ static int usbdev_release(struct inode *inode, struct file *file)
 		free_async(as);
 		as = async_getcompleted(ps);
 	}
+
 	kfree(ps);
 	return 0;
 }
@@ -1283,6 +1409,31 @@ static int proc_setconfig(struct usb_dev_state *ps, void __user *arg)
 	return status;
 }
 
+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) {
+			if (uurb->buffer_length > iter->vm_start + iter->size -
+					uurb_start) {
+				usbm = ERR_PTR(-EINVAL);
+			} else {
+				usbm = iter;
+				usbm->urb_use_count++;
+			}
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&ps->lock, flags);
+	return usbm;
+}
+
 static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb,
 			struct usbdevfs_iso_packet_desc __user *iso_frame_desc,
 			void __user *arg)
@@ -1439,6 +1590,19 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb
 		goto error;
 	}
 
+	as->usbm = find_memory_area(ps, uurb);
+	if (IS_ERR(as->usbm)) {
+		ret = PTR_ERR(as->usbm);
+		as->usbm = NULL;
+		goto error;
+	}
+
+	/* do not use SG buffers when memory mapped segments
+	 * are in use
+	 */
+	if (as->usbm)
+		num_sgs = 0;
+
 	u += sizeof(struct async) + sizeof(struct urb) + uurb->buffer_length +
 	     num_sgs * sizeof(struct scatterlist);
 	ret = usbfs_increase_memory_usage(u);
@@ -1476,29 +1640,35 @@ 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,
-				GFP_KERNEL);
-		if (!as->urb->transfer_buffer) {
-			ret = -ENOMEM;
-			goto error;
-		}
+		if (as->usbm) {
+			unsigned long uurb_start = (unsigned long)uurb->buffer;
 
-		if (!is_in) {
-			if (copy_from_user(as->urb->transfer_buffer,
-					   uurb->buffer,
-					   uurb->buffer_length)) {
-				ret = -EFAULT;
+			as->urb->transfer_buffer = as->usbm->mem +
+					(uurb_start - as->usbm->vm_start);
+		} else {
+			as->urb->transfer_buffer = kmalloc(uurb->buffer_length,
+					GFP_KERNEL);
+			if (!as->urb->transfer_buffer) {
+				ret = -ENOMEM;
 				goto error;
 			}
-		} else if (uurb->type == USBDEVFS_URB_TYPE_ISO) {
-			/*
-			 * Isochronous input data may end up being
-			 * discontiguous if some of the packets are short.
-			 * Clear the buffer so that the gaps don't leak
-			 * kernel data to userspace.
-			 */
-			memset(as->urb->transfer_buffer, 0,
-					uurb->buffer_length);
+			if (!is_in) {
+				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) {
+				/*
+				 * Isochronous input data may end up being
+				 * discontiguous if some of the packets are
+				 * short. Clear the buffer so that the gaps
+				 * don't leak kernel data to userspace.
+				 */
+				memset(as->urb->transfer_buffer, 0,
+						uurb->buffer_length);
+			}
 		}
 	}
 	as->urb->dev = ps->dev;
@@ -1545,10 +1715,14 @@ 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 (as->usbm) {
+		unsigned long uurb_start = (unsigned long)uurb->buffer;
+
+		as->urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+		as->urb->transfer_dma = as->usbm->dma_handle +
+				(uurb_start - as->usbm->vm_start);
+	} else if (is_in && uurb->buffer_length > 0)
 		as->userbuffer = uurb->buffer;
-	else
-		as->userbuffer = NULL;
 	as->signr = uurb->signr;
 	as->ifnum = ifnum;
 	as->pid = get_pid(task_pid(current));
@@ -1604,6 +1778,8 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb
 	return 0;
 
  error:
+	if (as && as->usbm)
+		dec_usb_memory_use_count(as->usbm, &as->usbm->urb_use_count);
 	kfree(isopkt);
 	kfree(dr);
 	if (as)
@@ -2047,7 +2223,7 @@ static int proc_get_capabilities(struct usb_dev_state *ps, void __user *arg)
 	__u32 caps;
 
 	caps = USBDEVFS_CAP_ZERO_PACKET | USBDEVFS_CAP_NO_PACKET_SIZE_LIM |
-			USBDEVFS_CAP_REAP_AFTER_DISCONNECT;
+			USBDEVFS_CAP_REAP_AFTER_DISCONNECT | USBDEVFS_CAP_MMAP;
 	if (!ps->dev->bus->no_stop_on_short)
 		caps |= USBDEVFS_CAP_BULK_CONTINUATION;
 	if (ps->dev->bus->sg_tablesize)
@@ -2373,6 +2549,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,
 };
diff --git a/include/uapi/linux/usbdevice_fs.h b/include/uapi/linux/usbdevice_fs.h
index 019ba1e..ecbd176 100644
--- a/include/uapi/linux/usbdevice_fs.h
+++ b/include/uapi/linux/usbdevice_fs.h
@@ -134,6 +134,7 @@ struct usbdevfs_hub_portinfo {
 #define USBDEVFS_CAP_NO_PACKET_SIZE_LIM		0x04
 #define USBDEVFS_CAP_BULK_SCATTER_GATHER	0x08
 #define USBDEVFS_CAP_REAP_AFTER_DISCONNECT	0x10
+#define USBDEVFS_CAP_MMAP			0x20
 
 /* USBDEVFS_DISCONNECT_CLAIM flags & struct */
 
-- 
2.1.4

[PATCH v2] Add support for usbfs zerocopy.

[Steinar H. Gunderson]

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 all kinds of transfers (even if
   nonsensical for control and interrupt 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.

There's a new capability USBDEVFS_CAP_MMAP that userspace can query to see
if the running kernel supports this functionality, if just trying mmap() is
not acceptable.

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@google.com>
Signed-off-by: Markus Rechberger <mrechberger@gmail.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
---
 drivers/usb/core/devio.c          | 227 +++++++++++++++++++++++++++++++++-----
 include/uapi/linux/usbdevice_fs.h |   1 +
 2 files changed, 203 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 38ae877c..0238c78 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,111 @@ static int connected(struct usb_dev_state *ps)
 			ps->dev->state != USB_STATE_NOTATTACHED);
 }
 
+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(&usbm->memlist);
+		spin_unlock_irqrestore(&ps->lock, flags);
+
+		usb_free_coherent(ps->dev, usbm->size, usbm->mem,
+				usbm->dma_handle);
+		usbfs_decrease_memory_usage(
+			usbm->size + sizeof(struct usb_memory));
+		kfree(usbm);
+	} else {
+		spin_unlock_irqrestore(&ps->lock, flags);
+	}
+}
+
+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;
+	void *mem;
+	unsigned long flags;
+	dma_addr_t dma_handle;
+	int ret;
+
+	ret = usbfs_increase_memory_usage(size + sizeof(struct usb_memory));
+	if (ret)
+		goto error;
+
+	usbm = kzalloc(sizeof(struct usb_memory), GFP_KERNEL);
+	if (!usbm) {
+		ret = -ENOMEM;
+		goto error_decrease_mem;
+	}
+
+	mem = usb_alloc_coherent(ps->dev, size, GFP_USER, &dma_handle);
+	if (!mem) {
+		ret = -ENOMEM;
+		goto error_free_usbm;
+	}
+
+	memset(mem, 0, size);
+
+	usbm->mem = mem;
+	usbm->dma_handle = dma_handle;
+	usbm->size = size;
+	usbm->ps = ps;
+	usbm->vm_start = vma->vm_start;
+	usbm->vma_use_count = 1;
+	INIT_LIST_HEAD(&usbm->memlist);
+
+	if (remap_pfn_range(vma, vma->vm_start,
+			virt_to_phys(usbm->mem) >> PAGE_SHIFT,
+			size, vma->vm_page_prot) < 0) {
+		dec_usb_memory_use_count(usbm, &usbm->vma_use_count);
+		return -EAGAIN;
+	}
+
+	vma->vm_flags |= VM_IO;
+	vma->vm_flags |= (VM_DONTEXPAND | VM_DONTDUMP);
+	vma->vm_ops = &usbdev_vm_ops;
+	vma->vm_private_data = usbm;
+
+	spin_lock_irqsave(&ps->lock, flags);
+	list_add_tail(&usbm->memlist, &ps->memory_list);
+	spin_unlock_irqrestore(&ps->lock, flags);
+
+	return 0;
+
+error_free_usbm:
+	kfree(usbm);
+error_decrease_mem:
+	usbfs_decrease_memory_usage(size + sizeof(struct usb_memory));
+error:
+	return ret;
+}
+
 static loff_t usbdev_lseek(struct file *file, loff_t offset, int orig)
 {
 	loff_t ret;
@@ -297,8 +416,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 +1034,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 +1087,7 @@ static int usbdev_release(struct inode *inode, struct file *file)
 		free_async(as);
 		as = async_getcompleted(ps);
 	}
+
 	kfree(ps);
 	return 0;
 }
@@ -1283,6 +1409,31 @@ static int proc_setconfig(struct usb_dev_state *ps, void __user *arg)
 	return status;
 }
 
+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) {
+			if (uurb->buffer_length > iter->vm_start + iter->size -
+					uurb_start) {
+				usbm = ERR_PTR(-EINVAL);
+			} else {
+				usbm = iter;
+				usbm->urb_use_count++;
+			}
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&ps->lock, flags);
+	return usbm;
+}
+
 static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb,
 			struct usbdevfs_iso_packet_desc __user *iso_frame_desc,
 			void __user *arg)
@@ -1439,6 +1590,19 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb
 		goto error;
 	}
 
+	as->usbm = find_memory_area(ps, uurb);
+	if (IS_ERR(as->usbm)) {
+		ret = PTR_ERR(as->usbm);
+		as->usbm = NULL;
+		goto error;
+	}
+
+	/* do not use SG buffers when memory mapped segments
+	 * are in use
+	 */
+	if (as->usbm)
+		num_sgs = 0;
+
 	u += sizeof(struct async) + sizeof(struct urb) + uurb->buffer_length +
 	     num_sgs * sizeof(struct scatterlist);
 	ret = usbfs_increase_memory_usage(u);
@@ -1476,29 +1640,35 @@ 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,
-				GFP_KERNEL);
-		if (!as->urb->transfer_buffer) {
-			ret = -ENOMEM;
-			goto error;
-		}
+		if (as->usbm) {
+			unsigned long uurb_start = (unsigned long)uurb->buffer;
 
-		if (!is_in) {
-			if (copy_from_user(as->urb->transfer_buffer,
-					   uurb->buffer,
-					   uurb->buffer_length)) {
-				ret = -EFAULT;
+			as->urb->transfer_buffer = as->usbm->mem +
+					(uurb_start - as->usbm->vm_start);
+		} else {
+			as->urb->transfer_buffer = kmalloc(uurb->buffer_length,
+					GFP_KERNEL);
+			if (!as->urb->transfer_buffer) {
+				ret = -ENOMEM;
 				goto error;
 			}
-		} else if (uurb->type == USBDEVFS_URB_TYPE_ISO) {
-			/*
-			 * Isochronous input data may end up being
-			 * discontiguous if some of the packets are short.
-			 * Clear the buffer so that the gaps don't leak
-			 * kernel data to userspace.
-			 */
-			memset(as->urb->transfer_buffer, 0,
-					uurb->buffer_length);
+			if (!is_in) {
+				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) {
+				/*
+				 * Isochronous input data may end up being
+				 * discontiguous if some of the packets are
+				 * short. Clear the buffer so that the gaps
+				 * don't leak kernel data to userspace.
+				 */
+				memset(as->urb->transfer_buffer, 0,
+						uurb->buffer_length);
+			}
 		}
 	}
 	as->urb->dev = ps->dev;
@@ -1545,10 +1715,14 @@ 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 (as->usbm) {
+		unsigned long uurb_start = (unsigned long)uurb->buffer;
+
+		as->urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+		as->urb->transfer_dma = as->usbm->dma_handle +
+				(uurb_start - as->usbm->vm_start);
+	} else if (is_in && uurb->buffer_length > 0)
 		as->userbuffer = uurb->buffer;
-	else
-		as->userbuffer = NULL;
 	as->signr = uurb->signr;
 	as->ifnum = ifnum;
 	as->pid = get_pid(task_pid(current));
@@ -1604,6 +1778,8 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb
 	return 0;
 
  error:
+	if (as && as->usbm)
+		dec_usb_memory_use_count(as->usbm, &as->usbm->urb_use_count);
 	kfree(isopkt);
 	kfree(dr);
 	if (as)
@@ -2047,7 +2223,7 @@ static int proc_get_capabilities(struct usb_dev_state *ps, void __user *arg)
 	__u32 caps;
 
 	caps = USBDEVFS_CAP_ZERO_PACKET | USBDEVFS_CAP_NO_PACKET_SIZE_LIM |
-			USBDEVFS_CAP_REAP_AFTER_DISCONNECT;
+			USBDEVFS_CAP_REAP_AFTER_DISCONNECT | USBDEVFS_CAP_MMAP;
 	if (!ps->dev->bus->no_stop_on_short)
 		caps |= USBDEVFS_CAP_BULK_CONTINUATION;
 	if (ps->dev->bus->sg_tablesize)
@@ -2373,6 +2549,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,
 };
diff --git a/include/uapi/linux/usbdevice_fs.h b/include/uapi/linux/usbdevice_fs.h
index 019ba1e..ecbd176 100644
--- a/include/uapi/linux/usbdevice_fs.h
+++ b/include/uapi/linux/usbdevice_fs.h
@@ -134,6 +134,7 @@ struct usbdevfs_hub_portinfo {
 #define USBDEVFS_CAP_NO_PACKET_SIZE_LIM		0x04
 #define USBDEVFS_CAP_BULK_SCATTER_GATHER	0x08
 #define USBDEVFS_CAP_REAP_AFTER_DISCONNECT	0x10
+#define USBDEVFS_CAP_MMAP			0x20
 
 /* USBDEVFS_DISCONNECT_CLAIM flags & struct */
 
-- 
2.1.4

Does vm_operations_struct require a .owner field?

[Alan Stern]

Hello:

Question: The vm_operations_struct structure contains lots of callback
pointers.  Is there any mechanism to prevent the callback routines and
the structure itself being unloaded from memory (if they are built into
modules) while the relevant VMAs are still in use?

Consider a simple example: A user program calls mmap(2) on a device
file.  Later on, the file is closed and the device driver's module is
unloaded.  But until munmap(2) is called or the user program exits, the
memory mapping and the corresponding VMA will remain in existence.  
(The man page for munmap specifically says "closing the file descriptor
does not unmap the region".)  Thus when the user program does do an
munmap(), the callback to vma->vm_ops->close will reference nonexistent
memory and cause an oops.

Normally this sort of thing is prevented by try_module_get(...->owner).  
But vm_operations_struct doesn't include a .owner field.

Am I missing something here?

Alan Stern

Re: Does vm_operations_struct require a .owner field?

[Kirill A. Shutemov]

On Tue, Jan 05, 2016 at 11:27:45AM -0500, Alan Stern wrote:
> Hello:
> 
> Question: The vm_operations_struct structure contains lots of callback
> pointers.  Is there any mechanism to prevent the callback routines and
> the structure itself being unloaded from memory (if they are built into
> modules) while the relevant VMAs are still in use?
> 
> Consider a simple example: A user program calls mmap(2) on a device
> file.  Later on, the file is closed and the device driver's module is
> unloaded.  But until munmap(2) is called or the user program exits, the
> memory mapping and the corresponding VMA will remain in existence.  
> (The man page for munmap specifically says "closing the file descriptor
> does not unmap the region".)  Thus when the user program does do an
> munmap(), the callback to vma->vm_ops->close will reference nonexistent
> memory and cause an oops.
> 
> Normally this sort of thing is prevented by try_module_get(...->owner).  
> But vm_operations_struct doesn't include a .owner field.
> 
> Am I missing something here?

mmap(2) takes reference of the file, therefore the file is not closed from
kernel POV until vma is gone and you cannot unload relevant module.
See get_file() in mmap_region().

-- 
 Kirill A. Shutemov

Re: Does vm_operations_struct require a .owner field?

[Alan Stern]

On Tue, 5 Jan 2016, Kirill A. Shutemov wrote:

> On Tue, Jan 05, 2016 at 11:27:45AM -0500, Alan Stern wrote:
> > Hello:
> > 
> > Question: The vm_operations_struct structure contains lots of callback
> > pointers.  Is there any mechanism to prevent the callback routines and
> > the structure itself being unloaded from memory (if they are built into
> > modules) while the relevant VMAs are still in use?
> > 
> > Consider a simple example: A user program calls mmap(2) on a device
> > file.  Later on, the file is closed and the device driver's module is
> > unloaded.  But until munmap(2) is called or the user program exits, the
> > memory mapping and the corresponding VMA will remain in existence.  
> > (The man page for munmap specifically says "closing the file descriptor
> > does not unmap the region".)  Thus when the user program does do an
> > munmap(), the callback to vma->vm_ops->close will reference nonexistent
> > memory and cause an oops.
> > 
> > Normally this sort of thing is prevented by try_module_get(...->owner).  
> > But vm_operations_struct doesn't include a .owner field.
> > 
> > Am I missing something here?
> 
> mmap(2) takes reference of the file, therefore the file is not closed from
> kernel POV until vma is gone and you cannot unload relevant module.
> See get_file() in mmap_region().

Thank you.  So it looks like I was worried about nothing.

Steinar, you can remove the try_module_get/module_put lines from your
patch.  Also, the list_del() and comment in usbdev_release() aren't 
needed -- at that point we know the memory_list has to be empty since 
there can't be any outstanding URBs or VMA references.  If you take 
those things out then the patch should be ready for merging.

Alan Stern

Re: Does vm_operations_struct require a .owner field?

[Steinar H. Gunderson]

On Tue, Jan 05, 2016 at 04:31:09PM -0500, Alan Stern wrote:
> Thank you.  So it looks like I was worried about nothing.
> 
> Steinar, you can remove the try_module_get/module_put lines from your
> patch.  Also, the list_del() and comment in usbdev_release() aren't 
> needed -- at that point we know the memory_list has to be empty since 
> there can't be any outstanding URBs or VMA references.  If you take 
> those things out then the patch should be ready for merging.

Good, thanks. Did so, compiled, testing it still works, sending :-)

/* Steinar */
-- 
Software Engineer, Google Switzerland

Re: [PATCH] Add support for usbfs zerocopy.

[Greg Kroah-Hartman]

On Thu, Nov 26, 2015 at 01:19:13AM +0100, Steinar H. Gunderson wrote:
> Add a new interface for userspace to preallocate memory that can be
> used with usbfs. This gives two primary benefits:

Please 'version' your patches, so that I have a chance to figure out
what patch is what, and what changed between patches.

otherwise the odds of me picking the "wrong" one is _very_ high...

thanks,

greg k-h

Re: [PATCH] Add support for usbfs zerocopy.

[Steinar H. Gunderson]

On Tue, Jan 05, 2016 at 04:11:43PM -0800, Greg Kroah-Hartman wrote:
>> Add a new interface for userspace to preallocate memory that can be
>> used with usbfs. This gives two primary benefits:
> Please 'version' your patches, so that I have a chance to figure out
> what patch is what, and what changed between patches.
> 
> otherwise the odds of me picking the "wrong" one is _very_ high...

OK. I won't make any attempt at reconstructing the history, but I'll resend
the one I just sent you as v2, ie. --reroll-count=2.

Somehow it feels like there should be a way to integrate this better into my
MUA, but hopefully this is soon all done anyway. :-)

/* Steinar */
-- 
Software Engineer, Google Switzerland

Re: [PATCH] Add support for usbfs zerocopy.

[Christoph Hellwig]

This is a completely broken usage of the mmap interface.  if you use
mmap on a device file you must use the actual mmap for the data
transfer.

Our interface for zero copy reads/writes is O_DIRECT, and that requires
not special memory allocation, just proper alignment.

Re: [PATCH] Add support for usbfs zerocopy.

[Steinar H. Gunderson]

On Tue, Jan 05, 2016 at 10:49:49PM -0800, Christoph Hellwig wrote:
> This is a completely broken usage of the mmap interface.  if you use
> mmap on a device file you must use the actual mmap for the data
> transfer.

Really? V4L does exactly the same thing, from what I can see. It's just a way
of allocating memory with specific properties, roughly similar to hugetlbfs.

> Our interface for zero copy reads/writes is O_DIRECT, and that requires
> not special memory allocation, just proper alignment.

But that assumes you are using I/O using read()/write(). There's no way you
can shoehorn USB isochronous reads into the read() interface, O_DIRECT or not.

/* Steinar */
-- 
Software Engineer, Google Switzerland

Re: [PATCH] Add support for usbfs zerocopy.

[Peter Stuge]

Steinar H. Gunderson wrote:
> > Our interface for zero copy reads/writes is O_DIRECT, and that requires
> > not special memory allocation, just proper alignment.
> 
> But that assumes you are using I/O using read()/write(). There's no way you
> can shoehorn USB isochronous reads into the read() interface, O_DIRECT or not.

How about aio?


//Peter

Re: [PATCH] Add support for usbfs zerocopy.

[Alan Stern]

On Wed, 6 Jan 2016, Steinar H. Gunderson wrote:

> On Tue, Jan 05, 2016 at 10:49:49PM -0800, Christoph Hellwig wrote:
> > This is a completely broken usage of the mmap interface.  if you use
> > mmap on a device file you must use the actual mmap for the data
> > transfer.
> 
> Really? V4L does exactly the same thing, from what I can see. It's just a way
> of allocating memory with specific properties, roughly similar to hugetlbfs.
> 
> > Our interface for zero copy reads/writes is O_DIRECT, and that requires
> > not special memory allocation, just proper alignment.
> 
> But that assumes you are using I/O using read()/write(). There's no way you
> can shoehorn USB isochronous reads into the read() interface, O_DIRECT or not.

Indeed, the I/O operations we are using with mmap here are not reads or 
writes; they are ioctls.  As far as I know, the kernel doesn't have any 
defined interface for zerocopy ioctls.

Furthermore, this approach _does_ use the mmap for data transfers.  I'm 
not sure what Christoph was referring to.

Alan Stern

Re: [PATCH] Add support for usbfs zerocopy.

[Steinar H. Gunderson]

On Wed, Jan 06, 2016 at 04:22:12PM +0100, Peter Stuge wrote:
>>> Our interface for zero copy reads/writes is O_DIRECT, and that requires
>>> not special memory allocation, just proper alignment.
>> But that assumes you are using I/O using read()/write(). There's no way you
>> can shoehorn USB isochronous reads into the read() interface, O_DIRECT or not.
> How about aio?

I don't really see how; a USB device does not look much like a file. (Where
would you stick the endpoint, for one? And how would you ever submit an URB
with multiple packets in it, which is essential?) It feels a bit like
trying to use UDP sockets with only read() and write().

In any case, the usbfs interface already exists and is stable. This is about
extending it; replacing it with something new from scratch to get zerocopy
would seem overkill.

/* Steinar */
-- 
Software Engineer, Google Switzerland

Re: [PATCH] Add support for usbfs zerocopy.

[Alan Stern]

On Wed, 6 Jan 2016, Peter Stuge wrote:

> Steinar H. Gunderson wrote:
> > > Our interface for zero copy reads/writes is O_DIRECT, and that requires
> > > not special memory allocation, just proper alignment.
> > 
> > But that assumes you are using I/O using read()/write(). There's no way you
> > can shoehorn USB isochronous reads into the read() interface, O_DIRECT or not.
> 
> How about aio?

aio is not zerocopy.  And it also doesn't solve the memory allocation 
problem that originally affected both Steiner and Markus.

(Were you thinking of "asynchronous" instead of "isochronous"?)

Alan Stern

Re: [PATCH v2] Add support for usbfs zerocopy.

[Lingzhu Xiang]

On Wed, Nov 25, 2015 at 7:19 PM, Steinar H. Gunderson <sesse@google.com> wrote:
> Add a new interface for userspace to preallocate memory that can be
> used with usbfs. This gives two primary benefits:

I got this when trying to allocate a little bit large buffer (~4MB)
using the new userspace libusb_dev_mem_alloc():

> [ 1706.212407] usb 2-1.1: reset SuperSpeed USB device number 3 using xhci_hcd
> [ 1706.234823] xhci_hcd 0000:00:14.0: swiotlb buffer is full (sz: 4325376 bytes)
> [ 1706.234827] swiotlb: coherent allocation failed for device 0000:00:14.0 size=4325376
> [ 1706.234830] CPU: 1 PID: 3233 Comm: Protonect Tainted: G     U  W       4.4.0-rc8-amd64 #1 Debian 4.4~rc8-1~exp1
> [ 1706.234831] Hardware name: LENOVO 20ALCTO1WW/20ALCTO1WW, BIOS GIET76WW (2.26 ) 08/27/2014
> [ 1706.234833]  0000000000000000 000000000f50c266 ffffffff812e6019 ffffffffffffffff
> [ 1706.234836]  ffffffff8130dc45 ffff88020000000b 0000000000420000 ffffffff81a2a0e0
> [ 1706.234838]  ffff880206263d80 0000000000000000 ffff88021c892f40 0000000000420040
> [ 1706.234841] Call Trace:
> [ 1706.234847]  [<ffffffff812e6019>] ? dump_stack+0x40/0x57
> [ 1706.234851]  [<ffffffff8130dc45>] ? swiotlb_alloc_coherent+0x135/0x150
> [ 1706.234867]  [<ffffffffa021deb1>] ? hcd_buffer_alloc+0xb1/0x130 [usbcore]
> [ 1706.234875]  [<ffffffffa0221ab5>] ? usbdev_mmap+0xa5/0x1b0 [usbcore]
> [ 1706.234880]  [<ffffffff813bbc25>] ? tty_insert_flip_string_fixed_flag+0x85/0xe0
> [ 1706.234885]  [<ffffffff8119af87>] ? mmap_region+0x3e7/0x660
> [ 1706.234888]  [<ffffffff8119b536>] ? do_mmap+0x336/0x420
> [ 1706.234892]  [<ffffffff8118213f>] ? vm_mmap_pgoff+0xaf/0xf0
> [ 1706.234895]  [<ffffffff811999dd>] ? SyS_mmap_pgoff+0x1ad/0x270
> [ 1706.234898]  [<ffffffff811d53b6>] ? SyS_write+0x76/0xc0
> [ 1706.234903]  [<ffffffff815829f2>] ? system_call_fast_compare_end+0xc/0x67

I understand there are some requirements on the allocation such that
large blocks are not always available. But what is the proper way to
determine the upper limit of the size such that the user can avoid
generating warnings like this? (Also, the application really wants to
be able to allocate large buffers, maybe tune swiotlb=?.)

Test results:

Basic testing with smaller buffer size shows the patch works well with
Kinect v2 (~260MB/s isochronous). Tests were performed on top of
4.4-rc8 with Debian config.

I'm still trying to verify whether this patch fixes page allocation
failure, but is having some trouble reproducing memory fragmentation.
I will later test it on the original machine which had the problem.

Regards,
Lingzhu

Re: [PATCH v2] Add support for usbfs zerocopy.

[Alan Stern]

On Wed, 6 Jan 2016, Lingzhu Xiang wrote:

> On Wed, Nov 25, 2015 at 7:19 PM, Steinar H. Gunderson <sesse@google.com> wrote:
> > Add a new interface for userspace to preallocate memory that can be
> > used with usbfs. This gives two primary benefits:
> 
> I got this when trying to allocate a little bit large buffer (~4MB)
> using the new userspace libusb_dev_mem_alloc():
> 
> > [ 1706.212407] usb 2-1.1: reset SuperSpeed USB device number 3 using xhci_hcd
> > [ 1706.234823] xhci_hcd 0000:00:14.0: swiotlb buffer is full (sz: 4325376 bytes)
> > [ 1706.234827] swiotlb: coherent allocation failed for device 0000:00:14.0 size=4325376
> > [ 1706.234830] CPU: 1 PID: 3233 Comm: Protonect Tainted: G     U  W       4.4.0-rc8-amd64 #1 Debian 4.4~rc8-1~exp1
> > [ 1706.234831] Hardware name: LENOVO 20ALCTO1WW/20ALCTO1WW, BIOS GIET76WW (2.26 ) 08/27/2014
> > [ 1706.234833]  0000000000000000 000000000f50c266 ffffffff812e6019 ffffffffffffffff
> > [ 1706.234836]  ffffffff8130dc45 ffff88020000000b 0000000000420000 ffffffff81a2a0e0
> > [ 1706.234838]  ffff880206263d80 0000000000000000 ffff88021c892f40 0000000000420040
> > [ 1706.234841] Call Trace:
> > [ 1706.234847]  [<ffffffff812e6019>] ? dump_stack+0x40/0x57
> > [ 1706.234851]  [<ffffffff8130dc45>] ? swiotlb_alloc_coherent+0x135/0x150
> > [ 1706.234867]  [<ffffffffa021deb1>] ? hcd_buffer_alloc+0xb1/0x130 [usbcore]
> > [ 1706.234875]  [<ffffffffa0221ab5>] ? usbdev_mmap+0xa5/0x1b0 [usbcore]
> > [ 1706.234880]  [<ffffffff813bbc25>] ? tty_insert_flip_string_fixed_flag+0x85/0xe0
> > [ 1706.234885]  [<ffffffff8119af87>] ? mmap_region+0x3e7/0x660
> > [ 1706.234888]  [<ffffffff8119b536>] ? do_mmap+0x336/0x420
> > [ 1706.234892]  [<ffffffff8118213f>] ? vm_mmap_pgoff+0xaf/0xf0
> > [ 1706.234895]  [<ffffffff811999dd>] ? SyS_mmap_pgoff+0x1ad/0x270
> > [ 1706.234898]  [<ffffffff811d53b6>] ? SyS_write+0x76/0xc0
> > [ 1706.234903]  [<ffffffff815829f2>] ? system_call_fast_compare_end+0xc/0x67
> 
> I understand there are some requirements on the allocation such that
> large blocks are not always available. But what is the proper way to
> determine the upper limit of the size such that the user can avoid
> generating warnings like this? (Also, the application really wants to
> be able to allocate large buffers, maybe tune swiotlb=?.)

It's debatable whether this should have generated a warning.  Why 
doesn't dma_alloc_coherent() simply fail silently?

Alan Stern

Re: [PATCH] Add support for usbfs zerocopy.

[Christoph Hellwig]

On Wed, Jan 06, 2016 at 03:45:12PM +0100, Steinar H. Gunderson wrote:
> On Tue, Jan 05, 2016 at 10:49:49PM -0800, Christoph Hellwig wrote:
> > This is a completely broken usage of the mmap interface.  if you use
> > mmap on a device file you must use the actual mmap for the data
> > transfer.
> 
> Really? V4L does exactly the same thing, from what I can see. It's just a way
> of allocating memory with specific properties, roughly similar to hugetlbfs.

V4l is generally speaking never an example for good practices.

Re: [PATCH] Add support for usbfs zerocopy.

[Christoph Hellwig]

On Wed, Jan 06, 2016 at 10:35:05AM -0500, Alan Stern wrote:
> Indeed, the I/O operations we are using with mmap here are not reads or 
> writes; they are ioctls.  As far as I know, the kernel doesn't have any 
> defined interface for zerocopy ioctls.

IF it was using mmap for I/O it would read in through the page fault
handler an then mark the page dirty for writeback by the VM.  Thats
clearly not the case.

Instead it's using mmap on a file as a pecial purpose anonymous
memory allocator, bypassing the VM and VM policies, including
allowing to pin kernel memory that way.

RE: [PATCH] Add support for usbfs zerocopy.

[David Laight]

From: Christoph Hellwig
> Sent: 08 January 2016 09:46
> On Wed, Jan 06, 2016 at 10:35:05AM -0500, Alan Stern wrote:
> > Indeed, the I/O operations we are using with mmap here are not reads or
> > writes; they are ioctls.  As far as I know, the kernel doesn't have any
> > defined interface for zerocopy ioctls.
> 
> IF it was using mmap for I/O it would read in through the page fault
> handler an then mark the page dirty for writeback by the VM.  Thats
> clearly not the case.

Indeed, and never is the case when mmap() is processed by a
driver rather than a filesystem.

> Instead it's using mmap on a file as a pecial purpose anonymous
> memory allocator, bypassing the VM and VM policies, including
> allowing to pin kernel memory that way.

Opening a driver often allocates kernel memory, not a big deal.

	David

Re: [PATCH] Add support for usbfs zerocopy.

[Oliver Neukum]

On Fri, 2016-01-08 at 01:45 -0800, Christoph Hellwig wrote:
> On Wed, Jan 06, 2016 at 10:35:05AM -0500, Alan Stern wrote:
> > Indeed, the I/O operations we are using with mmap here are not reads or 
> > writes; they are ioctls.  As far as I know, the kernel doesn't have any 
> > defined interface for zerocopy ioctls.
> 
> IF it was using mmap for I/O it would read in through the page fault
> handler an then mark the page dirty for writeback by the VM.  Thats
> clearly not the case.

That won't work because we need the ability to determine the chunk size
IO is done in. USB devices don't map to files, yet the memory they can
operate on depends on the device, so allocation in the kernel for
a specific device is a necessity.

	Regards
		Oliver

Re: [PATCH v2] Add support for usbfs zerocopy.

[Lingzhu Xiang]

On Wed, Nov 25, 2015 at 7:19 PM, Steinar H. Gunderson <sesse@google.com> wrote:
> Add a new interface for userspace to preallocate memory that can be
> used with usbfs. This gives two primary benefits:

Extended testing over one hour shows significant improved CPU usage
and stability:
The old CPU usage is about 12% +/- 5%. The new CPU usage is 4% +/- 1%.

The setup is an i7-4600U laptop running Kinect v2 with 260MB/s
isochronous and 20MB/s bulk transfers.

The improvement in reducing jitter is particularly preferable to
Kinect v2 driver because the isochronous data is structured and one
packet lost means discarding a lot of data.

Regards,
Lingzhu

Re: [PATCH v2] Add support for usbfs zerocopy.

[Steinar H. Gunderson]

On Fri, Jan 08, 2016 at 11:00:54PM -0500, Lingzhu Xiang wrote:
> Extended testing over one hour shows significant improved CPU usage
> and stability:
> The old CPU usage is about 12% +/- 5%. The new CPU usage is 4% +/- 1%.
> 
> The setup is an i7-4600U laptop running Kinect v2 with 260MB/s
> isochronous and 20MB/s bulk transfers.
> 
> The improvement in reducing jitter is particularly preferable to
> Kinect v2 driver because the isochronous data is structured and one
> packet lost means discarding a lot of data.

Thanks for testing this! It's not unexpected, but it's good to see others
are also seeing performance improvements.

/* Steinar */
-- 
Software Engineer, Google Switzerland

Re: [PATCH v2] Add support for usbfs zerocopy.

[Oliver Neukum]

On Thu, 2016-01-07 at 10:40 -0500, Alan Stern wrote:
> > I understand there are some requirements on the allocation such that
> > large blocks are not always available. But what is the proper way to
> > determine the upper limit of the size such that the user can avoid
> > generating warnings like this? (Also, the application really wants
> to
> > be able to allocate large buffers, maybe tune swiotlb=?.)
> 
> It's debatable whether this should have generated a warning.  Why 
> doesn't dma_alloc_coherent() simply fail silently?

I suspect many drivers to be unable to deal well with a failure.
Having this report makes "my device doesn't work" easier to solve
as a bug report.

Hence it seems to me that a driver which can handle a failure
with a good fallback should indicate this with a flag to the
VM layer.

	Regards
		Oliver
 

Re: [PATCH v2] Add support for usbfs zerocopy.

[Oliver Neukum]

On Mon, 2016-01-11 at 11:15 -0500, Konrad Rzeszutek Wilk wrote:
> On Mon, Jan 11, 2016 at 12:08:26PM +0100, Oliver Neukum wrote:

> > I suspect many drivers to be unable to deal well with a failure.
> > Having this report makes "my device doesn't work" easier to solve
> > as a bug report.
> > 
> > Hence it seems to me that a driver which can handle a failure
> > with a good fallback should indicate this with a flag to the
> > VM layer.
> 
> s/VM/device layer?

Actually no, if a driver does something that cannot be satisfied at all,
that should be logged. What we would like to go away is the log entry
due to a failure to allocate memory under memory pressure or
fragmentation.

	Regards
		Oliver

Re: [PATCH] Add support for usbfs zerocopy.

[Steinar H. Gunderson]

On Fri, Jan 08, 2016 at 01:45:35AM -0800, Christoph Hellwig wrote:
> IF it was using mmap for I/O it would read in through the page fault
> handler an then mark the page dirty for writeback by the VM.  Thats
> clearly not the case.
> 
> Instead it's using mmap on a file as a pecial purpose anonymous
> memory allocator, bypassing the VM and VM policies, including
> allowing to pin kernel memory that way.

FWIW, the allocated memory counts against the usbfs limits, so there's
no unbounded allocation opportunity here.

How do you suggest we proceed here? If mmap really is the wrong interface
(which is a bit frustrating after going through so many people :-) ),
what does the correct interface look like?

/* Steinar */
-- 
Software Engineer, Google Switzerland

Re: [PATCH] Add support for usbfs zerocopy.

[Alan Stern]

On Tue, 12 Jan 2016, Steinar H. Gunderson wrote:

> On Fri, Jan 08, 2016 at 01:45:35AM -0800, Christoph Hellwig wrote:
> > IF it was using mmap for I/O it would read in through the page fault
> > handler an then mark the page dirty for writeback by the VM.  Thats
> > clearly not the case.
> > 
> > Instead it's using mmap on a file as a pecial purpose anonymous
> > memory allocator, bypassing the VM and VM policies, including
> > allowing to pin kernel memory that way.
> 
> FWIW, the allocated memory counts against the usbfs limits, so there's
> no unbounded allocation opportunity here.
> 
> How do you suggest we proceed here? If mmap really is the wrong interface
> (which is a bit frustrating after going through so many people :-) ),
> what does the correct interface look like?

To me (and others on the mailing list), it appears that Christoph was
thinking of mmap as applied to a normal file, whereas the patch
concerns mmap applied to a device file.  One need not behave like
the other, which means the criticism was inappropriate.

Unless there are any other issues connected to this, I'm okay with the 
current version of the patch.

Alan Stern

Re: [PATCH] Add support for usbfs zerocopy.

[Lingzhu Xiang]

On Tue, Jan 12, 2016 at 5:05 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> Unless there are any other issues connected to this, I'm okay with the
> current version of the patch.

Any chance of this getting merged for 4.5?

Tested-by: Lingzhu Xiang <lingzhu.xiang@mail.utoronto.ca>

ACKs?

Regards,
Lingzhu

Re: [PATCH] Add support for usbfs zerocopy.

[Greg Kroah-Hartman]

On Wed, Jan 13, 2016 at 01:59:54AM -0500, Lingzhu Xiang wrote:
> On Tue, Jan 12, 2016 at 5:05 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> > Unless there are any other issues connected to this, I'm okay with the
> > current version of the patch.
> 
> Any chance of this getting merged for 4.5?

No way, it's too late and hasn't been in linux-next for a while.  I will
merge this to my tree after 4.5-rc1 is out to give it lots of time for
testing.

thanks,

greg k-h

Re: [PATCH v2] Add support for usbfs zerocopy.

[Greg Kroah-Hartman]

On Thu, Nov 26, 2015 at 01:19:13AM +0100, Steinar H. Gunderson wrote:
> Add a new interface for userspace to preallocate memory that can be
> used with usbfs. This gives two primary benefits:

Something is really wrong with your email client, it is saying this is
sent on Nov 26, the same exact time as your previous patch, yet you sent
this in January.  Which implies that this is an old patch and not an
updated one :(

Can you send me the latest verison of this, with the date set properly
on your machine for it?

thanks,

greg k-h

Re: [PATCH v2] Add support for usbfs zerocopy.

[Steinar H. Gunderson]

[-- Attachment #1: Type: text/plain, Size: 670 bytes --]

On Sun, Jan 24, 2016 at 01:12:08PM -0800, Greg Kroah-Hartman wrote:
> Something is really wrong with your email client, it is saying this is
> sent on Nov 26, the same exact time as your previous patch, yet you sent
> this in January.  Which implies that this is an old patch and not an
> updated one :(

It comes directly from git format-patch. I guess git commit --amend doesn't
properly reset the date?

> Can you send me the latest verison of this, with the date set properly
> on your machine for it?

I did git rebase --ignore-date HEAD^ just to reset the date. Sending it as an
attachment just to be sure.

/* Steinar */
-- 
Software Engineer, Google Switzerland

[-- Attachment #2: v3-0001-Add-support-for-usbfs-zerocopy.patch --]
[-- Type: text/x-diff, Size: 12211 bytes --]

>From e56d9235b343c5e70061e977639cc7dddeae8164 Mon Sep 17 00:00:00 2001
From: "Steinar H. Gunderson" <sesse@google.com>
Date: Mon, 25 Jan 2016 09:02:34 +0100
Subject: [PATCH v3] 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 all kinds of transfers (even if
   nonsensical for control and interrupt 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.

There's a new capability USBDEVFS_CAP_MMAP that userspace can query to see
if the running kernel supports this functionality, if just trying mmap() is
not acceptable.

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@google.com>
Signed-off-by: Markus Rechberger <mrechberger@gmail.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
---
 drivers/usb/core/devio.c          | 227 +++++++++++++++++++++++++++++++++-----
 include/uapi/linux/usbdevice_fs.h |   1 +
 2 files changed, 203 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 38ae877c..0238c78 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,111 @@ static int connected(struct usb_dev_state *ps)
 			ps->dev->state != USB_STATE_NOTATTACHED);
 }
 
+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(&usbm->memlist);
+		spin_unlock_irqrestore(&ps->lock, flags);
+
+		usb_free_coherent(ps->dev, usbm->size, usbm->mem,
+				usbm->dma_handle);
+		usbfs_decrease_memory_usage(
+			usbm->size + sizeof(struct usb_memory));
+		kfree(usbm);
+	} else {
+		spin_unlock_irqrestore(&ps->lock, flags);
+	}
+}
+
+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;
+	void *mem;
+	unsigned long flags;
+	dma_addr_t dma_handle;
+	int ret;
+
+	ret = usbfs_increase_memory_usage(size + sizeof(struct usb_memory));
+	if (ret)
+		goto error;
+
+	usbm = kzalloc(sizeof(struct usb_memory), GFP_KERNEL);
+	if (!usbm) {
+		ret = -ENOMEM;
+		goto error_decrease_mem;
+	}
+
+	mem = usb_alloc_coherent(ps->dev, size, GFP_USER, &dma_handle);
+	if (!mem) {
+		ret = -ENOMEM;
+		goto error_free_usbm;
+	}
+
+	memset(mem, 0, size);
+
+	usbm->mem = mem;
+	usbm->dma_handle = dma_handle;
+	usbm->size = size;
+	usbm->ps = ps;
+	usbm->vm_start = vma->vm_start;
+	usbm->vma_use_count = 1;
+	INIT_LIST_HEAD(&usbm->memlist);
+
+	if (remap_pfn_range(vma, vma->vm_start,
+			virt_to_phys(usbm->mem) >> PAGE_SHIFT,
+			size, vma->vm_page_prot) < 0) {
+		dec_usb_memory_use_count(usbm, &usbm->vma_use_count);
+		return -EAGAIN;
+	}
+
+	vma->vm_flags |= VM_IO;
+	vma->vm_flags |= (VM_DONTEXPAND | VM_DONTDUMP);
+	vma->vm_ops = &usbdev_vm_ops;
+	vma->vm_private_data = usbm;
+
+	spin_lock_irqsave(&ps->lock, flags);
+	list_add_tail(&usbm->memlist, &ps->memory_list);
+	spin_unlock_irqrestore(&ps->lock, flags);
+
+	return 0;
+
+error_free_usbm:
+	kfree(usbm);
+error_decrease_mem:
+	usbfs_decrease_memory_usage(size + sizeof(struct usb_memory));
+error:
+	return ret;
+}
+
 static loff_t usbdev_lseek(struct file *file, loff_t offset, int orig)
 {
 	loff_t ret;
@@ -297,8 +416,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 +1034,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 +1087,7 @@ static int usbdev_release(struct inode *inode, struct file *file)
 		free_async(as);
 		as = async_getcompleted(ps);
 	}
+
 	kfree(ps);
 	return 0;
 }
@@ -1283,6 +1409,31 @@ static int proc_setconfig(struct usb_dev_state *ps, void __user *arg)
 	return status;
 }
 
+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) {
+			if (uurb->buffer_length > iter->vm_start + iter->size -
+					uurb_start) {
+				usbm = ERR_PTR(-EINVAL);
+			} else {
+				usbm = iter;
+				usbm->urb_use_count++;
+			}
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&ps->lock, flags);
+	return usbm;
+}
+
 static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb,
 			struct usbdevfs_iso_packet_desc __user *iso_frame_desc,
 			void __user *arg)
@@ -1439,6 +1590,19 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb
 		goto error;
 	}
 
+	as->usbm = find_memory_area(ps, uurb);
+	if (IS_ERR(as->usbm)) {
+		ret = PTR_ERR(as->usbm);
+		as->usbm = NULL;
+		goto error;
+	}
+
+	/* do not use SG buffers when memory mapped segments
+	 * are in use
+	 */
+	if (as->usbm)
+		num_sgs = 0;
+
 	u += sizeof(struct async) + sizeof(struct urb) + uurb->buffer_length +
 	     num_sgs * sizeof(struct scatterlist);
 	ret = usbfs_increase_memory_usage(u);
@@ -1476,29 +1640,35 @@ 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,
-				GFP_KERNEL);
-		if (!as->urb->transfer_buffer) {
-			ret = -ENOMEM;
-			goto error;
-		}
+		if (as->usbm) {
+			unsigned long uurb_start = (unsigned long)uurb->buffer;
 
-		if (!is_in) {
-			if (copy_from_user(as->urb->transfer_buffer,
-					   uurb->buffer,
-					   uurb->buffer_length)) {
-				ret = -EFAULT;
+			as->urb->transfer_buffer = as->usbm->mem +
+					(uurb_start - as->usbm->vm_start);
+		} else {
+			as->urb->transfer_buffer = kmalloc(uurb->buffer_length,
+					GFP_KERNEL);
+			if (!as->urb->transfer_buffer) {
+				ret = -ENOMEM;
 				goto error;
 			}
-		} else if (uurb->type == USBDEVFS_URB_TYPE_ISO) {
-			/*
-			 * Isochronous input data may end up being
-			 * discontiguous if some of the packets are short.
-			 * Clear the buffer so that the gaps don't leak
-			 * kernel data to userspace.
-			 */
-			memset(as->urb->transfer_buffer, 0,
-					uurb->buffer_length);
+			if (!is_in) {
+				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) {
+				/*
+				 * Isochronous input data may end up being
+				 * discontiguous if some of the packets are
+				 * short. Clear the buffer so that the gaps
+				 * don't leak kernel data to userspace.
+				 */
+				memset(as->urb->transfer_buffer, 0,
+						uurb->buffer_length);
+			}
 		}
 	}
 	as->urb->dev = ps->dev;
@@ -1545,10 +1715,14 @@ 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 (as->usbm) {
+		unsigned long uurb_start = (unsigned long)uurb->buffer;
+
+		as->urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+		as->urb->transfer_dma = as->usbm->dma_handle +
+				(uurb_start - as->usbm->vm_start);
+	} else if (is_in && uurb->buffer_length > 0)
 		as->userbuffer = uurb->buffer;
-	else
-		as->userbuffer = NULL;
 	as->signr = uurb->signr;
 	as->ifnum = ifnum;
 	as->pid = get_pid(task_pid(current));
@@ -1604,6 +1778,8 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb
 	return 0;
 
  error:
+	if (as && as->usbm)
+		dec_usb_memory_use_count(as->usbm, &as->usbm->urb_use_count);
 	kfree(isopkt);
 	kfree(dr);
 	if (as)
@@ -2047,7 +2223,7 @@ static int proc_get_capabilities(struct usb_dev_state *ps, void __user *arg)
 	__u32 caps;
 
 	caps = USBDEVFS_CAP_ZERO_PACKET | USBDEVFS_CAP_NO_PACKET_SIZE_LIM |
-			USBDEVFS_CAP_REAP_AFTER_DISCONNECT;
+			USBDEVFS_CAP_REAP_AFTER_DISCONNECT | USBDEVFS_CAP_MMAP;
 	if (!ps->dev->bus->no_stop_on_short)
 		caps |= USBDEVFS_CAP_BULK_CONTINUATION;
 	if (ps->dev->bus->sg_tablesize)
@@ -2373,6 +2549,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,
 };
diff --git a/include/uapi/linux/usbdevice_fs.h b/include/uapi/linux/usbdevice_fs.h
index 019ba1e..ecbd176 100644
--- a/include/uapi/linux/usbdevice_fs.h
+++ b/include/uapi/linux/usbdevice_fs.h
@@ -134,6 +134,7 @@ struct usbdevfs_hub_portinfo {
 #define USBDEVFS_CAP_NO_PACKET_SIZE_LIM		0x04
 #define USBDEVFS_CAP_BULK_SCATTER_GATHER	0x08
 #define USBDEVFS_CAP_REAP_AFTER_DISCONNECT	0x10
+#define USBDEVFS_CAP_MMAP			0x20
 
 /* USBDEVFS_DISCONNECT_CLAIM flags & struct */
 
-- 
2.1.4

Re: [PATCH v2] Add support for usbfs zerocopy.

[Steinar H. Gunderson]

On Mon, Jan 25, 2016 at 09:03:57AM +0100, Steinar H. Gunderson wrote:
> I did git rebase --ignore-date HEAD^ just to reset the date. Sending it as an
> attachment just to be sure.

Hi Greg,

Did this work for you? Is there anything else I should do to this patch?

/* Steinar */
-- 
Software Engineer, Google Switzerland

Re: [PATCH v2] Add support for usbfs zerocopy.

[Greg Kroah-Hartman]

On Mon, Jan 25, 2016 at 09:03:57AM +0100, Steinar H. Gunderson wrote:
> On Sun, Jan 24, 2016 at 01:12:08PM -0800, Greg Kroah-Hartman wrote:
> > Something is really wrong with your email client, it is saying this is
> > sent on Nov 26, the same exact time as your previous patch, yet you sent
> > this in January.  Which implies that this is an old patch and not an
> > updated one :(
> 
> It comes directly from git format-patch. I guess git commit --amend doesn't
> properly reset the date?

I guess not, odd.

> > Can you send me the latest verison of this, with the date set properly
> > on your machine for it?
> 
> I did git rebase --ignore-date HEAD^ just to reset the date. Sending it as an
> attachment just to be sure.

Attachments don't work, you know better than that :(

I need a _real_ email that I don't have to hand-edit to get it to apply.

thanks,

greg k-h

[PATCH v4] Add support for usbfs zerocopy.

[Steinar H. Gunderson]

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 all kinds of transfers (even if
   nonsensical for control and interrupt 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.

There's a new capability USBDEVFS_CAP_MMAP that userspace can query to see
if the running kernel supports this functionality, if just trying mmap() is
not acceptable.

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@google.com>
Signed-off-by: Markus Rechberger <mrechberger@gmail.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
---
 drivers/usb/core/devio.c          | 227 +++++++++++++++++++++++++++++++++-----
 include/uapi/linux/usbdevice_fs.h |   1 +
 2 files changed, 203 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 59e7a33..a9fc6ff 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;
@@ -162,6 +176,111 @@ static int connected(struct usb_dev_state *ps)
 			ps->dev->state != USB_STATE_NOTATTACHED);
 }
 
+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(&usbm->memlist);
+		spin_unlock_irqrestore(&ps->lock, flags);
+
+		usb_free_coherent(ps->dev, usbm->size, usbm->mem,
+				usbm->dma_handle);
+		usbfs_decrease_memory_usage(
+			usbm->size + sizeof(struct usb_memory));
+		kfree(usbm);
+	} else {
+		spin_unlock_irqrestore(&ps->lock, flags);
+	}
+}
+
+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;
+	void *mem;
+	unsigned long flags;
+	dma_addr_t dma_handle;
+	int ret;
+
+	ret = usbfs_increase_memory_usage(size + sizeof(struct usb_memory));
+	if (ret)
+		goto error;
+
+	usbm = kzalloc(sizeof(struct usb_memory), GFP_KERNEL);
+	if (!usbm) {
+		ret = -ENOMEM;
+		goto error_decrease_mem;
+	}
+
+	mem = usb_alloc_coherent(ps->dev, size, GFP_USER, &dma_handle);
+	if (!mem) {
+		ret = -ENOMEM;
+		goto error_free_usbm;
+	}
+
+	memset(mem, 0, size);
+
+	usbm->mem = mem;
+	usbm->dma_handle = dma_handle;
+	usbm->size = size;
+	usbm->ps = ps;
+	usbm->vm_start = vma->vm_start;
+	usbm->vma_use_count = 1;
+	INIT_LIST_HEAD(&usbm->memlist);
+
+	if (remap_pfn_range(vma, vma->vm_start,
+			virt_to_phys(usbm->mem) >> PAGE_SHIFT,
+			size, vma->vm_page_prot) < 0) {
+		dec_usb_memory_use_count(usbm, &usbm->vma_use_count);
+		return -EAGAIN;
+	}
+
+	vma->vm_flags |= VM_IO;
+	vma->vm_flags |= (VM_DONTEXPAND | VM_DONTDUMP);
+	vma->vm_ops = &usbdev_vm_ops;
+	vma->vm_private_data = usbm;
+
+	spin_lock_irqsave(&ps->lock, flags);
+	list_add_tail(&usbm->memlist, &ps->memory_list);
+	spin_unlock_irqrestore(&ps->lock, flags);
+
+	return 0;
+
+error_free_usbm:
+	kfree(usbm);
+error_decrease_mem:
+	usbfs_decrease_memory_usage(size + sizeof(struct usb_memory));
+error:
+	return ret;
+}
+
 static ssize_t usbdev_read(struct file *file, char __user *buf, size_t nbytes,
 			   loff_t *ppos)
 {
@@ -278,8 +397,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);
@@ -893,6 +1017,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));
@@ -945,6 +1070,7 @@ static int usbdev_release(struct inode *inode, struct file *file)
 		free_async(as);
 		as = async_getcompleted(ps);
 	}
+
 	kfree(ps);
 	return 0;
 }
@@ -1266,6 +1392,31 @@ static int proc_setconfig(struct usb_dev_state *ps, void __user *arg)
 	return status;
 }
 
+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) {
+			if (uurb->buffer_length > iter->vm_start + iter->size -
+					uurb_start) {
+				usbm = ERR_PTR(-EINVAL);
+			} else {
+				usbm = iter;
+				usbm->urb_use_count++;
+			}
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&ps->lock, flags);
+	return usbm;
+}
+
 static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb,
 			struct usbdevfs_iso_packet_desc __user *iso_frame_desc,
 			void __user *arg)
@@ -1422,6 +1573,19 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb
 		goto error;
 	}
 
+	as->usbm = find_memory_area(ps, uurb);
+	if (IS_ERR(as->usbm)) {
+		ret = PTR_ERR(as->usbm);
+		as->usbm = NULL;
+		goto error;
+	}
+
+	/* do not use SG buffers when memory mapped segments
+	 * are in use
+	 */
+	if (as->usbm)
+		num_sgs = 0;
+
 	u += sizeof(struct async) + sizeof(struct urb) + uurb->buffer_length +
 	     num_sgs * sizeof(struct scatterlist);
 	ret = usbfs_increase_memory_usage(u);
@@ -1459,29 +1623,35 @@ 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,
-				GFP_KERNEL);
-		if (!as->urb->transfer_buffer) {
-			ret = -ENOMEM;
-			goto error;
-		}
+		if (as->usbm) {
+			unsigned long uurb_start = (unsigned long)uurb->buffer;
 
-		if (!is_in) {
-			if (copy_from_user(as->urb->transfer_buffer,
-					   uurb->buffer,
-					   uurb->buffer_length)) {
-				ret = -EFAULT;
+			as->urb->transfer_buffer = as->usbm->mem +
+					(uurb_start - as->usbm->vm_start);
+		} else {
+			as->urb->transfer_buffer = kmalloc(uurb->buffer_length,
+					GFP_KERNEL);
+			if (!as->urb->transfer_buffer) {
+				ret = -ENOMEM;
 				goto error;
 			}
-		} else if (uurb->type == USBDEVFS_URB_TYPE_ISO) {
-			/*
-			 * Isochronous input data may end up being
-			 * discontiguous if some of the packets are short.
-			 * Clear the buffer so that the gaps don't leak
-			 * kernel data to userspace.
-			 */
-			memset(as->urb->transfer_buffer, 0,
-					uurb->buffer_length);
+			if (!is_in) {
+				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) {
+				/*
+				 * Isochronous input data may end up being
+				 * discontiguous if some of the packets are
+				 * short. Clear the buffer so that the gaps
+				 * don't leak kernel data to userspace.
+				 */
+				memset(as->urb->transfer_buffer, 0,
+						uurb->buffer_length);
+			}
 		}
 	}
 	as->urb->dev = ps->dev;
@@ -1528,10 +1698,14 @@ 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 (as->usbm) {
+		unsigned long uurb_start = (unsigned long)uurb->buffer;
+
+		as->urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+		as->urb->transfer_dma = as->usbm->dma_handle +
+				(uurb_start - as->usbm->vm_start);
+	} else if (is_in && uurb->buffer_length > 0)
 		as->userbuffer = uurb->buffer;
-	else
-		as->userbuffer = NULL;
 	as->signr = uurb->signr;
 	as->ifnum = ifnum;
 	as->pid = get_pid(task_pid(current));
@@ -1587,6 +1761,8 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb
 	return 0;
 
  error:
+	if (as && as->usbm)
+		dec_usb_memory_use_count(as->usbm, &as->usbm->urb_use_count);
 	kfree(isopkt);
 	kfree(dr);
 	if (as)
@@ -2040,7 +2216,7 @@ static int proc_get_capabilities(struct usb_dev_state *ps, void __user *arg)
 	__u32 caps;
 
 	caps = USBDEVFS_CAP_ZERO_PACKET | USBDEVFS_CAP_NO_PACKET_SIZE_LIM |
-			USBDEVFS_CAP_REAP_AFTER_DISCONNECT;
+			USBDEVFS_CAP_REAP_AFTER_DISCONNECT | USBDEVFS_CAP_MMAP;
 	if (!ps->dev->bus->no_stop_on_short)
 		caps |= USBDEVFS_CAP_BULK_CONTINUATION;
 	if (ps->dev->bus->sg_tablesize)
@@ -2366,6 +2542,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,
 };
diff --git a/include/uapi/linux/usbdevice_fs.h b/include/uapi/linux/usbdevice_fs.h
index 019ba1e..ecbd176 100644
--- a/include/uapi/linux/usbdevice_fs.h
+++ b/include/uapi/linux/usbdevice_fs.h
@@ -134,6 +134,7 @@ struct usbdevfs_hub_portinfo {
 #define USBDEVFS_CAP_NO_PACKET_SIZE_LIM		0x04
 #define USBDEVFS_CAP_BULK_SCATTER_GATHER	0x08
 #define USBDEVFS_CAP_REAP_AFTER_DISCONNECT	0x10
+#define USBDEVFS_CAP_MMAP			0x20
 
 /* USBDEVFS_DISCONNECT_CLAIM flags & struct */
 
-- 
2.1.4

Re: [PATCH v2] Add support for usbfs zerocopy.

[Steinar H. Gunderson]

On Wed, Feb 03, 2016 at 01:23:17PM -0800, Greg Kroah-Hartman wrote:
> Attachments don't work, you know better than that :(

Since I've now been bitten by this several times: Is there any sort of best
practice for integrating git with MUAs? What I'm doing right now is
cut-and-paste from mutt to get the to/cc/in-reply-to headers right, and
that's suboptimal. It feels like it should be a FAQ, but I can't really find
anything obvious.

> I need a _real_ email that I don't have to hand-edit to get it to apply.

Trying again; sending v4 as a reply to your email. This is on top of git
master, since the patch no longer applied there. If you want it against any
other tree, please let me know which.

/* Steinar */
-- 
Software Engineer, Google Switzerland

Re: [PATCH v2] Add support for usbfs zerocopy.

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 828 bytes --]

"Steinar H. Gunderson" <sesse@google.com> writes:

> On Wed, Feb 03, 2016 at 01:23:17PM -0800, Greg Kroah-Hartman wrote:
>> Attachments don't work, you know better than that :(
>
> Since I've now been bitten by this several times: Is there any sort of best
> practice for integrating git with MUAs? What I'm doing right now is
> cut-and-paste from mutt to get the to/cc/in-reply-to headers right, and
> that's suboptimal. It feels like it should be a FAQ, but I can't really find
> anything obvious.

have you tried git send-email ? If you wanna send as a reply to another
message, use:

  $ git send-email --to foo@bar.com --cc bar@foo.com --in-reply-to=abcd-message-id@foo.bar.com

note that git send-email can read mutt aliases. It's all documented in
the man page:

  $ git help send-email

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH v2] Add support for usbfs zerocopy.

[Steinar H. Gunderson]

On Thu, Feb 04, 2016 at 12:15:50AM +0200, Felipe Balbi wrote:
>> Since I've now been bitten by this several times: Is there any sort of best
>> practice for integrating git with MUAs? What I'm doing right now is
>> cut-and-paste from mutt to get the to/cc/in-reply-to headers right, and
>> that's suboptimal. It feels like it should be a FAQ, but I can't really find
>> anything obvious.
> have you tried git send-email ? If you wanna send as a reply to another
> message, use:
> 
>   $ git send-email --to foo@bar.com --cc bar@foo.com --in-reply-to=abcd-message-id@foo.bar.com

Unfortunately that doesn't change anything; I still need to cut-and-paste the
to, cc and in-reply-to manually. What I'd want is something like
“git send-email as a reply to the last email in my outbox”.

Attaching the patch gives me exactly this, by the way, but seemingly ends up
with a format that's more cumbersome to receive (which is a bad tradeoff).

/* Steinar */
-- 
Software Engineer, Google Switzerland

Re: [PATCH v2] Add support for usbfs zerocopy.

[Bjørn Mork]

"Steinar H. Gunderson" <sesse@google.com> writes:
> On Thu, Feb 04, 2016 at 12:15:50AM +0200, Felipe Balbi wrote:
>>> Since I've now been bitten by this several times: Is there any sort of best
>>> practice for integrating git with MUAs? What I'm doing right now is
>>> cut-and-paste from mutt to get the to/cc/in-reply-to headers right, and
>>> that's suboptimal. It feels like it should be a FAQ, but I can't really find
>>> anything obvious.
>> have you tried git send-email ? If you wanna send as a reply to another
>> message, use:
>> 
>>   $ git send-email --to foo@bar.com --cc bar@foo.com --in-reply-to=abcd-message-id@foo.bar.com
>
> Unfortunately that doesn't change anything; I still need to cut-and-paste the
> to, cc and in-reply-to manually. What I'd want is something like
> “git send-email as a reply to the last email in my outbox”.
>
> Attaching the patch gives me exactly this, by the way, but seemingly ends up
> with a format that's more cumbersome to receive (which is a bad tradeoff).

Then use Mutt to reply, but include the patch inline instead of
attaching it.  I believe this is discussed in the Mutt section of
Documentation/email-clients.txt



Bjørn

Re: [PATCH v2] Add support for usbfs zerocopy.

[Steinar H. Gunderson]

On Thu, Feb 04, 2016 at 11:17:26AM +0100, Bjørn Mork wrote:
> Then use Mutt to reply, but include the patch inline instead of
> attaching it.  I believe this is discussed in the Mutt section of
> Documentation/email-clients.txt

Thanks; if that works (even though it changes the “From” line and such)
that's a good solution. It doesn't work for series of multiple patches, but I
can certainly live with that.

/* Steinar */
-- 
Software Engineer, Google Switzerland

Re: [PATCH v2] Add support for usbfs zerocopy.

[Steinar H. Gunderson]

On Wed, Feb 03, 2016 at 11:09:16PM +0100, Steinar H. Gunderson wrote:
> Trying again; sending v4 as a reply to your email.

Did the v4 sending work for you?

/* Steinar */
-- 
Software Engineer, Google Switzerland

Re: [PATCH] Add support for usbfs zerocopy.

[Sasha Levin]

On 11/25/2015 07:19 PM, Steinar H. Gunderson wrote:
> 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 all kinds of transfers (even if
>    nonsensical for control and interrupt 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.
> 
> There's a new capability USBDEVFS_CAP_MMAP that userspace can query to see
> if the running kernel supports this functionality, if just trying mmap() is
> not acceptable.
> 
> 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@google.com>
> Signed-off-by: Markus Rechberger <mrechberger@gmail.com>
> Acked-by: Alan Stern <stern@rowland.harvard.edu>

Hi,

I'm seeing the following warning while fuzzing:

[ 1595.188189] WARNING: CPU: 3 PID: 26063 at mm/page_alloc.c:3207 __alloc_pages_nodemask+0x960/0x29e0()

[ 1595.188287] Modules linked in:

[ 1595.188316] CPU: 3 PID: 26063 Comm: syz-executor Not tainted 4.5.0-rc5-next-20160223-sasha-00022-g03b30f1-dirty #2982

[ 1595.188362]  1ffff1001460ce89 ffff8800a30674d0 ffffffffa03cecad ffffffff00000003

[ 1595.188380]  fffffbfff5829420 0000000041b58ab3 ffffffffabb334e2 ffffffffa03ceb15

[ 1595.188395]  ffffffff9e5964c0 0000000041b58ab3 ffffffffabb4e542 ffffffff9e6ee830

[ 1595.188401] Call Trace:

[ 1595.188445] dump_stack (lib/dump_stack.c:53)
[ 1595.188529] warn_slowpath_common (kernel/panic.c:483)
[ 1595.188552] warn_slowpath_null (kernel/panic.c:517)
[ 1595.188561] __alloc_pages_nodemask (mm/page_alloc.c:3207 mm/page_alloc.c:3467)
[ 1595.188768] alloc_pages_current (mm/mempolicy.c:2088)
[ 1595.188784] alloc_kmem_pages (mm/page_alloc.c:3648)
[ 1595.188802] kmalloc_order (mm/slab_common.c:1014)
[ 1595.188819] __kmalloc (include/linux/slab.h:397 include/linux/slab.h:404 mm/slub.c:3573)
[ 1595.188870] hcd_buffer_alloc (include/linux/slab.h:477 drivers/usb/core/buffer.c:130)
[ 1595.188934] usb_alloc_coherent (drivers/usb/core/usb.c:736)
[ 1595.188941] usbdev_mmap (drivers/usb/core/devio.c:243)
[ 1595.189014] mmap_region (mm/mmap.c:1502)
[ 1595.189335] do_mmap (mm/mmap.c:1282)
[ 1595.189352] vm_mmap_pgoff (mm/util.c:335)
[ 1595.189384] SyS_mmap_pgoff (mm/mmap.c:1331 mm/mmap.c:1289)
[ 1595.189429] SyS_mmap (arch/x86/kernel/sys_x86_64.c:86)
[ 1595.189437] entry_SYSCALL_64_fastpath (arch/x86/entry/entry_64.S:200)


Thanks,
Sasha

Re: [PATCH] Add support for usbfs zerocopy.

[Steinar H. Gunderson]

On Wed, Feb 24, 2016 at 02:30:08PM -0500, Sasha Levin wrote:
> I'm seeing the following warning while fuzzing:
> [ 1595.188189] WARNING: CPU: 3 PID: 26063 at mm/page_alloc.c:3207 __alloc_pages_nodemask+0x960/0x29e0()
> [ 1595.188287] Modules linked in:
> [ 1595.188316] CPU: 3 PID: 26063 Comm: syz-executor Not tainted 4.5.0-rc5-next-20160223-sasha-00022-g03b30f1-dirty #2982

I think it was already established that one could cause kernel warnings if
trying to allocate large amounts of memory, but that the usbfs memory limits
could curb truly dangerous amounts. Someone please correct me if I'm
misunderstanding?

/* Steinar */
-- 
Software Engineer, Google Switzerland