On Wed, Feb 02, 2022 at 06:20:56PM +0100, Ingo Rohloff wrote:
> This patch implements the same functionality for FunctionFS as
> commit f7d34b445abc00e979b7 ("USB: Add support for usbfs zerocopy.")
> did for USB host devio.c
>
> For FunctionFS, each "struct file *" for an endpoint (except EP0), keeps
> a list of mmapped buffers. User space might use these buffers to avoid
> copying of data by the kernel, by employing Linux native AsyncIO via
> libaio.
>
> Standard read() and write() operations will NOT be zerocopy.
> Especially for reads, the expected USB transfer length is unclear;
> whereas an AIO request clearly specifies the maximum transfer length.
>
> Signed-off-by: Ingo Rohloff <ingo.rohloff@xxxxxxxxxxxxxx>
> ---
> drivers/usb/gadget/function/f_fs.c | 269 ++++++++++++++++++++++++++++-
> drivers/usb/gadget/function/u_fs.h | 4 +
> 2 files changed, 267 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
> index 7461d27e9604..a2e46704426f 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -201,6 +201,23 @@ struct ffs_epfile {
> unsigned char _pad;
> };
>
> +struct ffs_ep_listener {
> + struct ffs_epfile *epfile;
> + struct list_head ffsm_list;
> + spinlock_t ffsm_lock;
> +};
> +
> +struct ffs_memory {
> + struct list_head memlist;
> + struct ffs_ep_listener *listener;
> + char *kmem;
> + unsigned long vm_start;
> + u32 size;
> +
> + int vma_use_count;
> + int aio_use_count;
> +};
> +
> struct ffs_buffer {
> size_t length;
> char *data;
> @@ -227,6 +244,7 @@ struct ffs_io_data {
> bool use_sg;
>
> struct ffs_data *ffs;
> + struct ffs_memory *ffsm;
> };
>
> struct ffs_desc_helper {
> @@ -702,6 +720,200 @@ static const struct file_operations ffs_ep0_operations = {
> .poll = ffs_ep0_poll,
> };
>
> +/* Handling of mmapped transfers *******************************/
> +
> +/* Check whether it's okay to allocate more memory for mmap */
> +static int ffsm_increase_mmap_mem_usage(struct ffs_data *ffs, u64 amount)
> +{
> + u64 lim;
> +
> + lim = READ_ONCE(ffs->mmap_memory_mb);
> + lim <<= 20;
> +
> + atomic64_add(amount, &ffs->mmap_mem_usage);
> +
> + if (lim > 0 && atomic64_read(&ffs->mmap_mem_usage) > lim) {
What prevents it from changing right after you read this?
> + atomic64_sub(amount, &ffs->mmap_mem_usage);
Why not use a real lock instead of trying to do a fake one with this
atomic variable? You are not really protecting anything here correctly
from what I can tell.
thanks,
greg k-h
