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

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

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 
> Signed-off-by: Markus Rechberger 
> Acked-by: Alan Stern 

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]  11001460ce89 8800a30674d0 a03cecad 
0003

[ 1595.188380]  fbfff5829420 41b58ab3 abb334e2 
a03ceb15

[ 1595.188395]  9e5964c0 41b58ab3 abb4e542 
9e6ee830

[ 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.

[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 
> Signed-off-by: Markus Rechberger 
> Acked-by: Alan Stern 

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]  11001460ce89 8800a30674d0 a03cecad 
0003

[ 1595.188380]  fbfff5829420 41b58ab3 abb334e2 
a03ceb15

[ 1595.188395]  9e5964c0 41b58ab3 abb4e542 
9e6ee830

[ 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.

[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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[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 
Signed-off-by: Markus Rechberger 
Acked-by: Alan Stern 
---
 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 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -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(>lock, flags);
+   --*count;
+   if (usbm->urb_use_count == 0 && usbm->vma_use_count == 0) {
+   list_del(>memlist);
+   spin_unlock_irqrestore(>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(>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(>ps->lock, flags);
+   ++usbm->vma_use_count;
+   spin_unlock_irqrestore(>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, >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), 

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[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 
Signed-off-by: Markus Rechberger 
Acked-by: Alan Stern 
---
 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 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -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(>lock, flags);
+   --*count;
+   if (usbm->urb_use_count == 0 && usbm->vma_use_count == 0) {
+   list_del(>memlist);
+   spin_unlock_irqrestore(>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(>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(>ps->lock, flags);
+   ++usbm->vma_use_count;
+   spin_unlock_irqrestore(>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, >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)
+