[PATCH] USB: add __get_free_pages() in hcd_buffer_alloc function

[PATCH] USB: add __get_free_pages() in hcd_buffer_alloc function

[qiang.zhang]

From: Zqiang <qiang.zhang@windriver.com>

When USB hosts just use PIO, allocate memory by slab/slub
memory manager, if the required memory size is larger than
one or more page sizes, need allocate memory directly from
buddy systems.

Signed-off-by: Zqiang <qiang.zhang@windriver.com>
---
 drivers/usb/core/buffer.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c
index fbb087b728dc..28e7db9f93d5 100644
--- a/drivers/usb/core/buffer.c
+++ b/drivers/usb/core/buffer.c
@@ -131,7 +131,8 @@ void *hcd_buffer_alloc(
 	/* some USB hosts just use PIO */
 	if (!hcd_uses_dma(hcd)) {
 		*dma = ~(dma_addr_t) 0;
-		return kmalloc(size, mem_flags);
+		return size < PAGE_SIZE ? kmalloc(size, mem_flags)
+					: __get_free_pages(mem_flags, get_order(size));
 	}
 
 	for (i = 0; i < HCD_BUFFER_POOLS; i++) {
@@ -160,8 +161,8 @@ void hcd_buffer_free(
 	}
 
 	if (!hcd_uses_dma(hcd)) {
-		kfree(addr);
-		return;
+		return size < PAGE_SIZE ? kfree(addr)
+					: free_pages(addr, get_order(size));
 	}
 
 	for (i = 0; i < HCD_BUFFER_POOLS; i++) {
-- 
2.17.1

Re: [PATCH] USB: add __get_free_pages() in hcd_buffer_alloc function

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 4234 bytes --]

Hi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on v5.11-rc1 next-20201223]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/qiang-zhang-windriver-com/USB-add-__get_free_pages-in-hcd_buffer_alloc-function/20210103-162325
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: x86_64-randconfig-a006-20210103 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 7af6a134508cd1c7f75c6e3441ce436f220f30a4)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/904936636c0b20b1d9b2be051502c6d80c5fa1ee
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review qiang-zhang-windriver-com/USB-add-__get_free_pages-in-hcd_buffer_alloc-function/20210103-162325
        git checkout 904936636c0b20b1d9b2be051502c6d80c5fa1ee
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/usb/core/buffer.c:134:27: warning: pointer/integer type mismatch in conditional expression ('void *' and 'unsigned long') [-Wconditional-type-mismatch]
                   return size < PAGE_SIZE ? kmalloc(size, mem_flags)
                                           ^ ~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/usb/core/buffer.c:165:19: warning: incompatible pointer to integer conversion passing 'void *' to parameter of type 'unsigned long' [-Wint-conversion]
                                           : free_pages(addr, get_order(size));
                                                        ^~~~
   include/linux/gfp.h:582:38: note: passing argument to parameter 'addr' here
   extern void free_pages(unsigned long addr, unsigned int order);
                                        ^
   2 warnings generated.


vim +134 drivers/usb/core/buffer.c

   109	
   110	
   111	/* sometimes alloc/free could use kmalloc with GFP_DMA, for
   112	 * better sharing and to leverage mm/slab.c intelligence.
   113	 */
   114	
   115	void *hcd_buffer_alloc(
   116		struct usb_bus		*bus,
   117		size_t			size,
   118		gfp_t			mem_flags,
   119		dma_addr_t		*dma
   120	)
   121	{
   122		struct usb_hcd		*hcd = bus_to_hcd(bus);
   123		int			i;
   124	
   125		if (size == 0)
   126			return NULL;
   127	
   128		if (hcd->localmem_pool)
   129			return gen_pool_dma_alloc(hcd->localmem_pool, size, dma);
   130	
   131		/* some USB hosts just use PIO */
   132		if (!hcd_uses_dma(hcd)) {
   133			*dma = ~(dma_addr_t) 0;
 > 134			return size < PAGE_SIZE ? kmalloc(size, mem_flags)
   135						: __get_free_pages(mem_flags, get_order(size));
   136		}
   137	
   138		for (i = 0; i < HCD_BUFFER_POOLS; i++) {
   139			if (size <= pool_max[i])
   140				return dma_pool_alloc(hcd->pool[i], mem_flags, dma);
   141		}
   142		return dma_alloc_coherent(hcd->self.sysdev, size, dma, mem_flags);
   143	}
   144	
   145	void hcd_buffer_free(
   146		struct usb_bus		*bus,
   147		size_t			size,
   148		void			*addr,
   149		dma_addr_t		dma
   150	)
   151	{
   152		struct usb_hcd		*hcd = bus_to_hcd(bus);
   153		int			i;
   154	
   155		if (!addr)
   156			return;
   157	
   158		if (hcd->localmem_pool) {
   159			gen_pool_free(hcd->localmem_pool, (unsigned long)addr, size);
   160			return;
   161		}
   162	
   163		if (!hcd_uses_dma(hcd)) {
   164			return size < PAGE_SIZE ? kfree(addr)
 > 165						: free_pages(addr, get_order(size));

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 45088 bytes --]

Re: [PATCH] USB: add __get_free_pages() in hcd_buffer_alloc function

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 4264 bytes --]

