[v3,0/4] usb: gadget: functionfs: DMABUF import interface

Hi,

This small patchset adds three new IOCTLs that can be used to attach,
detach, or transfer from/to a DMABUF object.

This interface is being used at Analog Devices, to transfer data from
high-speed transceivers to USB in a zero-copy fashion, using also the
DMABUF import interface to the IIO subsystem which is being upstreamed
in parallel [1] (and has high chances to be accepted for 5.9, I think
Jonathan can confirm). The two are used by the Libiio software [2].

On a ZCU102 board with a FMComms3 daughter board, using the combination
of these two new interfaces yields a drastic improvement of the
throughput, from about 127 MiB/s using IIO's buffer read/write interface
+ read/write to the FunctionFS endpoints, to about 274 MiB/s when
passing around DMABUFs, for a lower CPU usage (0.85 load avg. before,
vs. 0.65 after).

Right now, *technically* there are no users of this interface, as
Analog Devices wants to wait until both interfaces are accepted upstream
to merge the DMABUF code in Libiio into the main branch, and Jonathan
wants to wait and see if this patchset is accepted to greenlight the
DMABUF interface in IIO as well. I think this isn't really a problem;
once everybody is happy with its part of the cake, we can merge them all
at once.

This is obviously for 5.9, and based on next-20240108.

Changelog:

- [3/4]:
  - Inline to_ffs_dma_fence() which was called only once.
  - Simplify ffs_dma_resv_lock()
  - Add comment explaining why we unref twice in ffs_dmabuf_detach()
  - Document uapi struct usb_ffs_dmabuf_transfer_req and IOCTLs
- [4/4]: New patch, as I figured out having documentation wouldn't hurt.

Cheers,
-Paul

[1] https://lore.kernel.org/linux-iio/219abc43b4fdd4a13b307ed2efaa0e6869e68e3f.camel@gmail.com/T/
[2] https://github.com/analogdevicesinc/libiio/tree/pcercuei/dev-new-dmabuf-api

Paul Cercueil (4):
  usb: gadget: Support already-mapped DMA SGs
  usb: gadget: functionfs: Factorize wait-for-endpoint code
  usb: gadget: functionfs: Add DMABUF import interface
  Documentation: usb: Document FunctionFS DMABUF API

 Documentation/usb/functionfs.rst    |  36 +++
 drivers/usb/gadget/function/f_fs.c  | 463 ++++++++++++++++++++++++++--
 drivers/usb/gadget/udc/core.c       |   7 +-
 include/linux/usb/gadget.h          |   2 +
 include/uapi/linux/usb/functionfs.h |  41 +++
 5 files changed, 528 insertions(+), 21 deletions(-)

Hi Vegard,

Le mardi 09 janvier 2024 à 14:08 +0100, Vegard Nossum a écrit :
> On 08/01/2024 13:00, Paul Cercueil wrote:
> > Add documentation for the three ioctls used to attach or detach
> > externally-created DMABUFs, and to request transfers from/to
> > previously
> > attached DMABUFs.
> > 
> > Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> > 
> > ---
> > v3: New patch
> > ---
> >   Documentation/usb/functionfs.rst | 36
> > ++++++++++++++++++++++++++++++++
> >   1 file changed, 36 insertions(+)
> 
> Hi,
> 
> I'd like to point out that this file (usb/functionfs.rst) is
> currently
> included by Documentation/subsystem-apis.rst, the top-level file for
> the
> "Kernel subsystem documentation" set of books, which describe
> internal
> APIs: "These books get into the details of how specific kernel
> subsystems work from the point of view of a kernel developer".
> 
> However, functionfs.rst (and especially your new additions) are
> documenting a userspace API, so it really belongs somewhere in
> Documentation/userspace-api/ -- that's where /proc, /sys, /dev and
> ioctl
> descriptions for userspace programmers belong.

Agreed. Even the original content prior to my additions describe a
userspace API.

