diff mbox series

usb: gadget: f_fs: Allow scatter-gather buffers

Message ID 20181106162640.4633-1-andrzej.p@samsung.com (mailing list archive)
State New, archived
Headers show
Series usb: gadget: f_fs: Allow scatter-gather buffers | expand

Commit Message

Andrzej Pietrasiewicz Nov. 6, 2018, 4:26 p.m. UTC
Some protocols implemented in userspace with FunctionFS might require large
buffers, e.g. 64kB or more. Currently the said memory is allocated with
kmalloc, which might fail should system memory be highly fragmented.

On the other hand, some UDC hardware allows scatter-gather operation and
this patch takes advantage of this capability: if the requested buffer
is larger than PAGE_SIZE and the UDC allows scatter-gather operation, then
the buffer is allocated with vmalloc and a scatterlist describing it is
created and passed to usb request.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
---

@Felipe: This time I used your correct address.

 drivers/usb/gadget/function/f_fs.c | 92 +++++++++++++++++++++++++++++++++++---
 1 file changed, 87 insertions(+), 5 deletions(-)

Comments

kernel test robot Nov. 9, 2018, 12:33 p.m. UTC | #1
Hi Andrzej,

I love your patch! Yet something to improve:

[auto build test ERROR on balbi-usb/next]
[also build test ERROR on v4.20-rc1 next-20181109]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Andrzej-Pietrasiewicz/usb-gadget-f_fs-Allow-scatter-gather-buffers/20181109-194916
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: sparc64-allyesconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=sparc64 

All error/warnings (new ones prefixed by >>):

   drivers/usb/gadget/function/f_fs.c: In function 'ffs_build_sg_list':
>> drivers/usb/gadget/function/f_fs.c:769:10: error: implicit declaration of function 'vmalloc'; did you mean 'kvmalloc'? [-Werror=implicit-function-declaration]
     vaddr = vmalloc(size);
             ^~~~~~~
             kvmalloc
>> drivers/usb/gadget/function/f_fs.c:769:8: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
     vaddr = vmalloc(size);
           ^
>> drivers/usb/gadget/function/f_fs.c:778:3: error: implicit declaration of function 'vfree'; did you mean 'kvfree'? [-Werror=implicit-function-declaration]
      vfree(vaddr);
      ^~~~~
      kvfree
   cc1: some warnings being treated as errors

vim +769 drivers/usb/gadget/function/f_fs.c

   755	
   756	/*
   757	 * allocate a virtually contiguous buffer and create a scatterlist describing it
   758	 * @sg_table	- pointer to a place to be filled with sg_table contents
   759	 * @size	- required buffer size
   760	 */
   761	static void *ffs_build_sg_list(struct sg_table *sg_table, size_t size)
   762	{
   763		struct page **pages;
   764		void *vaddr;
   765		unsigned long ptr;
   766		unsigned int n_pages;
   767		int i;
   768	
 > 769		vaddr = vmalloc(size);
   770		if (!vaddr)
   771			return NULL;
   772	
   773		n_pages =  (PAGE_ALIGN((unsigned long)vaddr + size) -
   774			((unsigned long)vaddr & PAGE_MASK))
   775			>> PAGE_SHIFT;
   776		pages = kvmalloc_array(n_pages, sizeof(struct page *), GFP_KERNEL);
   777		if (!pages) {
 > 778			vfree(vaddr);
   779	
   780			return NULL;
   781		}
   782		for (i = 0, ptr = (unsigned long)vaddr & PAGE_MASK; i < n_pages;
   783			++i, ptr += PAGE_SIZE)
   784			pages[i] = vmalloc_to_page((void *)ptr);
   785	
   786		if (sg_alloc_table_from_pages(sg_table, pages, n_pages,
   787			((unsigned long)vaddr) & ~PAGE_MASK, size, GFP_KERNEL)) {
   788			kvfree(pages);
   789			vfree(vaddr);
   790	
   791			return NULL;
   792		}
   793		kvfree(pages);
   794	
   795		return vaddr;
   796	}
   797	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox series

Patch

diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index 3ada83d..e83fa63 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -19,6 +19,7 @@ 
 #include <linux/export.h>
 #include <linux/hid.h>
 #include <linux/module.h>
+#include <linux/scatterlist.h>
 #include <linux/sched/signal.h>
 #include <linux/uio.h>
 #include <asm/unaligned.h>
@@ -219,6 +220,8 @@  struct ffs_io_data {
 
 	struct usb_ep *ep;
 	struct usb_request *req;
+	struct sg_table sgt;
+	bool use_sg;
 
 	struct ffs_data *ffs;
 };
