| Message ID | 20210103081902.2381-1-qiang.zhang@windriver.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | USB: add __get_free_pages() in hcd_buffer_alloc function | expand |
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
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
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
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++) {