Hi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on v5.11-rc1 next-20201223]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/qiang-zhang-windriver-com/USB-add-__get_free_pages-in-hcd_buffer_alloc-function/20210103-162325
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: parisc-randconfig-r033-20210103 (attached as .config)
compiler: hppa-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/904936636c0b20b1d9b2be051502c6d80c5fa1ee
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review qiang-zhang-windriver-com/USB-add-__get_free_pages-in-hcd_buffer_alloc-function/20210103-162325
        git checkout 904936636c0b20b1d9b2be051502c6d80c5fa1ee
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=parisc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/usb/core/buffer.c: In function 'hcd_buffer_alloc':
>> drivers/usb/core/buffer.c:135:6: warning: pointer/integer type mismatch in conditional expression
     135 |      : __get_free_pages(mem_flags, get_order(size));
         |      ^
   drivers/usb/core/buffer.c: In function 'hcd_buffer_free':
>> drivers/usb/core/buffer.c:165:19: warning: passing argument 1 of 'free_pages' makes integer from pointer without a cast [-Wint-conversion]
     165 |      : free_pages(addr, get_order(size));
         |                   ^~~~
         |                   |
         |                   void *
   In file included from include/linux/umh.h:4,
                    from include/linux/kmod.h:9,
                    from include/linux/module.h:16,
                    from drivers/usb/core/buffer.c:11:
   include/linux/gfp.h:582:38: note: expected 'long unsigned int' but argument is of type 'void *'
     582 | extern void free_pages(unsigned long addr, unsigned int order);
         |                        ~~~~~~~~~~~~~~^~~~