@@ -750,6 +753,70 @@  static ssize_t ffs_copy_to_iter(void *data, int data_len, struct iov_iter *iter)
 	return ret;
 }
 
+/*
+ * allocate a virtually contiguous buffer and create a scatterlist describing it
+ * @sg_table	- pointer to a place to be filled with sg_table contents
+ * @size	- required buffer size
+ */
+static void *ffs_build_sg_list(struct sg_table *sg_table, size_t size)
+{
+	struct page **pages;
+	void *vaddr;
+	unsigned long ptr;
+	unsigned int n_pages;
+	int i;
+
+	vaddr = vmalloc(size);
+	if (!vaddr)
+		return NULL;
+
+	n_pages =  (PAGE_ALIGN((unsigned long)vaddr + size) -
+		((unsigned long)vaddr & PAGE_MASK))
+		>> PAGE_SHIFT;
+	pages = kvmalloc_array(n_pages, sizeof(struct page *), GFP_KERNEL);
+	if (!pages) {
+		vfree(vaddr);
+
+		return NULL;
+	}
+	for (i = 0, ptr = (unsigned long)vaddr & PAGE_MASK; i < n_pages;
+		++i, ptr += PAGE_SIZE)
+		pages[i] = vmalloc_to_page((void *)ptr);
+
+	if (sg_alloc_table_from_pages(sg_table, pages, n_pages,
+		((unsigned long)vaddr) & ~PAGE_MASK, size, GFP_KERNEL)) {
+		kvfree(pages);
+		vfree(vaddr);
+
+		return NULL;
+	}
+	kvfree(pages);
+
+	return vaddr;
+}
+
+static inline void *ffs_alloc_buffer(struct ffs_io_data *io_data,
+	size_t data_len)
+{
+	if (io_data->use_sg)
+		return ffs_build_sg_list(&io_data->sgt, data_len);
+
+	return kmalloc(data_len, GFP_KERNEL);
+}
+
+static inline void ffs_free_buffer(struct ffs_io_data *io_data)
+{
+	if (!io_data->buf)
+		return;
+
+	if (io_data->use_sg) {
+		sg_free_table(&io_data->sgt);
+		vfree(io_data->buf);
+	} else {
+		kfree(io_data->buf);
+	}
+}
+
 static void ffs_user_copy_worker(struct work_struct *work)
 {
 	struct ffs_io_data *io_data = container_of(work, struct ffs_io_data,
@@ -777,7 +844,7 @@  static void ffs_user_copy_worker(struct work_struct *work)
 
 	if (io_data->read)
 		kfree(io_data->to_free);
-	kfree(io_data->buf);
+	ffs_free_buffer(io_data);
 	kfree(io_data);
 }
 
@@ -933,6 +1000,7 @@  static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
 		 * earlier
 		 */
 		gadget = epfile->ffs->gadget;
+		io_data->use_sg = gadget->sg_supported && data_len > PAGE_SIZE;
 
 		spin_lock_irq(&epfile->ffs->eps_lock);
 		/* In the meantime, endpoint got disabled or changed. */
@@ -949,7 +1017,7 @@  static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
 			data_len = usb_ep_align_maybe(gadget, ep->ep, data_len);
 		spin_unlock_irq(&epfile->ffs->eps_lock);
 
-		data = kmalloc(data_len, GFP_KERNEL);
+		data = ffs_alloc_buffer(io_data, data_len);
 		if (unlikely(!data)) {
 			ret = -ENOMEM;
 			goto error_mutex;
@@ -989,9 +1057,17 @@  static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
 		bool interrupted = false;
 
 		req = ep->req;
-		req->buf      = data;
+		if (io_data->use_sg) {
+			req->buf      = NULL;
+			req->sg	= io_data->sgt.sgl;
+			req->num_sgs = io_data->sgt.nents;
+		} else {
+			req->buf      = data;
+		}
 		req->length   = data_len;
 
+		io_data->buf = data;
+
 		req->context  = &done;
 		req->complete = ffs_epfile_io_complete;
 
@@ -1023,7 +1099,13 @@  static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
 	} else if (!(req = usb_ep_alloc_request(ep->ep, GFP_ATOMIC))) {
 		ret = -ENOMEM;
 	} else {
-		req->buf      = data;
+		if (io_data->use_sg) {
+			req->buf      = NULL;
+			req->sg	= io_data->sgt.sgl;
+			req->num_sgs = io_data->sgt.nents;
+		} else {
+			req->buf      = data;
+		}
 		req->length   = data_len;
 
 		io_data->buf = data;
@@ -1053,7 +1135,7 @@  static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
 error_mutex:
 	mutex_unlock(&epfile->mutex);
 error:
-	kfree(data);
+	ffs_free_buffer(io_data);
 	return ret;
 }