> 
> I'm not NAKing the patch -- I just want to draw attention to this
> discrepancy. Maybe we can separate the kernel-implementation details
> (stuff about __init sections and stuff) from the new ioctl() info?
> 
> Looking at <https://docs.kernel.org/usb/> I see that there are many
> other adjacent documents that are also not really documenting kernel
> implementation details, rough categorization as follows:
> 
> USB support
> -----------
> 
> - Linux ACM driver v0.16 ==> admin/user info
> - Authorizing (or not) your USB devices to connect to the system ==> 
> admin/user info
> - ChipIdea Highspeed Dual Role Controller Driver => admin/user info
> - DWC3 driver ==> driver TODOs (can be moved into source code?)
> - EHCI driver ==> technical info + driver details
> - How FunctionFS works
> - Linux USB gadget configured through configfs ==> userspace API + 
> implementation
> - Linux USB HID gadget driver ==> implementation + userspace API
> - Multifunction Composite Gadget ==> technical + user info
> - Linux USB Printer Gadget Driver ==> userspace API
> - Linux Gadget Serial Driver v2.0 ==> user/admin + userspace API
> - Linux UVC Gadget Driver ==> user/admin + userspace API
> - Gadget Testing ==> user/admin + userspace API
> - Infinity Usb Unlimited Readme ==> user/admin
> - Mass Storage Gadget (MSG) ==> user/admin
> - USB 7-Segment Numeric Display ==> user/admin
> - mtouchusb driver ==> user/admin
> - OHCI ==> technical info
> - USB Raw Gadget ==> userspace API
> - USB/IP protocol ==> technical info
> - usbmon ==> user/admin + userspace API
> - USB serial ==> user/admin + technical info
> - USB references
> - Linux CDC ACM inf
> - Linux inf
> - USB devfs drop permissions source
> - Credits
> 
> By "admin/user info", I mean things that a user would have to do or
> run
> (e.g. modprobe + flags) to make use of a driver; "technical info" is
> more like device specifications (transfer speeds, modes of operation,
> etc.); "userspace API" is stuff like configfs and ioctls; "driver
> details" is really implementation details and internal
> considerations.
> 
> The last ones I don't even really know how to categorize.
> 
> I'm guessing nobody is really enthralled by the idea of splitting
> Documentation/usb/ up like this?
> 
>    Documentation/admin-guide/usb/
>    Documentation/driver-api/usb/ (this one actually exists already)
>    Documentation/userspace-api/usb/
> 
> For the stuff that is _actually_ internal to a specific driver (so
> not
> useful for end users, not useful for admins, not generic USB info,
> and
> not useful for userspace programmers), I would honestly propose to
> just
> move it directly into the driver's source code, or, if the text is
> obsolete, just get rid of it completely.
> 
> The distinction between user/admin and userspace API is pretty clear
> (one is for end users, the other is for userspace _programmers_), but
> it
> can sometimes be hard to determine whether something falls in one or
> the
> other category.
> 
> In any case -- it looks like almost all of the usb/ directory does
> not
> document "how specific kernel subsystems work from the point of view
> of
> a kernel developer" so maybe we should just move the include to
> userspace-api/ for now as an obvious improvement (if still not 100%
> correct):
> 
> diff --git a/Documentation/subsystem-apis.rst 
> b/Documentation/subsystem-apis.rst
> index 2d353fb8ea26..fe972f57bf4c 100644
> --- a/Documentation/subsystem-apis.rst
> +++ b/Documentation/subsystem-apis.rst
> @@ -81,7 +81,6 @@ Storage interfaces
>      security/index
>      crypto/index
>      bpf/index
> -   usb/index
>      PCI/index
>      misc-devices/index
>      peci/index
> diff --git a/Documentation/userspace-api/index.rst 
> b/Documentation/userspace-api/index.rst
> index 82f9dbd228f5..e60cd9174ada 100644
> --- a/Documentation/userspace-api/index.rst
> +++ b/Documentation/userspace-api/index.rst
> @@ -41,6 +41,7 @@ Subsystem-specific documentation:
>      tee
>      isapnp
>      dcdbas
> +   ../usb/index
> 
>   Kernel ABIs: These documents describe the the ABI between the Linux
>   kernel and userspace, and the relative stability of these
> interfaces.
> 
> 
> Thoughts?

Makes sense to me. There's definitely some cleanup to be done in the
USB documentation.

> Vegard

Cheers,
-Paul