From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk0-f177.google.com (mail-qk0-f177.google.com [209.85.220.177]) by kanga.kvack.org (Postfix) with ESMTP id 62D3D6B0006 for ; Tue, 5 Jan 2016 11:27:47 -0500 (EST) Received: by mail-qk0-f177.google.com with SMTP id q19so87048000qke.3 for ; Tue, 05 Jan 2016 08:27:47 -0800 (PST) Received: from iolanthe.rowland.org (iolanthe.rowland.org. [192.131.102.54]) by mx.google.com with SMTP id a192si25758281qkb.70.2016.01.05.08.27.46 for ; Tue, 05 Jan 2016 08:27:46 -0800 (PST) Date: Tue, 5 Jan 2016 11:27:45 -0500 (EST) From: Alan Stern Subject: Does vm_operations_struct require a .owner field? In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-linux-mm@kvack.org List-ID: To: linux-mm@kvack.org Cc: Kernel development list , David Laight , "'Steinar H. Gunderson'" , "linux-usb@vger.kernel.org" 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 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f41.google.com (mail-wm0-f41.google.com [74.125.82.41]) by kanga.kvack.org (Postfix) with ESMTP id DAE1F6B0006 for ; Tue, 5 Jan 2016 15:58:15 -0500 (EST) Received: by mail-wm0-f41.google.com with SMTP id f206so47892450wmf.0 for ; Tue, 05 Jan 2016 12:58:15 -0800 (PST) Received: from mail-wm0-x231.google.com (mail-wm0-x231.google.com. [2a00:1450:400c:c09::231]) by mx.google.com with ESMTPS id ci1si154652187wjc.27.2016.01.05.12.58.14 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 05 Jan 2016 12:58:14 -0800 (PST) Received: by mail-wm0-x231.google.com with SMTP id u188so38112247wmu.1 for ; Tue, 05 Jan 2016 12:58:14 -0800 (PST) Date: Tue, 5 Jan 2016 22:58:12 +0200 From: "Kirill A. Shutemov" Subject: Re: Does vm_operations_struct require a .owner field? Message-ID: <20160105205812.GA24738@node.shutemov.name> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: owner-linux-mm@kvack.org List-ID: To: Alan Stern Cc: linux-mm@kvack.org, Kernel development list , David Laight , "'Steinar H. Gunderson'" , "linux-usb@vger.kernel.org" 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 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qg0-f42.google.com (mail-qg0-f42.google.com [209.85.192.42]) by kanga.kvack.org (Postfix) with ESMTP id 3C8F36B0003 for ; Tue, 5 Jan 2016 16:31:11 -0500 (EST) Received: by mail-qg0-f42.google.com with SMTP id e32so198803878qgf.3 for ; Tue, 05 Jan 2016 13:31:11 -0800 (PST) Received: from iolanthe.rowland.org (iolanthe.rowland.org. [192.131.102.54]) by mx.google.com with SMTP id t37si65527323qgt.88.2016.01.05.13.31.10 for ; Tue, 05 Jan 2016 13:31:10 -0800 (PST) Date: Tue, 5 Jan 2016 16:31:09 -0500 (EST) From: Alan Stern Subject: Re: Does vm_operations_struct require a .owner field? In-Reply-To: <20160105205812.GA24738@node.shutemov.name> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-linux-mm@kvack.org List-ID: To: "Kirill A. Shutemov" Cc: linux-mm@kvack.org, Kernel development list , David Laight , "'Steinar H. Gunderson'" , "linux-usb@vger.kernel.org" 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 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f53.google.com (mail-wm0-f53.google.com [74.125.82.53]) by kanga.kvack.org (Postfix) with ESMTP id 9BFD4800C7 for ; Tue, 5 Jan 2016 18:54:26 -0500 (EST) Received: by mail-wm0-f53.google.com with SMTP id f206so52937413wmf.0 for ; Tue, 05 Jan 2016 15:54:26 -0800 (PST) Received: from mail-wm0-x22c.google.com (mail-wm0-x22c.google.com. [2a00:1450:400c:c09::22c]) by mx.google.com with ESMTPS id i9si128540445wjf.175.2016.01.05.15.54.25 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 05 Jan 2016 15:54:25 -0800 (PST) Received: by mail-wm0-x22c.google.com with SMTP id f206so41699553wmf.0 for ; Tue, 05 Jan 2016 15:54:25 -0800 (PST) Date: Wed, 6 Jan 2016 00:54:21 +0100 From: "Steinar H. Gunderson" Subject: Re: Does vm_operations_struct require a .owner field? Message-ID: <20160105235418.GA1599@imap.gmail.com> References: <20160105205812.GA24738@node.shutemov.name> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: owner-linux-mm@kvack.org List-ID: To: Alan Stern Cc: "Kirill A. Shutemov" , linux-mm@kvack.org, Kernel development list , David Laight , "linux-usb@vger.kernel.org" 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 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf0-f172.google.com (mail-pf0-f172.google.com [209.85.192.172]) by kanga.kvack.org (Postfix) with ESMTP id 08DC36B0254 for ; Fri, 8 Jan 2016 04:45:38 -0500 (EST) Received: by mail-pf0-f172.google.com with SMTP id e65so7649699pfe.0 for ; Fri, 08 Jan 2016 01:45:38 -0800 (PST) Received: from bombadil.infradead.org (bombadil.infradead.org. [2001:1868:205::9]) by mx.google.com with ESMTPS id ko6si21928224pab.2.2016.01.08.01.45.37 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 08 Jan 2016 01:45:37 -0800 (PST) Date: Fri, 8 Jan 2016 01:45:35 -0800 From: Christoph Hellwig Subject: Re: [PATCH] Add support for usbfs zerocopy. Message-ID: <20160108094535.GA17286@infradead.org> References: <20160106144512.GA21737@imap.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: owner-linux-mm@kvack.org List-ID: To: Alan Stern Cc: "Steinar H. Gunderson" , Christoph Hellwig , Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org 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. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf0-f179.google.com (mail-pf0-f179.google.com [209.85.192.179]) by kanga.kvack.org (Postfix) with ESMTP id 7AF43828DE for ; Fri, 8 Jan 2016 05:25:37 -0500 (EST) Received: by mail-pf0-f179.google.com with SMTP id q63so8289037pfb.1 for ; Fri, 08 Jan 2016 02:25:37 -0800 (PST) Received: from smtp-out4.electric.net (smtp-out4.electric.net. [192.162.216.183]) by mx.google.com with ESMTPS id 83si4115218pfs.84.2016.01.08.02.25.36 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 08 Jan 2016 02:25:36 -0800 (PST) From: David Laight Subject: RE: [PATCH] Add support for usbfs zerocopy. Date: Fri, 8 Jan 2016 10:22:52 +0000 Message-ID: <063D6719AE5E284EB5DD2968C1650D6D1CCC00A7@AcuExch.aculab.com> References: <20160106144512.GA21737@imap.gmail.com> <20160108094535.GA17286@infradead.org> In-Reply-To: <20160108094535.GA17286@infradead.org> Content-Language: en-US Content-Type: text/plain; charset="Windows-1252" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Sender: owner-linux-mm@kvack.org List-ID: To: 'Christoph Hellwig' , Alan Stern Cc: "Steinar H. Gunderson" , Greg Kroah-Hartman , "linux-usb@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" 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. >=20 > 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 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lb0-f180.google.com (mail-lb0-f180.google.com [209.85.217.180]) by kanga.kvack.org (Postfix) with ESMTP id 5EDDF828DE for ; Fri, 8 Jan 2016 11:04:21 -0500 (EST) Received: by mail-lb0-f180.google.com with SMTP id oh2so227063100lbb.3 for ; Fri, 08 Jan 2016 08:04:21 -0800 (PST) Received: from mx2.suse.de (mx2.suse.de. [195.135.220.15]) by mx.google.com with ESMTPS id 88si62271248lfv.243.2016.01.08.08.04.19 for (version=TLS1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 08 Jan 2016 08:04:19 -0800 (PST) Message-ID: <1452268927.11435.3.camel@suse.com> Subject: Re: [PATCH] Add support for usbfs zerocopy. From: Oliver Neukum Date: Fri, 08 Jan 2016 17:02:07 +0100 In-Reply-To: <20160108094535.GA17286@infradead.org> References: <20160106144512.GA21737@imap.gmail.com> <20160108094535.GA17286@infradead.org> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: To: Christoph Hellwig Cc: Alan Stern , "Steinar H. Gunderson" , Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org 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 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f49.google.com (mail-wm0-f49.google.com [74.125.82.49]) by kanga.kvack.org (Postfix) with ESMTP id E70A0828DF for ; Tue, 12 Jan 2016 16:26:53 -0500 (EST) Received: by mail-wm0-f49.google.com with SMTP id l65so269159289wmf.1 for ; Tue, 12 Jan 2016 13:26:53 -0800 (PST) Received: from mail-wm0-x232.google.com (mail-wm0-x232.google.com. [2a00:1450:400c:c09::232]) by mx.google.com with ESMTPS id wx4si178596669wjc.156.2016.01.12.13.26.52 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 12 Jan 2016 13:26:52 -0800 (PST) Received: by mail-wm0-x232.google.com with SMTP id l65so269158805wmf.1 for ; Tue, 12 Jan 2016 13:26:52 -0800 (PST) Date: Tue, 12 Jan 2016 22:26:48 +0100 From: "Steinar H. Gunderson" Subject: Re: [PATCH] Add support for usbfs zerocopy. Message-ID: <20160112212644.GA6172@imap.gmail.com> References: <20160106144512.GA21737@imap.gmail.com> <20160108094535.GA17286@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160108094535.GA17286@infradead.org> Sender: owner-linux-mm@kvack.org List-ID: To: Christoph Hellwig Cc: Alan Stern , Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org 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 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ig0-f176.google.com (mail-ig0-f176.google.com [209.85.213.176]) by kanga.kvack.org (Postfix) with ESMTP id 845DE828DF for ; Tue, 12 Jan 2016 17:05:29 -0500 (EST) Received: by mail-ig0-f176.google.com with SMTP id h5so98538396igh.0 for ; Tue, 12 Jan 2016 14:05:29 -0800 (PST) Received: from iolanthe.rowland.org (iolanthe.rowland.org. [192.131.102.54]) by mx.google.com with SMTP id 66si5560785iop.49.2016.01.12.14.05.28 for ; Tue, 12 Jan 2016 14:05:28 -0800 (PST) Date: Tue, 12 Jan 2016 17:05:27 -0500 (EST) From: Alan Stern Subject: Re: [PATCH] Add support for usbfs zerocopy. In-Reply-To: <20160112212644.GA6172@imap.gmail.com> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-linux-mm@kvack.org List-ID: To: "Steinar H. Gunderson" Cc: Christoph Hellwig , Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org 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 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752497AbcAEXy7 (ORCPT ); Tue, 5 Jan 2016 18:54:59 -0500 Received: from cassarossa.samfundet.no ([193.35.52.29]:47826 "EHLO cassarossa.samfundet.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752088AbcAEXyl (ORCPT ); Tue, 5 Jan 2016 18:54:41 -0500 In-Reply-To: References: From: "Steinar H. Gunderson" Date: Thu, 26 Nov 2015 01:19:13 +0100 Subject: [PATCH] Add support for usbfs zerocopy. To: Greg Kroah-Hartman Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stern@rowland.harvard.edu Message-Id: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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(&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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752471AbcAFAZT (ORCPT ); Tue, 5 Jan 2016 19:25:19 -0500 Received: from cassarossa.samfundet.no ([193.35.52.29]:52479 "EHLO cassarossa.samfundet.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751798AbcAFAZR (ORCPT ); Tue, 5 Jan 2016 19:25:17 -0500 In-Reply-To: <20160106001143.GA1171@kroah.com> References: <20160106001143.GA1171@kroah.com> From: "Steinar H. Gunderson" Date: Thu, 26 Nov 2015 01:19:13 +0100 Subject: [PATCH v2] Add support for usbfs zerocopy. To: Greg Kroah-Hartman Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stern@rowland.harvard.edu Message-Id: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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(&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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752209AbcAEQ1t (ORCPT ); Tue, 5 Jan 2016 11:27:49 -0500 Received: from iolanthe.rowland.org ([192.131.102.54]:60578 "HELO iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1752045AbcAEQ1q (ORCPT ); Tue, 5 Jan 2016 11:27:46 -0500 Date: Tue, 5 Jan 2016 11:27:45 -0500 (EST) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: linux-mm@kvack.org cc: Kernel development list , David Laight , "'Steinar H. Gunderson'" , "linux-usb@vger.kernel.org" Subject: Does vm_operations_struct require a .owner field? In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754527AbcAEU6V (ORCPT ); Tue, 5 Jan 2016 15:58:21 -0500 Received: from mail-wm0-f42.google.com ([74.125.82.42]:36163 "EHLO mail-wm0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753539AbcAEU6Q (ORCPT ); Tue, 5 Jan 2016 15:58:16 -0500 Date: Tue, 5 Jan 2016 22:58:12 +0200 From: "Kirill A. Shutemov" To: Alan Stern Cc: linux-mm@kvack.org, Kernel development list , David Laight , "'Steinar H. Gunderson'" , "linux-usb@vger.kernel.org" Subject: Re: Does vm_operations_struct require a .owner field? Message-ID: <20160105205812.GA24738@node.shutemov.name> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23.1 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752637AbcAEVbM (ORCPT ); Tue, 5 Jan 2016 16:31:12 -0500 Received: from iolanthe.rowland.org ([192.131.102.54]:58662 "HELO iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1751959AbcAEVbK (ORCPT ); Tue, 5 Jan 2016 16:31:10 -0500 Date: Tue, 5 Jan 2016 16:31:09 -0500 (EST) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: "Kirill A. Shutemov" cc: linux-mm@kvack.org, Kernel development list , David Laight , "'Steinar H. Gunderson'" , "linux-usb@vger.kernel.org" Subject: Re: Does vm_operations_struct require a .owner field? In-Reply-To: <20160105205812.GA24738@node.shutemov.name> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752426AbcAEXy5 (ORCPT ); Tue, 5 Jan 2016 18:54:57 -0500 Received: from mail-wm0-f44.google.com ([74.125.82.44]:36815 "EHLO mail-wm0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752064AbcAEXy0 (ORCPT ); Tue, 5 Jan 2016 18:54:26 -0500 Date: Wed, 6 Jan 2016 00:54:21 +0100 From: "Steinar H. Gunderson" To: Alan Stern Cc: "Kirill A. Shutemov" , linux-mm@kvack.org, Kernel development list , David Laight , "linux-usb@vger.kernel.org" Subject: Re: Does vm_operations_struct require a .owner field? Message-ID: <20160105235418.GA1599@imap.gmail.com> References: <20160105205812.GA24738@node.shutemov.name> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752254AbcAFAMK (ORCPT ); Tue, 5 Jan 2016 19:12:10 -0500 Received: from mail.linuxfoundation.org ([140.211.169.12]:49831 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751598AbcAFAMI (ORCPT ); Tue, 5 Jan 2016 19:12:08 -0500 Date: Tue, 5 Jan 2016 16:11:43 -0800 From: Greg Kroah-Hartman To: "Steinar H. Gunderson" Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stern@rowland.harvard.edu Subject: Re: [PATCH] Add support for usbfs zerocopy. Message-ID: <20160106001143.GA1171@kroah.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752163AbcAFAYy (ORCPT ); Tue, 5 Jan 2016 19:24:54 -0500 Received: from mail-wm0-f44.google.com ([74.125.82.44]:33223 "EHLO mail-wm0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751798AbcAFAYu (ORCPT ); Tue, 5 Jan 2016 19:24:50 -0500 Date: Wed, 6 Jan 2016 01:24:47 +0100 From: "Steinar H. Gunderson" To: Greg Kroah-Hartman Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stern@rowland.harvard.edu Subject: Re: [PATCH] Add support for usbfs zerocopy. Message-ID: <20160106002443.GA3544@imap.gmail.com> References: <20160106001143.GA1171@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160106001143.GA1171@kroah.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752376AbcAFGtw (ORCPT ); Wed, 6 Jan 2016 01:49:52 -0500 Received: from bombadil.infradead.org ([198.137.202.9]:35206 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751969AbcAFGtv (ORCPT ); Wed, 6 Jan 2016 01:49:51 -0500 Date: Tue, 5 Jan 2016 22:49:49 -0800 From: Christoph Hellwig To: "Steinar H. Gunderson" Cc: Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stern@rowland.harvard.edu Subject: Re: [PATCH] Add support for usbfs zerocopy. Message-ID: <20160106064949.GA14998@infradead.org> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-SRS-Rewrite: SMTP reverse-path rewritten from by bombadil.infradead.org See http://www.infradead.org/rpr.html Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752194AbcAFOpU (ORCPT ); Wed, 6 Jan 2016 09:45:20 -0500 Received: from mail-wm0-f44.google.com ([74.125.82.44]:34837 "EHLO mail-wm0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751653AbcAFOpP (ORCPT ); Wed, 6 Jan 2016 09:45:15 -0500 Date: Wed, 6 Jan 2016 15:45:12 +0100 From: "Steinar H. Gunderson" To: Christoph Hellwig Cc: Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stern@rowland.harvard.edu Subject: Re: [PATCH] Add support for usbfs zerocopy. Message-ID: <20160106144512.GA21737@imap.gmail.com> References: <20160106064949.GA14998@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160106064949.GA14998@infradead.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752194AbcAFP26 (ORCPT ); Wed, 6 Jan 2016 10:28:58 -0500 Received: from foo.stuge.se ([212.116.89.98]:33032 "EHLO foo.stuge.se" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751717AbcAFP2z (ORCPT ); Wed, 6 Jan 2016 10:28:55 -0500 X-Greylist: delayed 400 seconds by postgrey-1.27 at vger.kernel.org; Wed, 06 Jan 2016 10:28:55 EST Message-ID: <20160106152212.15218.qmail@stuge.se> Date: Wed, 6 Jan 2016 16:22:12 +0100 From: Peter Stuge To: "Steinar H. Gunderson" Cc: Christoph Hellwig , Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stern@rowland.harvard.edu Subject: Re: [PATCH] Add support for usbfs zerocopy. Mail-Followup-To: "Steinar H. Gunderson" , Christoph Hellwig , Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stern@rowland.harvard.edu References: <20160106064949.GA14998@infradead.org> <20160106144512.GA21737@imap.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160106144512.GA21737@imap.gmail.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752258AbcAFPfI (ORCPT ); Wed, 6 Jan 2016 10:35:08 -0500 Received: from iolanthe.rowland.org ([192.131.102.54]:53110 "HELO iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1751777AbcAFPfG (ORCPT ); Wed, 6 Jan 2016 10:35:06 -0500 Date: Wed, 6 Jan 2016 10:35:05 -0500 (EST) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: "Steinar H. Gunderson" cc: Christoph Hellwig , Greg Kroah-Hartman , , Subject: Re: [PATCH] Add support for usbfs zerocopy. In-Reply-To: <20160106144512.GA21737@imap.gmail.com> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752552AbcAFPgJ (ORCPT ); Wed, 6 Jan 2016 10:36:09 -0500 Received: from mail-wm0-f54.google.com ([74.125.82.54]:38360 "EHLO mail-wm0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752079AbcAFPgF (ORCPT ); Wed, 6 Jan 2016 10:36:05 -0500 Date: Wed, 6 Jan 2016 16:36:01 +0100 From: "Steinar H. Gunderson" To: Peter Stuge Cc: Christoph Hellwig , Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stern@rowland.harvard.edu Subject: Re: [PATCH] Add support for usbfs zerocopy. Message-ID: <20160106153601.GA25632@imap.gmail.com> References: <20160106064949.GA14998@infradead.org> <20160106144512.GA21737@imap.gmail.com> <20160106152212.15218.qmail@stuge.se> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160106152212.15218.qmail@stuge.se> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752072AbcAFPjU (ORCPT ); Wed, 6 Jan 2016 10:39:20 -0500 Received: from iolanthe.rowland.org ([192.131.102.54]:53126 "HELO iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1751755AbcAFPjQ (ORCPT ); Wed, 6 Jan 2016 10:39:16 -0500 Date: Wed, 6 Jan 2016 10:39:15 -0500 (EST) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: Peter Stuge cc: "Steinar H. Gunderson" , Christoph Hellwig , Greg Kroah-Hartman , , Subject: Re: [PATCH] Add support for usbfs zerocopy. In-Reply-To: <20160106152212.15218.qmail@stuge.se> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752763AbcAGCfD (ORCPT ); Wed, 6 Jan 2016 21:35:03 -0500 Received: from mail-by2on0061.outbound.protection.outlook.com ([207.46.100.61]:4320 "EHLO na01-by2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752575AbcAGCe5 (ORCPT ); Wed, 6 Jan 2016 21:34:57 -0500 X-Greylist: delayed 1067 seconds by postgrey-1.27 at vger.kernel.org; Wed, 06 Jan 2016 21:34:57 EST Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=lingzhu.xiang@mail.utoronto.ca; MIME-Version: 1.0 In-Reply-To: References: <20160106001143.GA1171@kroah.com> From: Lingzhu Xiang Date: Wed, 6 Jan 2016 21:01:11 -0500 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v2] Add support for usbfs zerocopy. To: "Steinar H. Gunderson" CC: Greg Kroah-Hartman , , , Content-Type: text/plain; charset="UTF-8" X-Originating-IP: [209.85.160.174] X-ClientProxiedBy: CY1PR13CA0063.namprd13.prod.outlook.com (25.163.230.159) To BLUPR03MB360.namprd03.prod.outlook.com (10.141.75.155) X-Microsoft-Exchange-Diagnostics: 1;BLUPR03MB360;2:BuCaigOMROozMrFTUL2b7cxzN8dKCyrTCszcQH5xoYgQejL81Alim6TdEYxP9Ak+h/Ag+YAXh1xZsLspWFk0b6N2fQqnkK1Ffiq6AYyglyRer4ZzyT4wkoy4mAUZZmp66IGkAovJ88esJ2YC+Xl9uA==;3:zEZ/lvdqQBJEwFBnpDGyMdasaBE6bhMoUnMX+8qRWtsB9y0LNqOThg5RFHdPpA54DKNgicA9PENFEXPScTqBik8D1JJgtN6ONkzc7mDV4k7g2FO7JcQWSxz2NDFjrbMc;25:rjKJUFTsIguGvq0XdcWFdK6anfU0XiDrIqSaBLWps+9htfXVADd+CIixxek7UhGKvI4ykSathqaoiKnAJ7wpXVAOG+MfUHsYjbFVM5/TDSgyi8pbw+IIt5JJi9pP4ll4Td2vn73BHg85ASmWrapfNJg6eHUR82nlZeB45oPz6P6soKPievD/83SvEbJo9sKkp1L8VrVq5o3UI+uaXkiJ8w==;20:I0YG4sXASXiTB51c1Mao6OKTBvyoFglJ21YUArT+OoWUQ/l4YNOf4nwys9aTw3qgvJpB00xSrbKs5D8d1gDmlzRbOSIcNNiv3QgTSczpx8jQ5OZG3SuKFUHy7is/b05zJxNNtCxp9zgt2gJR1ImCltXpwmBQNOPMCW0kjSaQ00M= X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BLUPR03MB360; X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(8121501046)(5005006)(520078)(10201501046)(3002001);SRVR:BLUPR03MB360;BCL:0;PCL:0;RULEID:;SRVR:BLUPR03MB360; X-Microsoft-Exchange-Diagnostics: 1;BLUPR03MB360;4:+xb6Ar2iU06jiJuL+nC7ubyWrmGElsfvVbCppa9Np3queJ/AHlK+8dWTPIiNEb4/fmOT1aXuipPzYvnJfTB8Lg92XQuZgtzO2D0+fxzs8wfr4/jBfwoHUnrM+K/Q0bi4HtHfzhjApk6hukWwtLOUzTdEXTQMDvRGf3tcfbIotNZ45hi54Johu5hHGVO0FNnVbsTTyrHAEDfU8vvpo9AN7sSLePUNT5p9cBD/SA0spWzdTQzlJacrvaPlcUNuSn++/gnGZ2H/x82iSrNWH6uD1WivCf61EDamR7ey2Ve208o8d3dLnq+RG/OlJiGkTZDooxmRBcNslt1fU2mhRSKVteXxVufR3hmLaUJE96L9t9fcn5iftyLtJJmwNE4ukzyk X-Forefront-PRVS: 0814A2C7A3 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(6009001)(199003)(189002)(377424004)(24454002)(377454003)(479174004)(122386002)(189998001)(97736004)(105586002)(101416001)(59536001)(42186005)(122856001)(106356001)(69596002)(3846002)(575784001)(87976001)(23676002)(19580395003)(61726006)(4326007)(47776003)(74482002)(2950100001)(98316002)(50466002)(586003)(5001960100002)(92566002)(4001430100002)(61266001)(5820100001)(50986999)(40100003)(81156007)(1096002)(4001450100002)(107886002)(5004730100002)(19580405001)(93516999)(55446002)(110136002)(86362001)(5008740100001)(6116002)(66066001)(54356999)(76176999)(2906002)(63696999)(450100001)(55456009);DIR:OUT;SFP:1101;SCL:1;SRVR:BLUPR03MB360;H:mail-yk0-f174.google.com;FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtCTFVQUjAzTUIzNjA7MjM6aHhaSytzSjhxbTh5ejNEakhMcVRIOGlMOEhv?= =?utf-8?B?QUdTVDI0WlFqUUk5ZmgrQmVUaVQvVXl3TUJDbVRFQWlaR2VrNmRBajFuaVFF?= =?utf-8?B?ZitpeEFKVGNHZmNYdE83Y2dGMGt6SXZsRktsSjZ5eU1FeFUvaFFCNEhGNmk4?= =?utf-8?B?ZDVuUzU0NW5hRm1halo5elV4NWczTElvNFBZTXNKSkxHYWlDek1BMjNVVW12?= =?utf-8?B?TVlWSWVWTVZnL1kxUEhsRU1YZ2ZNdndmZkxRaE1tN0UvYlRSNjdnbVU0Uk9I?= =?utf-8?B?NWNmTzBMSzVVQlNpM2pzWXhNTFlrbVlMT0lVSCtBNzJLS2p4R3pYQ3RHaGpB?= =?utf-8?B?UFFUdnc4KzBPZ2xDbzljeHBvN1VnTS96eGV0S2c5YlI1cWRUMXY1N3A5VHQx?= =?utf-8?B?RHFHN1NXR0h6L1pxOTVVM1VENlVwa1FockZuOUVGRXJ0SWtBaTJVajJwMXR2?= =?utf-8?B?YUY3SkFFeERKMTkrVHc3ZW5TZWY1YzNnQ2dWVGN2Q2hpWkt4RHo2YzgvT25N?= =?utf-8?B?djRlSGRBdVI0SmE0QUcwZ1drbjE5SzJZV3VhNlNueEFzWFBKOG9xSnRLblM5?= =?utf-8?B?aE01QjZqVEMvc1huZVlzOWhjZnFtRmZldVQzZURNbDJ3Qk5vdjBLM2d0Kzlh?= =?utf-8?B?NUdjZmxoUXRCelZCd3pQYXVmbEUzK0dTTWRwUmdib1JQYVZOcXpFelc2UTRa?= =?utf-8?B?TUhBRmx0eEZTZDN4cjdNcGdPQ3VuWUxkMTlrbWRDdFM1eXFmd3pTN09OVnY4?= =?utf-8?B?Nk1ESG0zeGpsYzJadEFocHdiTGU4YzZkTTFYZE9LN3lvWVRLS2FmeHFzUFF6?= =?utf-8?B?bGVqNk9ESzRRdktWWG1Cc0FKY0FMNmlhZDVESTl6WmkxZjNhUWJSaU9jZGxU?= =?utf-8?B?aW9URVdhSXl2V3NKUk9weUtiYWF5cU52cm0xNGlkTTVlaEY0OUp4SWZiT0Q4?= =?utf-8?B?a1JTM0FnL2ZZbENRakxFQjRpRytYVVcwRWlyTjRVdWMrTjFuV3NSRUwza3BR?= =?utf-8?B?cVNRWUVYMmdOZm9hWVNZUVJ6SHFNYkFhYlJPTkQxQ1hTajJZTEZRQTBmQmJX?= =?utf-8?B?R1E2azZCRnI2b1BHWngzV1NzZnBQalB4K1k1NlM0ZXRadUdvWCtyMzZncTBi?= =?utf-8?B?bjBSeHltYXd4eWRzMUg5WEhUeVRRTlBMOFFqN1Q1WFVjQUNSS09kaUo2b1d6?= =?utf-8?B?b1ZtaG03MXFSK1NseW4yQVNnZUplaFBicGFlR3RJZjlmVDZCclFQSHp6TTM1?= =?utf-8?B?YjBxbDFnL21JMzBJS2ZIVFJKTFF1Uk1ZbWJldmx0bFBjQzdERXVLLzRENDdn?= =?utf-8?B?OExkNGUzVHJ0aE5tQThnc0lpMGNYc1BjOHY3VWJ6VzM1SXQyR2ErcGVHQmZL?= =?utf-8?B?Qk5wd0VhT1VlRk5TRVBzSjZCMUdBWnN2U0QzUHloVTNsMTMwdFo4NDV6YjFS?= =?utf-8?B?QzloOWd5TXdNeHlWYkMvVml5bDBtTURVdTdicmN6bXhXNDJpazdKVXp4VUps?= =?utf-8?B?RFRYd0pZK2ZEU0d2dHU3MWtDUlhONTBTWm12ZFIyUVlXR1VQVlFCUE5PU1hZ?= =?utf-8?B?MTNwS0s3YlNvRTFOeDhMVUJsUDdnN3VoMXNmSERXYURnd1pYTlpMRUFVMkVR?= =?utf-8?B?UVBFOGNPQWJZdEpjWUtsVG9raGhpVENoSWFqOUZzeU1MTGRqQ2RuWGtGemlW?= =?utf-8?B?S2lDQmVtemNEWGZRdnpSa1dFc3NLZzNqbkllalJHVEN5VGEvVGwvbzJYYWJJ?= =?utf-8?B?VUNoY1g0bkpiVXJKQlZNb05UZWlyMXRBUDBlM0VsaUhDOC92T2QyenU0QitR?= =?utf-8?B?STNqcXh0NlMzYmRqNFpiZGEyM0hUUjQ1ODJmSlh6Yi9HeWc2c1VvNTI3NTRp?= =?utf-8?B?Ymh0WUNaQ1ptRU1OSkRnRUFqOHpuOGE0YmIzTHVvc2ptV2M3VnRDRWtOSXZB?= =?utf-8?Q?qGxn8tKS/uGPm82U88X8d4C0XZXzw=3D?= X-Microsoft-Exchange-Diagnostics: 1;BLUPR03MB360;5:5ipV9k3CQ34ApJXlt/Dyj6EvhxH4f7Ioncz2B1Lbm5wd9mQHkiXrKhPNgS+EjE8fq8CMBdkG3brKedEaxECVodUDlIPhUcPznTsMA22dYIIhrY7okhqoPM+nvEmiO7UKwjmwnfMHLQCkYkrcfe+C6A==;24:/5ca7/GOQ0qZBXZDZ9SqZi36qfU/aUxWcptZzsJ/3zTv2R5JLzm8eqCPyS+K5a7yH/jgis3Q53iYSk8MKrltE3YY053n8lhLSlaLQ9/o04g= SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: mail.utoronto.ca X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Jan 2016 02:01:56.0301 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLUPR03MB360 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Nov 25, 2015 at 7: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: 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] [] ? dump_stack+0x40/0x57 > [ 1706.234851] [] ? swiotlb_alloc_coherent+0x135/0x150 > [ 1706.234867] [] ? hcd_buffer_alloc+0xb1/0x130 [usbcore] > [ 1706.234875] [] ? usbdev_mmap+0xa5/0x1b0 [usbcore] > [ 1706.234880] [] ? tty_insert_flip_string_fixed_flag+0x85/0xe0 > [ 1706.234885] [] ? mmap_region+0x3e7/0x660 > [ 1706.234888] [] ? do_mmap+0x336/0x420 > [ 1706.234892] [] ? vm_mmap_pgoff+0xaf/0xf0 > [ 1706.234895] [] ? SyS_mmap_pgoff+0x1ad/0x270 > [ 1706.234898] [] ? SyS_write+0x76/0xc0 > [ 1706.234903] [] ? 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752876AbcAGPk7 (ORCPT ); Thu, 7 Jan 2016 10:40:59 -0500 Received: from iolanthe.rowland.org ([192.131.102.54]:46112 "HELO iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1751651AbcAGPk5 (ORCPT ); Thu, 7 Jan 2016 10:40:57 -0500 Date: Thu, 7 Jan 2016 10:40:56 -0500 (EST) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: Lingzhu Xiang cc: "Steinar H. Gunderson" , Greg Kroah-Hartman , Konrad Rzeszutek Wilk , USB list , Kernel development list Subject: Re: [PATCH v2] Add support for usbfs zerocopy. In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 6 Jan 2016, Lingzhu Xiang wrote: > On Wed, Nov 25, 2015 at 7: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: > > 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] [] ? dump_stack+0x40/0x57 > > [ 1706.234851] [] ? swiotlb_alloc_coherent+0x135/0x150 > > [ 1706.234867] [] ? hcd_buffer_alloc+0xb1/0x130 [usbcore] > > [ 1706.234875] [] ? usbdev_mmap+0xa5/0x1b0 [usbcore] > > [ 1706.234880] [] ? tty_insert_flip_string_fixed_flag+0x85/0xe0 > > [ 1706.234885] [] ? mmap_region+0x3e7/0x660 > > [ 1706.234888] [] ? do_mmap+0x336/0x420 > > [ 1706.234892] [] ? vm_mmap_pgoff+0xaf/0xf0 > > [ 1706.234895] [] ? SyS_mmap_pgoff+0x1ad/0x270 > > [ 1706.234898] [] ? SyS_write+0x76/0xc0 > > [ 1706.234903] [] ? 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932079AbcAHJnI (ORCPT ); Fri, 8 Jan 2016 04:43:08 -0500 Received: from bombadil.infradead.org ([198.137.202.9]:42456 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750794AbcAHJnF (ORCPT ); Fri, 8 Jan 2016 04:43:05 -0500 Date: Fri, 8 Jan 2016 01:43:03 -0800 From: Christoph Hellwig To: "Steinar H. Gunderson" Cc: Christoph Hellwig , Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stern@rowland.harvard.edu Subject: Re: [PATCH] Add support for usbfs zerocopy. Message-ID: <20160108094303.GA9234@infradead.org> References: <20160106064949.GA14998@infradead.org> <20160106144512.GA21737@imap.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160106144512.GA21737@imap.gmail.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-SRS-Rewrite: SMTP reverse-path rewritten from by bombadil.infradead.org See http://www.infradead.org/rpr.html Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754219AbcAHJpl (ORCPT ); Fri, 8 Jan 2016 04:45:41 -0500 Received: from bombadil.infradead.org ([198.137.202.9]:42586 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751397AbcAHJpi (ORCPT ); Fri, 8 Jan 2016 04:45:38 -0500 Date: Fri, 8 Jan 2016 01:45:35 -0800 From: Christoph Hellwig To: Alan Stern Cc: "Steinar H. Gunderson" , Christoph Hellwig , Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH] Add support for usbfs zerocopy. Message-ID: <20160108094535.GA17286@infradead.org> References: <20160106144512.GA21737@imap.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-SRS-Rewrite: SMTP reverse-path rewritten from by bombadil.infradead.org See http://www.infradead.org/rpr.html Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754760AbcAHKZm (ORCPT ); Fri, 8 Jan 2016 05:25:42 -0500 Received: from smtp-out4.electric.net ([192.162.216.186]:64064 "EHLO smtp-out4.electric.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754646AbcAHKZi convert rfc822-to-8bit (ORCPT ); Fri, 8 Jan 2016 05:25:38 -0500 From: David Laight To: "'Christoph Hellwig'" , Alan Stern CC: "Steinar H. Gunderson" , Greg Kroah-Hartman , "linux-usb@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" Subject: RE: [PATCH] Add support for usbfs zerocopy. Thread-Topic: [PATCH] Add support for usbfs zerocopy. Thread-Index: AQHRSflS3FqNe1oABkOH570G6/nu557xaD3w Date: Fri, 8 Jan 2016 10:22:52 +0000 Message-ID: <063D6719AE5E284EB5DD2968C1650D6D1CCC00A7@AcuExch.aculab.com> References: <20160106144512.GA21737@imap.gmail.com> <20160108094535.GA17286@infradead.org> In-Reply-To: <20160108094535.GA17286@infradead.org> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.202.99.200] Content-Type: text/plain; charset="Windows-1252" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 X-Outbound-IP: 213.249.233.130 X-Env-From: David.Laight@ACULAB.COM X-PolicySMART: 3396946, 3397078 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932412AbcAHQEV (ORCPT ); Fri, 8 Jan 2016 11:04:21 -0500 Received: from mx2.suse.de ([195.135.220.15]:52276 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755700AbcAHQET (ORCPT ); Fri, 8 Jan 2016 11:04:19 -0500 Message-ID: <1452268927.11435.3.camel@suse.com> Subject: Re: [PATCH] Add support for usbfs zerocopy. From: Oliver Neukum To: Christoph Hellwig Cc: Alan Stern , "Steinar H. Gunderson" , Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org Date: Fri, 08 Jan 2016 17:02:07 +0100 In-Reply-To: <20160108094535.GA17286@infradead.org> References: <20160106144512.GA21737@imap.gmail.com> <20160108094535.GA17286@infradead.org> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.12.11 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752258AbcAIEQg (ORCPT ); Fri, 8 Jan 2016 23:16:36 -0500 Received: from mail-bn1on0056.outbound.protection.outlook.com ([157.56.110.56]:40222 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752056AbcAIEQ3 (ORCPT ); Fri, 8 Jan 2016 23:16:29 -0500 X-Greylist: delayed 889 seconds by postgrey-1.27 at vger.kernel.org; Fri, 08 Jan 2016 23:16:28 EST Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=lingzhu.xiang@mail.utoronto.ca; MIME-Version: 1.0 In-Reply-To: References: <20160106001143.GA1171@kroah.com> From: Lingzhu Xiang Date: Fri, 8 Jan 2016 23:00:54 -0500 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v2] Add support for usbfs zerocopy. To: "Steinar H. Gunderson" CC: Greg Kroah-Hartman , , , Alan Stern Content-Type: text/plain; charset="UTF-8" X-Originating-IP: [209.85.160.171] X-ClientProxiedBy: BN1PR12CA0027.namprd12.prod.outlook.com (25.160.77.37) To DM2PR03MB368.namprd03.prod.outlook.com (10.141.55.22) X-Microsoft-Exchange-Diagnostics: 1;DM2PR03MB368;2:be5IDAXWVRfD1+AfOlmWGtlDDXrsJeYXz7ZxE5RTAOb/s7nxoh23KtnCIVn79NMZ2gzBweTng5TAyW0vUKqLNbKpU5gyiAe1krhMD6tXSIUZeEWXJ9D+/cBrd/XEn98X9LOS4uc3eg6QG7BjnEQAEg==;3:qbLUkMPBHja9LxqnkwYF8IbUfa3X7MP275PvwhGrqq3DtEx8HMDjDGaYSHeYf8DzuVK6enKtGyfaNZE5gSq3zHGEdM5SoXMkCS056bh/2Jbs/CN/G816JIh/yQ77gVeX;25:4DcDllRrGWHxepQcUz12TIytbFmtjiQTSvICjJdFah9kijCUsILvfiFrbeUWxMsQKgEKENuTNHKmGrF2wSladknmm34KlDKpWXvuEyarJu8Ov6ZTUkQJ3m6jETAq1aApgXT47gQl+dIxWscGffzZ+NLx1mdGCjScL4+JDWuxOz9BmvyZbIlWOeBdiZqFSH7ZhrOdXvCHZ12rSiXxlb/qjvOkvLVy0YyrgpRsU2sLS7tPcoBLM/SHerSevPZrJ8zh;20:rL3hH7/Zn+uqcok1bexQPNa61dbyDFIeWx5wOAaKv83X7xWQIhIjvMc2hB68OnNRx1cqjunrewfpiL4D+w/vXUxAyOHFFm+MxZme4Jq2W2al60aHZL9KOlXLzee2H/vRkebePMrCguLN93ObFFwTJOgJZe77oPtFqVfbmjm4aIE= X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:DM2PR03MB368; X-MS-Office365-Filtering-Correlation-Id: eb0806aa-8ff5-443d-c610-08d318a99276 X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(520078)(5005006)(8121501046)(3002001)(10201501046);SRVR:DM2PR03MB368;BCL:0;PCL:0;RULEID:;SRVR:DM2PR03MB368; X-Microsoft-Exchange-Diagnostics: 1;DM2PR03MB368;4:ZJdTO1SiLAkQUxB30DXdG7+YLJakH+CWODzOTyz93CE4o/OV25B5y11GLFt+CpRWMfF+MtwJsGJB9lOjSqpiGAOO/ZZvoTBBQn58mKvGRqLlAE1O3r1li1XqCyoEB6HP4X4qh5r9VsjPez16fcqnGgnYupbCWBAXZ9bczNn/GZojaHaY2uYxNI/sZ9tRESLY04DZIJOWLm2m5UJ0p2v8aXegzwfZBHh4b7Z1wCHu2iVW5ELK0uc9YSfXkgLJ+LIWREFcwqYqhTy4eRnQIM1cDOyAEGrjazI4nPVZntBTz+3YLzn/vzB8eAFyLIqg5cMkfqx1iPLMDOp1CLlCwEPyJ5gm0K8/meiOpfO0IC5hVxAmypLZPbsPI3rUu9nWvcih X-Forefront-PRVS: 0816F1D86E X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(6009001)(24454002)(189002)(199003)(377454003)(97736004)(189998001)(59536001)(6116002)(4001430100002)(74482002)(1096002)(3846002)(586003)(61266001)(107886002)(5001960100002)(5008740100001)(2950100001)(47776003)(110136002)(66066001)(93516999)(92566002)(4326007)(23676002)(61726006)(81156007)(105586002)(122386002)(101416001)(69596002)(40100003)(106356001)(42186005)(122856001)(55446002)(5004730100002)(50466002)(4001450100002)(5820100001)(19580405001)(19580395003)(450100001)(86362001)(87976001)(2906002)(50986999)(54356999)(76176999)(63696999)(55456009);DIR:OUT;SFP:1101;SCL:1;SRVR:DM2PR03MB368;H:mail-yk0-f171.google.com;FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtETTJQUjAzTUIzNjg7MjM6cGNjcVVFQkQ2K2x3dktrSjJZaDgyK3NMTkVp?= =?utf-8?B?RmpvZ1Y5ak1ndmVIdThHdW02ZlE0K29kVHIvQzZnaG1EVllISE45SG1UbVBW?= =?utf-8?B?UmE0djVXcGZMK1BaNWZscjNGYWZxdGgrSEJsVWdiVWZQcDFjZGFHc092a2c0?= =?utf-8?B?K0NkcXNCN2p1SGJSeHFQcDYzR3BZOFQzWng2WHV4bkgzU1JLT2QyMmJxN21l?= =?utf-8?B?dHlkamlZbEhyQ1lEYTNNWHVka2U1RTJyVFJrcXNBZnlKMFhsaVRVNXBwd0FI?= =?utf-8?B?dmJIOWtjeEtnTVBFRXEvOWJwMlAyRUtEaHNpVENadHB5cmZ0RFRKV2p2Rjlt?= =?utf-8?B?a1RqbDUwa2JaY21mTGR4YWJFcHhpMVljQXczV3NoN1IwbXBPMkUxRkJYSDhy?= =?utf-8?B?OGZwUnRSWDIvVFBHa2tieXZYNldxakk2TUdZOWE2YlAvMWhBKzJyS0JGWHZt?= =?utf-8?B?d0ZCa2lWRmhoNTVzY1NxT29ZbzViWFpaMWFzT2l2ZVlxcmNOb21remd5SW8v?= =?utf-8?B?RExUcGlBb25USnBaUVpPWllZcDl2OGFYTmdONUtOa1ZHWm1aZDFLcXlyTzA5?= =?utf-8?B?ZTRmOE5zNm44S1JFUTFDVG10d0RoS3VUSWVqLzhwSThUTU5iRXA0NDhmNTlK?= =?utf-8?B?N2hNV1ZTQnJFY3lNeHpzbHNzUUh4Zy81OXZqVitEVnljbkJwMEJCd0NIT2sr?= =?utf-8?B?R2ovMlJsUzM5dXZ0cWwvU1JTck1OMHhmcTJ0emhwODJYc0xUM2NVUWxsMEVR?= =?utf-8?B?cEdzL2g4eGp4YXZFNmQyNFN2U1kxbDh6MXpXRjFxcHBmbWl1a1psU1FUY2xX?= =?utf-8?B?SElveFEvN2ZlODRmUlcraDgwb0ozd09XbUVKdm1RWlVaUGxOZHFaOG9LdTQy?= =?utf-8?B?MGxSYmNLRWJwQ3Y0ZWtGYTRydjk3MERVTnpLMHZWYUJVOWIwMmZ4NmlEaWlU?= =?utf-8?B?K095aXlxRVI5dGdGRU5YcmFsMTR1MFVLcUpOMEcxVngxQ0luSWxJcjZYZVps?= =?utf-8?B?dXhtWnZ6eEJ3UUpta0ExWWxhMURickU3L3QxMmxUNFdJM0c3cTdDYnZHdjlH?= =?utf-8?B?aE5ab3drWjBXdU5uV24wOW9MR2V5VEdNN0NIR2YzSW9sZW5yQ0M5QjJvd3NE?= =?utf-8?B?alQ2L1JlWURub1RzNlg0d2UzK2sxdVBzSUYzY3MwUDh1YXlJMDRJZVd4Y2hl?= =?utf-8?B?SjFCTmFlSHpEaVphcVZVUXdwZUNxZzBLeDVsYTcxSXEzdW14Qnh1ZHEvcWhN?= =?utf-8?B?OWVYVjk2RjlBZzk2d1BXekc1ZVBIRWtZMlBHK2FaZXhpdjFmY1dyQ2pCMThP?= =?utf-8?B?bDM1bTl5ZHhQaVFyRjZ5QkZQaEM1bFc5L1NXQllZTVR4YTFmRjNCL1A5MGJG?= =?utf-8?B?blRzZDBIK0d0ekhGazRQWnhtbEx6YmtNT3ZWYjYzbmZqYi9mamt5T1htTzZR?= =?utf-8?B?YzQrVEd0VU1Fdy9QcHJxREhYWERMVGhIdzRJZjdyS1IyV1ZlMlg2NzNlTkNz?= =?utf-8?B?cGJHT2ZZMWdvOWJjUTZMdndmQnZ4QVdUOUVZMU9UWVh5SzhPTDVmaUZMRzlB?= =?utf-8?B?ZkZwYTZxbnpLYyszOG9CVGJlR3V2Smlic0NTTVNITDlIWlpHYXJwZFBhZ0ZU?= =?utf-8?B?K2o1YmlLTXJHNldwcm40bjZiRWM4d2dtN0E1bnBpODg3dHp6bTh4WGdmQ1hs?= =?utf-8?B?WE9ybnFKbDVYbWhkM0ptYlk0LzNEaHYvZXlCeGRhTWhURFY3aDRxZG1CU1NJ?= =?utf-8?B?SkFyVUkyU1p1am9IQzVqeEY0dVhXZElmeEtJU1RHWXdybitldExuMENwZVdH?= =?utf-8?Q?mMCYUkX1MRsU?= X-Microsoft-Exchange-Diagnostics: 1;DM2PR03MB368;5:I60xCgiGdC6BymCnI8ygWzwD6gOlM9Lv05GMZ38j8TSROfrbcGj3zJSdVGEtfV/KEp4F6jwnp5UaNIJ8eXk1MV2khf/10MV3lDqyZIQ9HkoCFXXgOZGztevIvCGHXaIEbNJi1ad/p/ajuTw9zoSdkA==;24:ajBdOYZiNcxxlVspDc00C8lyerrFtv2YMa92sUpgng2VGzAZJ8vtjEPwm7YXWZyL1RzmcPe/G6fW3ML0PsVpKHGfsVbPuxFNQcz99oWTVR4= SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: mail.utoronto.ca X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Jan 2016 04:01:37.5169 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM2PR03MB368 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Nov 25, 2015 at 7: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: 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753702AbcAIK51 (ORCPT ); Sat, 9 Jan 2016 05:57:27 -0500 Received: from mail-wm0-f53.google.com ([74.125.82.53]:35305 "EHLO mail-wm0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752398AbcAIK5Z (ORCPT ); Sat, 9 Jan 2016 05:57:25 -0500 Date: Sat, 9 Jan 2016 11:57:21 +0100 From: "Steinar H. Gunderson" To: Lingzhu Xiang Cc: Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Alan Stern Subject: Re: [PATCH v2] Add support for usbfs zerocopy. Message-ID: <20160109105717.GB23584@imap.gmail.com> References: <20160106001143.GA1171@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759583AbcAKLKr (ORCPT ); Mon, 11 Jan 2016 06:10:47 -0500 Received: from mx2.suse.de ([195.135.220.15]:33931 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759098AbcAKLKp (ORCPT ); Mon, 11 Jan 2016 06:10:45 -0500 Message-ID: <1452510506.3907.3.camel@suse.com> Subject: Re: [PATCH v2] Add support for usbfs zerocopy. From: Oliver Neukum To: Alan Stern Cc: Lingzhu Xiang , "Steinar H. Gunderson" , Greg Kroah-Hartman , Konrad Rzeszutek Wilk , USB list , Kernel development list Date: Mon, 11 Jan 2016 12:08:26 +0100 In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.12.11 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S964824AbcALKCx (ORCPT ); Tue, 12 Jan 2016 05:02:53 -0500 Received: from mx2.suse.de ([195.135.220.15]:38420 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752603AbcALKCu (ORCPT ); Tue, 12 Jan 2016 05:02:50 -0500 Message-ID: <1452592833.6017.2.camel@suse.com> Subject: Re: [PATCH v2] Add support for usbfs zerocopy. From: Oliver Neukum To: Konrad Rzeszutek Wilk Cc: "Steinar H. Gunderson" , Greg Kroah-Hartman , Lingzhu Xiang , Alan Stern , Kernel development list , USB list Date: Tue, 12 Jan 2016 11:00:33 +0100 In-Reply-To: <20160111161504.GG10641@char.us.oracle.com> References: <1452510506.3907.3.camel@suse.com> <20160111161504.GG10641@char.us.oracle.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.12.11 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754701AbcALV1F (ORCPT ); Tue, 12 Jan 2016 16:27:05 -0500 Received: from mail-wm0-f41.google.com ([74.125.82.41]:38786 "EHLO mail-wm0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752453AbcALV0x (ORCPT ); Tue, 12 Jan 2016 16:26:53 -0500 Date: Tue, 12 Jan 2016 22:26:48 +0100 From: "Steinar H. Gunderson" To: Christoph Hellwig Cc: Alan Stern , Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH] Add support for usbfs zerocopy. Message-ID: <20160112212644.GA6172@imap.gmail.com> References: <20160106144512.GA21737@imap.gmail.com> <20160108094535.GA17286@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160108094535.GA17286@infradead.org> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753454AbcALWF3 (ORCPT ); Tue, 12 Jan 2016 17:05:29 -0500 Received: from iolanthe.rowland.org ([192.131.102.54]:44066 "HELO iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1750915AbcALWF2 (ORCPT ); Tue, 12 Jan 2016 17:05:28 -0500 Date: Tue, 12 Jan 2016 17:05:27 -0500 (EST) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: "Steinar H. Gunderson" cc: Christoph Hellwig , Greg Kroah-Hartman , , , Subject: Re: [PATCH] Add support for usbfs zerocopy. In-Reply-To: <20160112212644.GA6172@imap.gmail.com> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754762AbcAMHAx (ORCPT ); Wed, 13 Jan 2016 02:00:53 -0500 Received: from mail-bl2on0068.outbound.protection.outlook.com ([65.55.169.68]:8928 "EHLO na01-bl2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750948AbcAMHAk (ORCPT ); Wed, 13 Jan 2016 02:00:40 -0500 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=lingzhu.xiang@mail.utoronto.ca; MIME-Version: 1.0 In-Reply-To: References: <20160112212644.GA6172@imap.gmail.com> From: Lingzhu Xiang Date: Wed, 13 Jan 2016 01:59:54 -0500 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH] Add support for usbfs zerocopy. To: Alan Stern CC: "Steinar H. Gunderson" , Greg Kroah-Hartman , , Content-Type: text/plain; charset="UTF-8" X-Originating-IP: [209.85.160.169] X-ClientProxiedBy: BN1PR08CA0044.namprd08.prod.outlook.com (10.242.217.172) To BY2PR03MB363.namprd03.prod.outlook.com (10.242.237.16) X-Microsoft-Exchange-Diagnostics: 1;BY2PR03MB363;2:UsQd0KL3pe/Dagw/f/tXI6NZZZYXJw2aQgo4mae+Yz900wA/ZBppDFnuGvdzNOPZnc+3mQrxZC4HI70TGl+KCYrDaB+9NueZRXMmiGkUxD28WDFfyexExCuk266u6JP75OmY3t0VnzXvgd+jyre4IQ==;3:TC+PbdbUcBPmS5lR2PEky0CwlwaYoPuDamU0NAu5xcKsDgePlHliaXFhOQPprYX+FJXh0/UsGs4QgVUqgAGbiWNswu4ESlap/qSDKkuVBGDhaUbJGjzRQ1nWUGOzh+1Z;25:KukgyM+BYNPOaqTxrw9m0LPDjbKxZbIJWhJhSLe33hVv2XTuTcTTRGkPQKdODTYFZwIolKSuiDI3f39VrKY75C0Eyki8bZPASb8Ut/a240VkR8Z/bzBuhyE3yaw4mMkBCBomA/ejPEaj/WwGOaNbf3kw4Yga/VyTlbqLekhr7wxz/X+TjvNnEEU70GhRiRlwDRIraVhTXbTZcqTpyyAah+OhbgtoZlTQ2rcE2VIjHa3+ztuOILPoB/w6v4YT3RlL;20:W5aj1ixxO41Ni4AUOrgRCli3+hZYVT/9AarnLQTpQO0HykLs45CfObFLvnPBWB2PBrG67n4Cqjc+2I0TaVXnHspuv5gwXokT6RXOWy80S8zk+qNp7y4ewd527219YJl5YIPtNjongE6esnUJIDGvC/E0nO46X9fotXtocJwKizU= X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BY2PR03MB363; X-MS-Office365-Filtering-Correlation-Id: ed0f9d3e-47d6-4c43-2bff-08d31be73d6a X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(35762410373642); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(5005006)(520078)(8121501046)(3002001)(10201501046);SRVR:BY2PR03MB363;BCL:0;PCL:0;RULEID:;SRVR:BY2PR03MB363; X-Microsoft-Exchange-Diagnostics: 1;BY2PR03MB363;4:hPt/qzuBa8SbqFDlILFzRP8tKSpqZdot3KJUqOZVaLnR1GKiJ+D8MngJDSRVKJT0RH/Ms2A1JPWrltQWoDMnrvn/PnmZOqF+drVKauYfHJxvISSMeOS0XGpi+Zp0zZFF1cLNZnxhcR+0u620R+qZhOtsxoz+IWOHeqegz30basKnTqeB8WZWDibb0Um30v5cmRa4AOvxWhC5qeRHqQPdffumgqxyttqYfKlP/y04gdvxs55ykGDEVgaZZR03Qq91a1orBQ7z/lFflImuwNNxAKsES82gURLzsH+Ct/xF4PyP42fyXF57+/Qc2z/p1JVoD2C1lYQ/3nm4OTNv6SkcpD44qjvVaaR6o9z6SIbe+yuaruyFa3m1orkLJ3gPmUf8d7Ctu3CuBE6W+rex79STpNB8Jaw3whJ4AvrgEiSL/Wk= X-Forefront-PRVS: 08200063E9 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(6009001)(189002)(377454003)(24454002)(199003)(81156007)(6116002)(93516999)(3846002)(586003)(61266001)(450100001)(97736004)(122856001)(5820100001)(42186005)(40100003)(122386002)(47776003)(92566002)(66066001)(1096002)(2906002)(110136002)(4326007)(4001450100002)(5008740100001)(189998001)(2950100001)(101416001)(19580395003)(87976001)(59536001)(69596002)(61726006)(74482002)(19580405001)(50466002)(54356999)(106356001)(5004730100002)(105586002)(5001960100002)(76176999)(50986999)(63696999)(55446002)(23676002)(86362001)(2171001)(55456009);DIR:OUT;SFP:1101;SCL:1;SRVR:BY2PR03MB363;H:mail-yk0-f169.google.com;FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtCWTJQUjAzTUIzNjM7MjM6M281NDNYb3JNTWJ3cUR3dTRXY05qczdKL0FW?= =?utf-8?B?VVZ5cjg3ZFgzZCs1NjNDNXM5dEsxVW1sRVZmNDhad21OUVNRKzQweUVPN0lH?= =?utf-8?B?MHNjNzF4elhEaE16MVM0RVZpYVVtS2ZVVE9tekJxYXR0b2JOdUJFcnRIZWlr?= =?utf-8?B?VjJISEl3M2ROK1hobEJIdGdPTDNXUnVpWlJ3WXA1OC9kK2hqRFU2Zi8rb3Nh?= =?utf-8?B?dEJGQ016ZDkrbU93c3l6WnNCSjhSSDFqS3UzbXN3WTlzSFprMEprM2NDQy90?= =?utf-8?B?bTF4SlJrU281ZXpzVVBhSnYrNkpQRVFJbVZIL00xdEliMFZoN0ZTRHhHcTFE?= =?utf-8?B?cmh3VmxEKzlKYTN5LzF4cWtOZjk5ZUJTVmYva3dtTXZtSnZXbTY3L3pCVDg1?= =?utf-8?B?QVJsR1MyQjZZQWJkayt1RWhvWGFLNWlJRmw3UUNKYXpXemcyOHVJcVVDMXlw?= =?utf-8?B?ZW80aDZ3RTJETHpwWHR0ZVdZVEVlY05zZUpteURqM0hJQ2xvU2xKVUVSbm5x?= =?utf-8?B?Qy84bklHSTkvZW9oTGdBWUVzTmFjb3hRbGZVSm9BQTM1Vm9RZXJqTWNiUXNr?= =?utf-8?B?UThIaUM4R3VFQWlDVVc1blMvMW4yc3JldlU2QmlyR0NIQUJQcjVzRWlkNHBl?= =?utf-8?B?bGUvRFVkdGVoRWcvN2FrM1lRQjZKbUVva0thckpnUTB4YUw3NXFBVCt4QlB0?= =?utf-8?B?eFA0ckh2SXJkNmJkTkU1YUJ1WFkxczIvNE5tcXNZcXZwY29nRy9HQlJvYzVy?= =?utf-8?B?MlpST2FCSmlBR1dGd2wrc2ZqRUMzb3BDVklUS1dETTNWMjlwbm1xMGxhR1Z6?= =?utf-8?B?cGZlNTl4akliaEVXYnFUNEk0OXE1L1pMZlprMm5zQ2ZabFhuWU5NcVQwQmFh?= =?utf-8?B?cm9lU1RpdWdzb2FYUWdOZ3dBTHpzL091cVRxb2lvblB3VlJ5emkvMGhaczlO?= =?utf-8?B?Y3ZWUjQxWFVBWkZYdlZHdVk4TEdzZkRzZmJMU21zZjJKelpnOHdiNU5VdzQ4?= =?utf-8?B?MS9iZEwyaE9FZklPV2JxMUZ4ZlBDR2tGTVFTVDV2TE9tM1A2bFM4eGlhdXJG?= =?utf-8?B?aVVXM2NJSDFJRWdQWnc3TUNURG13ZmQrV3MxTmZWNmxyYzlYQTBLNml5OU5h?= =?utf-8?B?Uyt0MUFWYjR3Si9Demk3NzBZbTFzOE5nZFdubXYrNGtOazVOTytvQ1hXek1m?= =?utf-8?B?bG50aUdXblFucFdoQTNTdFB4V3VWSXRSaE1vWk96UnpNSDVrTGtJTDVVeGlW?= =?utf-8?B?VjFnblZWQVRSNFF1Z21Ec3A1N0VoQ0dzdnJDb0ZIZkZ0QTRjZTVlTUdKSnh3?= =?utf-8?B?SGVrS3JjZU8xVUw5cHRJL3V1RzdYNmZidkw0YU04aVVVWEt0YmhLZDYxcC9s?= =?utf-8?B?UnBBcStzZmYxQ0ZmOEt4RnRDMVBIVGM4TGQzaXArYmpScUY4cENsNTZPL2xN?= =?utf-8?B?aEFCL1E5VGZNVXRXTnpZTTN6VmVKbGlZazRLTTRzU2V4R2IrUjJQT1o4amVs?= =?utf-8?B?Z2R6MlpHdEZwWE5EUThGRlZjRGZ6VXJkeUNsMXZxVjhJSmxXT2NkSjNPMXNy?= =?utf-8?B?TlJBc2d2UUw2S05EMFMvVHdDTitUbFhpZEtyMy9WQnI4L0pVbzdxK09GT3ZH?= =?utf-8?B?Yy9DWVN2dDFESVNsSzV1YlJqeU9pNlFlRjkwN1BtcU5LTzJ0L3VMQ2hVVEFt?= =?utf-8?B?OTFhTXc4STEwdUkxMUhTNzVxVC9tazJ1aXF2UW0rOUp2dWMrcXI0alRxWXo1?= =?utf-8?Q?vib+mrydPOiyUsbw6QwGVdewGyDX1V3kZhzo=3D?= X-Microsoft-Exchange-Diagnostics: 1;BY2PR03MB363;5:XthltPkoPRof/9H+SbQoNP+FTi7YIxPhKUlH+M+QsZI25bP4Mn1HVHFDcHUNBgArqguzZIudHfpp4HiiNVPdtegkIdvFmHvzDOwk7F26xX8pCp6wg2A2qYLKhgCZXAhJY+qZoEMwf6ZWSe6kPCqQsw==;24:p16t1XBfHHNA5HgmeETd6lCI+pkLwFoascpuj3UtaVB1doyqqKXV4MxyaHsRm6BwBci3iWVmt2gth5lA89DvcpdWatguQJrPlnAAsyNx7nY= SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: mail.utoronto.ca X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Jan 2016 07:00:37.1135 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY2PR03MB363 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jan 12, 2016 at 5:05 PM, Alan Stern 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 ACKs? Regards, Lingzhu From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932261AbcAMRVM (ORCPT ); Wed, 13 Jan 2016 12:21:12 -0500 Received: from mail.linuxfoundation.org ([140.211.169.12]:40598 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752023AbcAMRVJ (ORCPT ); Wed, 13 Jan 2016 12:21:09 -0500 Date: Wed, 13 Jan 2016 09:21:08 -0800 From: Greg Kroah-Hartman To: Lingzhu Xiang Cc: Alan Stern , "Steinar H. Gunderson" , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] Add support for usbfs zerocopy. Message-ID: <20160113172108.GA30081@kroah.com> References: <20160112212644.GA6172@imap.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jan 13, 2016 at 01:59:54AM -0500, Lingzhu Xiang wrote: > On Tue, Jan 12, 2016 at 5:05 PM, Alan Stern 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754155AbcAXVML (ORCPT ); Sun, 24 Jan 2016 16:12:11 -0500 Received: from mail.linuxfoundation.org ([140.211.169.12]:57710 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751723AbcAXVMJ (ORCPT ); Sun, 24 Jan 2016 16:12:09 -0500 Date: Sun, 24 Jan 2016 13:12:08 -0800 From: Greg Kroah-Hartman To: "Steinar H. Gunderson" Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stern@rowland.harvard.edu Subject: Re: [PATCH v2] Add support for usbfs zerocopy. Message-ID: <20160124211208.GA8696@kroah.com> References: <20160106001143.GA1171@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755465AbcAYIEH (ORCPT ); Mon, 25 Jan 2016 03:04:07 -0500 Received: from mail-wm0-f47.google.com ([74.125.82.47]:38378 "EHLO mail-wm0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751913AbcAYIEB (ORCPT ); Mon, 25 Jan 2016 03:04:01 -0500 Date: Mon, 25 Jan 2016 09:03:57 +0100 From: "Steinar H. Gunderson" To: Greg Kroah-Hartman Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stern@rowland.harvard.edu Subject: Re: [PATCH v2] Add support for usbfs zerocopy. Message-ID: <20160125080352.GA30907@imap.gmail.com> References: <20160106001143.GA1171@kroah.com> <20160124211208.GA8696@kroah.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="BXVAT5kNtrzKuDFl" Content-Disposition: inline In-Reply-To: <20160124211208.GA8696@kroah.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --BXVAT5kNtrzKuDFl Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 --BXVAT5kNtrzKuDFl Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v3-0001-Add-support-for-usbfs-zerocopy.patch" >>From e56d9235b343c5e70061e977639cc7dddeae8164 Mon Sep 17 00:00:00 2001 From: "Steinar H. Gunderson" 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 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(&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 --BXVAT5kNtrzKuDFl-- From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754634AbcBBKe5 (ORCPT ); Tue, 2 Feb 2016 05:34:57 -0500 Received: from mail-wm0-f49.google.com ([74.125.82.49]:38793 "EHLO mail-wm0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754281AbcBBKez (ORCPT ); Tue, 2 Feb 2016 05:34:55 -0500 Date: Tue, 2 Feb 2016 11:34:51 +0100 From: "Steinar H. Gunderson" To: Greg Kroah-Hartman Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stern@rowland.harvard.edu Subject: Re: [PATCH v2] Add support for usbfs zerocopy. Message-ID: <20160202103451.GA79339@imap.gmail.com> References: <20160106001143.GA1171@kroah.com> <20160124211208.GA8696@kroah.com> <20160125080352.GA30907@imap.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160125080352.GA30907@imap.gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755900AbcBCVXY (ORCPT ); Wed, 3 Feb 2016 16:23:24 -0500 Received: from mail.linuxfoundation.org ([140.211.169.12]:39338 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755867AbcBCVXS (ORCPT ); Wed, 3 Feb 2016 16:23:18 -0500 Date: Wed, 3 Feb 2016 13:23:17 -0800 From: Greg Kroah-Hartman To: "Steinar H. Gunderson" Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stern@rowland.harvard.edu Subject: Re: [PATCH v2] Add support for usbfs zerocopy. Message-ID: <20160203212317.GA7506@kroah.com> References: <20160106001143.GA1171@kroah.com> <20160124211208.GA8696@kroah.com> <20160125080352.GA30907@imap.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160125080352.GA30907@imap.gmail.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965547AbcBCWJf (ORCPT ); Wed, 3 Feb 2016 17:09:35 -0500 Received: from cassarossa.samfundet.no ([193.35.52.29]:54338 "EHLO cassarossa.samfundet.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965524AbcBCWJ1 (ORCPT ); Wed, 3 Feb 2016 17:09:27 -0500 In-Reply-To: <20160203212317.GA7506@kroah.com> References: <20160203212317.GA7506@kroah.com> From: "Steinar H. Gunderson" Date: Wed, 3 Feb 2016 22:58:26 +0100 Subject: [PATCH v4] Add support for usbfs zerocopy. To: Greg Kroah-Hartman Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stern@rowland.harvard.edu Message-Id: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 59e7a33..a9fc6ff 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; @@ -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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965566AbcBCWJj (ORCPT ); Wed, 3 Feb 2016 17:09:39 -0500 Received: from mail-wm0-f51.google.com ([74.125.82.51]:34056 "EHLO mail-wm0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965373AbcBCWJV (ORCPT ); Wed, 3 Feb 2016 17:09:21 -0500 Date: Wed, 3 Feb 2016 23:09:16 +0100 From: "Steinar H. Gunderson" To: Greg Kroah-Hartman Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stern@rowland.harvard.edu Subject: Re: [PATCH v2] Add support for usbfs zerocopy. Message-ID: <20160203220911.GA1675@imap.gmail.com> References: <20160106001143.GA1171@kroah.com> <20160124211208.GA8696@kroah.com> <20160125080352.GA30907@imap.gmail.com> <20160203212317.GA7506@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160203212317.GA7506@kroah.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756502AbcBCWQJ (ORCPT ); Wed, 3 Feb 2016 17:16:09 -0500 Received: from mail-lf0-f48.google.com ([209.85.215.48]:33244 "EHLO mail-lf0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756428AbcBCWQH (ORCPT ); Wed, 3 Feb 2016 17:16:07 -0500 From: Felipe Balbi To: "Steinar H. Gunderson" , Greg Kroah-Hartman Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stern@rowland.harvard.edu Subject: Re: [PATCH v2] Add support for usbfs zerocopy. In-Reply-To: <20160203220911.GA1675@imap.gmail.com> References: <20160106001143.GA1171@kroah.com> <20160124211208.GA8696@kroah.com> <20160125080352.GA30907@imap.gmail.com> <20160203212317.GA7506@kroah.com> <20160203220911.GA1675@imap.gmail.com> User-Agent: Notmuch/0.21 (http://notmuchmail.org) Emacs/25.0.90.1 (x86_64-pc-linux-gnu) Date: Thu, 04 Feb 2016 00:15:50 +0200 Message-ID: <878u31phih.fsf@ti.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha1; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable "Steinar H. Gunderson" 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 be= st > 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 f= ind > 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=3Dabcd-m= essage-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 =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWsnwWAAoJEIaOsuA1yqREGHQP/2eEWgawLIXzULXt/O071/By /rT/8Fvh9TWwRgFFbIaqDRNdP8K7MowZWhPkzcgB8exZcDLTiAa4+5D5Ac6kePKy aHGFvuXCShqg493iZjDoiIhx7EDc0MhevE9Pi86PfH21TLRyoVVYZbaSK9xrJCZL QGWeMs1ERbUmEhUWStRJKiJbKTuSVzMPWugcutYtrrtUo6y3cRT9RmHcCrmCz0H4 xzd9wNscrFIO0gr2ATiJLZmRjsNpY0ISxTR2F0QK2o2GmJ/xkFl6x2sUdjNYBUAC HXsL8nuCMznHIgdKU7cM4MuLCjiLD+btWaQtIDga7p97TfGTS0pfgd12hzXsR2zb BFpOyk5qUITLSr2haaeaRMnRGWD1fyfZSy3DUZqML/yiM35JdXGYb8LfQ3kX23iB GK2x2a6SErtRJE7Xb0uBO9Kj0uEkSXwCuz/Sq2QIUR8HW/JbWMIoc8St6K8/Tx8w ulNpR/CPXj+ZlmOr7Bk4Qf6t6ZR0bCm8r3y+WnkzmgANAt0q7HCB04uuTs//13lo b6mTZD59hAh47KqyAYmkuO9U79LViHeHR25Jgg/IKDJKXVY6ZFlEoO/i542IjqjM u/8SPJwq+fZ1OPf5deNFfg7WmZlMQ1is+f6auAtZuH/oCYuSoAbEat4w/6uE72aH /pIYiUqdOVHmjSipPpae =tnUj -----END PGP SIGNATURE----- --=-=-=-- From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755536AbcBCXks (ORCPT ); Wed, 3 Feb 2016 18:40:48 -0500 Received: from mail-wm0-f54.google.com ([74.125.82.54]:38814 "EHLO mail-wm0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752279AbcBCXkn (ORCPT ); Wed, 3 Feb 2016 18:40:43 -0500 Date: Thu, 4 Feb 2016 00:40:39 +0100 From: "Steinar H. Gunderson" To: Felipe Balbi Cc: Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stern@rowland.harvard.edu Subject: Re: [PATCH v2] Add support for usbfs zerocopy. Message-ID: <20160203234035.GA3533@imap.gmail.com> References: <20160106001143.GA1171@kroah.com> <20160124211208.GA8696@kroah.com> <20160125080352.GA30907@imap.gmail.com> <20160203212317.GA7506@kroah.com> <20160203220911.GA1675@imap.gmail.com> <878u31phih.fsf@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <878u31phih.fsf@ti.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934083AbcBDKTz (ORCPT ); Thu, 4 Feb 2016 05:19:55 -0500 Received: from canardo.mork.no ([148.122.252.1]:44462 "EHLO canardo.mork.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933282AbcBDKTw convert rfc822-to-8bit (ORCPT ); Thu, 4 Feb 2016 05:19:52 -0500 From: =?utf-8?Q?Bj=C3=B8rn_Mork?= To: "Steinar H. Gunderson" Cc: Felipe Balbi , Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stern@rowland.harvard.edu Subject: Re: [PATCH v2] Add support for usbfs zerocopy. Organization: m References: <20160106001143.GA1171@kroah.com> <20160124211208.GA8696@kroah.com> <20160125080352.GA30907@imap.gmail.com> <20160203212317.GA7506@kroah.com> <20160203220911.GA1675@imap.gmail.com> <878u31phih.fsf@ti.com> <20160203234035.GA3533@imap.gmail.com> Date: Thu, 04 Feb 2016 11:17:26 +0100 In-Reply-To: <20160203234035.GA3533@imap.gmail.com> (Steinar H. Gunderson's message of "Thu, 4 Feb 2016 00:40:39 +0100") Message-ID: <87lh704w5l.fsf@nemi.mork.no> User-Agent: Gnus/5.130013 (Ma Gnus v0.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org "Steinar H. Gunderson" 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756356AbcBDK0u (ORCPT ); Thu, 4 Feb 2016 05:26:50 -0500 Received: from mail-wm0-f47.google.com ([74.125.82.47]:35185 "EHLO mail-wm0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756248AbcBDK0r (ORCPT ); Thu, 4 Feb 2016 05:26:47 -0500 Date: Thu, 4 Feb 2016 11:26:43 +0100 From: "Steinar H. Gunderson" To: =?iso-8859-1?Q?Bj=F8rn?= Mork Cc: Felipe Balbi , Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stern@rowland.harvard.edu Subject: Re: [PATCH v2] Add support for usbfs zerocopy. Message-ID: <20160204102643.GA88749@imap.gmail.com> References: <20160106001143.GA1171@kroah.com> <20160124211208.GA8696@kroah.com> <20160125080352.GA30907@imap.gmail.com> <20160203212317.GA7506@kroah.com> <20160203220911.GA1675@imap.gmail.com> <878u31phih.fsf@ti.com> <20160203234035.GA3533@imap.gmail.com> <87lh704w5l.fsf@nemi.mork.no> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <87lh704w5l.fsf@nemi.mork.no> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751189AbcBLUzv (ORCPT ); Fri, 12 Feb 2016 15:55:51 -0500 Received: from mail-wm0-f51.google.com ([74.125.82.51]:35556 "EHLO mail-wm0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751055AbcBLUzj (ORCPT ); Fri, 12 Feb 2016 15:55:39 -0500 Date: Fri, 12 Feb 2016 21:55:36 +0100 From: "Steinar H. Gunderson" To: Greg Kroah-Hartman Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stern@rowland.harvard.edu Subject: Re: [PATCH v2] Add support for usbfs zerocopy. Message-ID: <20160212205536.GA39638@imap.gmail.com> References: <20160106001143.GA1171@kroah.com> <20160124211208.GA8696@kroah.com> <20160125080352.GA30907@imap.gmail.com> <20160203212317.GA7506@kroah.com> <20160203220911.GA1675@imap.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160203220911.GA1675@imap.gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755657AbcBXTaR (ORCPT ); Wed, 24 Feb 2016 14:30:17 -0500 Received: from aserp1040.oracle.com ([141.146.126.69]:16525 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752316AbcBXTaP (ORCPT ); Wed, 24 Feb 2016 14:30:15 -0500 Subject: Re: [PATCH] Add support for usbfs zerocopy. To: "Steinar H. Gunderson" , Greg Kroah-Hartman References: Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stern@rowland.harvard.edu From: Sasha Levin Message-ID: <56CE04C0.4050603@oracle.com> Date: Wed, 24 Feb 2016 14:30:08 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Source-IP: userv0022.oracle.com [156.151.31.74] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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] 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756759AbcBXTdI (ORCPT ); Wed, 24 Feb 2016 14:33:08 -0500 Received: from mail-wm0-f43.google.com ([74.125.82.43]:37253 "EHLO mail-wm0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753126AbcBXTdF (ORCPT ); Wed, 24 Feb 2016 14:33:05 -0500 Date: Wed, 24 Feb 2016 20:33:01 +0100 From: "Steinar H. Gunderson" To: Sasha Levin Cc: Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stern@rowland.harvard.edu Subject: Re: [PATCH] Add support for usbfs zerocopy. Message-ID: <20160224193257.GA24059@imap.gmail.com> References: <56CE04C0.4050603@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56CE04C0.4050603@oracle.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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