Message ID | 20200716173247.78912-1-cuibixuan@huawei.com |
---|---|
State | Changes Requested |
Delegated to: | David Miller |
Headers | show |
Series | net: neterion: vxge: reduce stack usage in VXGE_COMPLETE_VPATH_TX | expand |
On Thu, 2020-07-16 at 17:32 +0000, Bixuan Cui wrote: > Fix the warning: [-Werror=-Wframe-larger-than=] [] > diff --git a/drivers/net/ethernet/neterion/vxge/vxge-main.c b/drivers/net/ethernet/neterion/vxge/vxge-main.c [] > @@ -100,8 +100,14 @@ static inline void VXGE_COMPLETE_VPATH_TX(struct vxge_fifo *fifo) > struct sk_buff **temp; > #define NR_SKB_COMPLETED 128 > struct sk_buff *completed[NR_SKB_COMPLETED]; > + struct sk_buff **completed; > int more; > > + completed = kcalloc(NR_SKB_COMPLETED, sizeof(*completed), > + GFP_KERNEL); I doubt this is a good idea. Check the callers interrupt status.
On 2020/7/16 17:46, Joe Perches wrote: > I doubt this is a good idea. > Check the callers interrupt status. yes, it's not good idea to alloc memory in interrupt handler, I will think more while fix warning. :) Thanks.
Hi Bixuan, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on sparc-next/master] [also build test WARNING on net-next/master net/master linus/master v5.8-rc5 next-20200716] [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/Bixuan-Cui/net-neterion-vxge-reduce-stack-usage-in-VXGE_COMPLETE_VPATH_TX/20200716-173219 base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc-next.git master config: s390-allyesconfig (attached as .config) compiler: s390-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 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=s390 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 >>): 91 | #define this_cpu_add_8(pcp, val) arch_this_cpu_add(pcp, val, "laag", "agsi", long) | ^~~~~~~~~~~~~~~~~ include/linux/percpu-defs.h:380:11: note: in expansion of macro 'this_cpu_add_8' 380 | case 8: stem##8(variable, __VA_ARGS__);break; \ | ^~~~ include/linux/percpu-defs.h:509:33: note: in expansion of macro '__pcpu_size_call' 509 | #define this_cpu_add(pcp, val) __pcpu_size_call(this_cpu_add_, pcp, val) | ^~~~~~~~~~~~~~~~ include/linux/percpu-defs.h:519:33: note: in expansion of macro 'this_cpu_add' 519 | #define this_cpu_sub(pcp, val) this_cpu_add(pcp, -(typeof(pcp))(val)) | ^~~~~~~~~~~~ include/linux/percpu-defs.h:521:28: note: in expansion of macro 'this_cpu_sub' 521 | #define this_cpu_dec(pcp) this_cpu_sub(pcp, 1) | ^~~~~~~~~~~~ include/net/sch_generic.h:872:2: note: in expansion of macro 'this_cpu_dec' 872 | this_cpu_dec(sch->cpu_qstats->qlen); | ^~~~~~~~~~~~ include/net/sch_generic.h: In function 'qdisc_qstats_cpu_requeues_inc': arch/s390/include/asm/percpu.h:74:21: warning: comparison is always true due to limited range of data type [-Wtype-limits] 74 | ((szcast)val__ > -129) && ((szcast)val__ < 128)) { \ | ^ arch/s390/include/asm/percpu.h:91:34: note: in expansion of macro 'arch_this_cpu_add' 91 | #define this_cpu_add_8(pcp, val) arch_this_cpu_add(pcp, val, "laag", "agsi", long) | ^~~~~~~~~~~~~~~~~ include/linux/percpu-defs.h:380:11: note: in expansion of macro 'this_cpu_add_8' 380 | case 8: stem##8(variable, __VA_ARGS__);break; \ | ^~~~ include/linux/percpu-defs.h:509:33: note: in expansion of macro '__pcpu_size_call' 509 | #define this_cpu_add(pcp, val) __pcpu_size_call(this_cpu_add_, pcp, val) | ^~~~~~~~~~~~~~~~ include/linux/percpu-defs.h:520:28: note: in expansion of macro 'this_cpu_add' 520 | #define this_cpu_inc(pcp) this_cpu_add(pcp, 1) | ^~~~~~~~~~~~ include/net/sch_generic.h:877:2: note: in expansion of macro 'this_cpu_inc' 877 | this_cpu_inc(sch->cpu_qstats->requeues); | ^~~~~~~~~~~~ include/net/sch_generic.h: In function 'qdisc_qstats_cpu_drop': arch/s390/include/asm/percpu.h:74:21: warning: comparison is always true due to limited range of data type [-Wtype-limits] 74 | ((szcast)val__ > -129) && ((szcast)val__ < 128)) { \ | ^ arch/s390/include/asm/percpu.h:91:34: note: in expansion of macro 'arch_this_cpu_add' 91 | #define this_cpu_add_8(pcp, val) arch_this_cpu_add(pcp, val, "laag", "agsi", long) | ^~~~~~~~~~~~~~~~~ include/linux/percpu-defs.h:380:11: note: in expansion of macro 'this_cpu_add_8' 380 | case 8: stem##8(variable, __VA_ARGS__);break; \ | ^~~~ include/linux/percpu-defs.h:509:33: note: in expansion of macro '__pcpu_size_call' 509 | #define this_cpu_add(pcp, val) __pcpu_size_call(this_cpu_add_, pcp, val) | ^~~~~~~~~~~~~~~~ include/linux/percpu-defs.h:520:28: note: in expansion of macro 'this_cpu_add' 520 | #define this_cpu_inc(pcp) this_cpu_add(pcp, 1) | ^~~~~~~~~~~~ include/net/sch_generic.h:902:2: note: in expansion of macro 'this_cpu_inc' 902 | this_cpu_inc(sch->cpu_qstats->drops); | ^~~~~~~~~~~~ include/net/sch_generic.h: In function 'qdisc_update_stats_at_enqueue': arch/s390/include/asm/percpu.h:74:21: warning: comparison is always true due to limited range of data type [-Wtype-limits] 74 | ((szcast)val__ > -129) && ((szcast)val__ < 128)) { \ | ^ arch/s390/include/asm/percpu.h:91:34: note: in expansion of macro 'arch_this_cpu_add' 91 | #define this_cpu_add_8(pcp, val) arch_this_cpu_add(pcp, val, "laag", "agsi", long) | ^~~~~~~~~~~~~~~~~ include/linux/percpu-defs.h:380:11: note: in expansion of macro 'this_cpu_add_8' 380 | case 8: stem##8(variable, __VA_ARGS__);break; \ | ^~~~ include/linux/percpu-defs.h:509:33: note: in expansion of macro '__pcpu_size_call' 509 | #define this_cpu_add(pcp, val) __pcpu_size_call(this_cpu_add_, pcp, val) | ^~~~~~~~~~~~~~~~ include/net/sch_generic.h:1101:3: note: in expansion of macro 'this_cpu_add' 1101 | this_cpu_add(sch->cpu_qstats->backlog, pkt_len); | ^~~~~~~~~~~~ include/net/sch_generic.h: In function 'mini_qdisc_qstats_cpu_drop': arch/s390/include/asm/percpu.h:74:21: warning: comparison is always true due to limited range of data type [-Wtype-limits] 74 | ((szcast)val__ > -129) && ((szcast)val__ < 128)) { \ | ^ arch/s390/include/asm/percpu.h:91:34: note: in expansion of macro 'arch_this_cpu_add' 91 | #define this_cpu_add_8(pcp, val) arch_this_cpu_add(pcp, val, "laag", "agsi", long) | ^~~~~~~~~~~~~~~~~ include/linux/percpu-defs.h:380:11: note: in expansion of macro 'this_cpu_add_8' 380 | case 8: stem##8(variable, __VA_ARGS__);break; \ | ^~~~ include/linux/percpu-defs.h:509:33: note: in expansion of macro '__pcpu_size_call' 509 | #define this_cpu_add(pcp, val) __pcpu_size_call(this_cpu_add_, pcp, val) | ^~~~~~~~~~~~~~~~ include/linux/percpu-defs.h:520:28: note: in expansion of macro 'this_cpu_add' 520 | #define this_cpu_inc(pcp) this_cpu_add(pcp, 1) | ^~~~~~~~~~~~ include/net/sch_generic.h:1270:2: note: in expansion of macro 'this_cpu_inc' 1270 | this_cpu_inc(miniq->cpu_qstats->drops); | ^~~~~~~~~~~~ drivers/net/ethernet/neterion/vxge/vxge-main.c: In function 'VXGE_COMPLETE_VPATH_TX': drivers/net/ethernet/neterion/vxge/vxge-main.c:103:19: error: conflicting types for 'completed' 103 | struct sk_buff **completed; | ^~~~~~~~~ drivers/net/ethernet/neterion/vxge/vxge-main.c:102:18: note: previous declaration of 'completed' was here 102 | struct sk_buff *completed[NR_SKB_COMPLETED]; | ^~~~~~~~~ drivers/net/ethernet/neterion/vxge/vxge-main.c:126:2: error: implicit declaration of function 'free' [-Werror=implicit-function-declaration] 126 | free(completed); | ^~~~ >> drivers/net/ethernet/neterion/vxge/vxge-main.c:126:2: warning: incompatible implicit declaration of built-in function 'free' drivers/net/ethernet/neterion/vxge/vxge-main.c:60:1: note: include '<stdlib.h>' or provide a declaration of 'free' 59 | #include "vxge-main.h" +++ |+#include <stdlib.h> 60 | #include "vxge-reg.h" cc1: some warnings being treated as errors vim +/free +126 drivers/net/ethernet/neterion/vxge/vxge-main.c 96 97 static inline void VXGE_COMPLETE_VPATH_TX(struct vxge_fifo *fifo) 98 { 99 struct sk_buff **skb_ptr = NULL; 100 struct sk_buff **temp; 101 #define NR_SKB_COMPLETED 128 102 struct sk_buff *completed[NR_SKB_COMPLETED]; 103 struct sk_buff **completed; 104 int more; 105 106 completed = kcalloc(NR_SKB_COMPLETED, sizeof(*completed), 107 GFP_KERNEL); 108 if (!completed) 109 return; 110 111 do { 112 more = 0; 113 skb_ptr = completed; 114 115 if (__netif_tx_trylock(fifo->txq)) { 116 vxge_hw_vpath_poll_tx(fifo->handle, &skb_ptr, 117 NR_SKB_COMPLETED, &more); 118 __netif_tx_unlock(fifo->txq); 119 } 120 121 /* free SKBs */ 122 for (temp = completed; temp != skb_ptr; temp++) 123 dev_consume_skb_irq(*temp); 124 } while (more); 125 > 126 free(completed); 127 } 128 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Bixuan, Thank you for the patch! Yet something to improve: [auto build test ERROR on sparc-next/master] [also build test ERROR on net-next/master net/master linus/master v5.8-rc5 next-20200716] [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/Bixuan-Cui/net-neterion-vxge-reduce-stack-usage-in-VXGE_COMPLETE_VPATH_TX/20200716-173219 base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc-next.git master config: arc-allyesconfig (attached as .config) compiler: arc-elf-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 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arc If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): drivers/net/ethernet/neterion/vxge/vxge-main.c: In function 'VXGE_COMPLETE_VPATH_TX': >> drivers/net/ethernet/neterion/vxge/vxge-main.c:103:19: error: conflicting types for 'completed' 103 | struct sk_buff **completed; | ^~~~~~~~~ drivers/net/ethernet/neterion/vxge/vxge-main.c:102:18: note: previous declaration of 'completed' was here 102 | struct sk_buff *completed[NR_SKB_COMPLETED]; | ^~~~~~~~~ >> drivers/net/ethernet/neterion/vxge/vxge-main.c:126:2: error: implicit declaration of function 'free' [-Werror=implicit-function-declaration] 126 | free(completed); | ^~~~ drivers/net/ethernet/neterion/vxge/vxge-main.c:60:1: note: 'free' is defined in header '<stdlib.h>'; did you forget to '#include <stdlib.h>'? 59 | #include "vxge-main.h" +++ |+#include <stdlib.h> 60 | #include "vxge-reg.h" cc1: some warnings being treated as errors vim +/completed +103 drivers/net/ethernet/neterion/vxge/vxge-main.c 96 97 static inline void VXGE_COMPLETE_VPATH_TX(struct vxge_fifo *fifo) 98 { 99 struct sk_buff **skb_ptr = NULL; 100 struct sk_buff **temp; 101 #define NR_SKB_COMPLETED 128 102 struct sk_buff *completed[NR_SKB_COMPLETED]; > 103 struct sk_buff **completed; 104 int more; 105 106 completed = kcalloc(NR_SKB_COMPLETED, sizeof(*completed), 107 GFP_KERNEL); 108 if (!completed) 109 return; 110 111 do { 112 more = 0; 113 skb_ptr = completed; 114 115 if (__netif_tx_trylock(fifo->txq)) { 116 vxge_hw_vpath_poll_tx(fifo->handle, &skb_ptr, 117 NR_SKB_COMPLETED, &more); 118 __netif_tx_unlock(fifo->txq); 119 } 120 121 /* free SKBs */ 122 for (temp = completed; temp != skb_ptr; temp++) 123 dev_consume_skb_irq(*temp); 124 } while (more); 125 > 126 free(completed); 127 } 128 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Thu, 16 Jul 2020 17:32:47 +0000 Bixuan Cui <cuibixuan@huawei.com> wrote: > Fix the warning: [-Werror=-Wframe-larger-than=] > > drivers/net/ethernet/neterion/vxge/vxge-main.c: > In function'VXGE_COMPLETE_VPATH_TX.isra.37': > drivers/net/ethernet/neterion/vxge/vxge-main.c:119:1: > warning: the frame size of 1056 bytes is larger than 1024 bytes > > Signed-off-by: Bixuan Cui <cuibixuan@huawei.com> Dropping the NR_SKB_COMPLETED to 16 won't have much impact on performance, and shrink the size. Doing 16 skb's at a time instead of 128 probably costs less than one allocation. Especially since it is unlikely that the device completed that many transmits at once.
On 2020/7/20 1:05, Stephen Hemminger wrote: > On Thu, 16 Jul 2020 17:32:47 +0000 > Bixuan Cui <cuibixuan@huawei.com> wrote: > >> Fix the warning: [-Werror=-Wframe-larger-than=] >> >> drivers/net/ethernet/neterion/vxge/vxge-main.c: >> In function'VXGE_COMPLETE_VPATH_TX.isra.37': >> drivers/net/ethernet/neterion/vxge/vxge-main.c:119:1: >> warning: the frame size of 1056 bytes is larger than 1024 bytes >> >> Signed-off-by: Bixuan Cui <cuibixuan@huawei.com> > Dropping the NR_SKB_COMPLETED to 16 won't have much impact > on performance, and shrink the size. > > Doing 16 skb's at a time instead of 128 probably costs > less than one allocation. Especially since it is unlikely > that the device completed that many transmits at once. > > I will send the v2 patch based on your suggestions. thanks
diff --git a/drivers/net/ethernet/neterion/vxge/vxge-main.c b/drivers/net/ethernet/neterion/vxge/vxge-main.c index b0faa737b817..97ddfc9debd4 100644 --- a/drivers/net/ethernet/neterion/vxge/vxge-main.c +++ b/drivers/net/ethernet/neterion/vxge/vxge-main.c @@ -100,8 +100,14 @@ static inline void VXGE_COMPLETE_VPATH_TX(struct vxge_fifo *fifo) struct sk_buff **temp; #define NR_SKB_COMPLETED 128 struct sk_buff *completed[NR_SKB_COMPLETED]; + struct sk_buff **completed; int more; + completed = kcalloc(NR_SKB_COMPLETED, sizeof(*completed), + GFP_KERNEL); + if (!completed) + return; + do { more = 0; skb_ptr = completed; @@ -116,6 +122,8 @@ static inline void VXGE_COMPLETE_VPATH_TX(struct vxge_fifo *fifo) for (temp = completed; temp != skb_ptr; temp++) dev_consume_skb_irq(*temp); } while (more); + + free(completed); } static inline void VXGE_COMPLETE_ALL_TX(struct vxgedev *vdev)
Fix the warning: [-Werror=-Wframe-larger-than=] drivers/net/ethernet/neterion/vxge/vxge-main.c: In function'VXGE_COMPLETE_VPATH_TX.isra.37': drivers/net/ethernet/neterion/vxge/vxge-main.c:119:1: warning: the frame size of 1056 bytes is larger than 1024 bytes Signed-off-by: Bixuan Cui <cuibixuan@huawei.com> --- drivers/net/ethernet/neterion/vxge/vxge-main.c | 8 ++++++++ 1 file changed, 8 insertions(+) -- 2.17.1