vim +135 drivers/usb/core/buffer.c

   109	
   110	
   111	/* sometimes alloc/free could use kmalloc with GFP_DMA, for
   112	 * better sharing and to leverage mm/slab.c intelligence.
   113	 */
   114	
   115	void *hcd_buffer_alloc(
   116		struct usb_bus		*bus,
   117		size_t			size,
   118		gfp_t			mem_flags,
   119		dma_addr_t		*dma
   120	)
   121	{
   122		struct usb_hcd		*hcd = bus_to_hcd(bus);
   123		int			i;
   124	
   125		if (size == 0)
   126			return NULL;
   127	
   128		if (hcd->localmem_pool)
   129			return gen_pool_dma_alloc(hcd->localmem_pool, size, dma);
   130	
   131		/* some USB hosts just use PIO */
   132		if (!hcd_uses_dma(hcd)) {
   133			*dma = ~(dma_addr_t) 0;
   134			return size < PAGE_SIZE ? kmalloc(size, mem_flags)
 > 135						: __get_free_pages(mem_flags, get_order(size));
   136		}
   137	
   138		for (i = 0; i < HCD_BUFFER_POOLS; i++) {
   139			if (size <= pool_max[i])
   140				return dma_pool_alloc(hcd->pool[i], mem_flags, dma);
   141		}
   142		return dma_alloc_coherent(hcd->self.sysdev, size, dma, mem_flags);
   143	}
   144	
   145	void hcd_buffer_free(
   146		struct usb_bus		*bus,
   147		size_t			size,
   148		void			*addr,
   149		dma_addr_t		dma
   150	)
   151	{
   152		struct usb_hcd		*hcd = bus_to_hcd(bus);
   153		int			i;
   154	
   155		if (!addr)
   156			return;
   157	
   158		if (hcd->localmem_pool) {
   159			gen_pool_free(hcd->localmem_pool, (unsigned long)addr, size);
   160			return;
   161		}
   162	
   163		if (!hcd_uses_dma(hcd)) {
   164			return size < PAGE_SIZE ? kfree(addr)
 > 165						: free_pages(addr, get_order(size));

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 40576 bytes --]

Re: [PATCH] USB: add __get_free_pages() in hcd_buffer_alloc function

[Greg KH]

On Sun, Jan 03, 2021 at 04:19:02PM +0800, qiang.zhang@windriver.com wrote:
> From: Zqiang <qiang.zhang@windriver.com>
> 
> When USB hosts just use PIO, allocate memory by slab/slub
> memory manager, if the required memory size is larger than
> one or more page sizes, need allocate memory directly from
> buddy systems.

That says _what_ you are doing, but not _why_ you need to do this.

What in-tree host controller has this problem and on what platform?

> 
> Signed-off-by: Zqiang <qiang.zhang@windriver.com>
> ---
>  drivers/usb/core/buffer.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c
> index fbb087b728dc..28e7db9f93d5 100644
> --- a/drivers/usb/core/buffer.c
> +++ b/drivers/usb/core/buffer.c
> @@ -131,7 +131,8 @@ void *hcd_buffer_alloc(
>  	/* some USB hosts just use PIO */
>  	if (!hcd_uses_dma(hcd)) {
>  		*dma = ~(dma_addr_t) 0;
> -		return kmalloc(size, mem_flags);
> +		return size < PAGE_SIZE ? kmalloc(size, mem_flags)
> +					: __get_free_pages(mem_flags, get_order(size));

Please never use ? : statements if at all possible.  Use a real if ()
instead.

Oh, and make sure your code actually builds properly :(

What platforms did you test this on?

thanks,

greg k-h

回复: [PATCH] USB: add __get_free_pages() in hcd_buffer_alloc function

[Zhang, Qiang]

________________________________________
发件人: Greg KH <gregkh@linuxfoundation.org>
发送时间: 2021年1月3日 19:03
收件人: Zhang, Qiang
抄送: stern@rowland.harvard.edu; linux-usb@vger.kernel.org
主题: Re: [PATCH] USB: add __get_free_pages() in hcd_buffer_alloc function

On Sun, Jan 03, 2021 at 04:19:02PM +0800, qiang.zhang@windriver.com wrote:
> From: Zqiang <qiang.zhang@windriver.com>
>
> When USB hosts just use PIO, allocate memory by slab/slub
> memory manager, if the required memory size is larger than
> one or more page sizes, need allocate memory directly from
> buddy systems.

>That says _what_ you are doing, but not _why_ you need to do this.

>What in-tree host controller has this problem and on what platform?

>
> Signed-off-by: Zqiang <qiang.zhang@windriver.com>
> ---
>  drivers/usb/core/buffer.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c
> index fbb087b728dc..28e7db9f93d5 100644
> --- a/drivers/usb/core/buffer.c
> +++ b/drivers/usb/core/buffer.c
> @@ -131,7 +131,8 @@ void *hcd_buffer_alloc(
>       /* some USB hosts just use PIO */
>       if (!hcd_uses_dma(hcd)) {
>               *dma = ~(dma_addr_t) 0;
> -             return kmalloc(size, mem_flags);
> +             return size < PAGE_SIZE ? kmalloc(size, mem_flags)
> +                                     : __get_free_pages(mem_flags, get_order(size));

>Please never use ? : statements if at all possible.  Use a real if ()
>instead.

>Oh, and make sure your code actually builds properly :(

 I'm very sorry. It was my mistake, I will modify and resend.  
 if some hcd hosts not support DMA  and localmem_pool，
 when we call usb_alloc_coherent function ，It's  actually 
 just a call kmalloc function, it is only suitable for allocating 
 memory smaller than one pagesize,  if size is larger than the 
 size of a pagesize, need allocate memory directly from buddy 
 system.

 
>What platforms did you test this on?

>thanks,

>greg k-h