| Message ID | 20240824074033.2134514-5-lihongbo22@huawei.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Use max/min to simplify the code | expand |
Hi Hongbo,
kernel test robot noticed the following build errors:
[auto build test ERROR on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Hongbo-Li/net-mac80211-use-max-to-simplify-the-code/20240826-154029
base: net-next/main
patch link: https://lore.kernel.org/r/20240824074033.2134514-5-lihongbo22%40huawei.com
patch subject: [PATCH net-next 4/8] net/core: Use min()/max() to simplify the code
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20240828/202408281516.thVNxpEo-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 08e5a1de8227512d4774a534b91cb2353cef6284)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240828/202408281516.thVNxpEo-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408281516.thVNxpEo-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from net/core/pktgen.c:126:
In file included from include/linux/ptrace.h:10:
In file included from include/linux/pid_namespace.h:7:
In file included from include/linux/mm.h:2228:
include/linux/vmstat.h:517:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
517 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
In file included from net/core/pktgen.c:129:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
548 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
561 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from net/core/pktgen.c:129:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
574 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from net/core/pktgen.c:129:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
585 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
595 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
605 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
>> net/core/pktgen.c:2796:14: error: call to '__compiletime_assert_743' declared with 'error' attribute: min(datalen/frags, (1UL << 12)) signedness error
2796 | frag_len = min(datalen/frags, PAGE_SIZE);
| ^
include/linux/minmax.h:129:19: note: expanded from macro 'min'
129 | #define min(x, y) __careful_cmp(min, x, y)
| ^
include/linux/minmax.h:105:2: note: expanded from macro '__careful_cmp'
105 | __careful_cmp_once(op, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
| ^
include/linux/minmax.h:100:2: note: expanded from macro '__careful_cmp_once'
100 | BUILD_BUG_ON_MSG(!__types_ok(x,y,ux,uy), \
| ^
note: (skipping 2 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
include/linux/compiler_types.h:498:2: note: expanded from macro '_compiletime_assert'
498 | __compiletime_assert(condition, msg, prefix, suffix)
| ^
include/linux/compiler_types.h:491:4: note: expanded from macro '__compiletime_assert'
491 | prefix ## suffix(); \
| ^
<scratch space>:6:1: note: expanded from here
6 | __compiletime_assert_743
| ^
7 warnings and 1 error generated.
vim +2796 net/core/pktgen.c
2769
2770 static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb,
2771 int datalen)
2772 {
2773 struct timespec64 timestamp;
2774 struct pktgen_hdr *pgh;
2775
2776 pgh = skb_put(skb, sizeof(*pgh));
2777 datalen -= sizeof(*pgh);
2778
2779 if (pkt_dev->nfrags <= 0) {
2780 skb_put_zero(skb, datalen);
2781 } else {
2782 int frags = pkt_dev->nfrags;
2783 int i, len;
2784 int frag_len;
2785
2786
2787 if (frags > MAX_SKB_FRAGS)
2788 frags = MAX_SKB_FRAGS;
2789 len = datalen - frags * PAGE_SIZE;
2790 if (len > 0) {
2791 skb_put_zero(skb, len);
2792 datalen = frags * PAGE_SIZE;
2793 }
2794
2795 i = 0;
> 2796 frag_len = min(datalen/frags, PAGE_SIZE);
2797 while (datalen > 0) {
2798 if (unlikely(!pkt_dev->page)) {
2799 int node = numa_node_id();
2800
2801 if (pkt_dev->node >= 0 && (pkt_dev->flags & F_NODE))
2802 node = pkt_dev->node;
2803 pkt_dev->page = alloc_pages_node(node, GFP_KERNEL | __GFP_ZERO, 0);
2804 if (!pkt_dev->page)
2805 break;
2806 }
2807 get_page(pkt_dev->page);
2808
2809 /*last fragment, fill rest of data*/
2810 if (i == (frags - 1))
2811 skb_frag_fill_page_desc(&skb_shinfo(skb)->frags[i],
2812 pkt_dev->page, 0,
2813 min(datalen, PAGE_SIZE));
2814 else
2815 skb_frag_fill_page_desc(&skb_shinfo(skb)->frags[i],
2816 pkt_dev->page, 0, frag_len);
2817
2818 datalen -= skb_frag_size(&skb_shinfo(skb)->frags[i]);
2819 skb->len += skb_frag_size(&skb_shinfo(skb)->frags[i]);
2820 skb->data_len += skb_frag_size(&skb_shinfo(skb)->frags[i]);
2821 i++;
2822 skb_shinfo(skb)->nr_frags = i;
2823 }
2824 }
2825
2826 /* Stamp the time, and sequence number,
2827 * convert them to network byte order
2828 */
2829 pgh->pgh_magic = htonl(PKTGEN_MAGIC);
2830 pgh->seq_num = htonl(pkt_dev->seq_num);
2831
2832 if (pkt_dev->flags & F_NO_TIMESTAMP) {
2833 pgh->tv_sec = 0;
2834 pgh->tv_usec = 0;
2835 } else {
2836 /*
2837 * pgh->tv_sec wraps in y2106 when interpreted as unsigned
2838 * as done by wireshark, or y2038 when interpreted as signed.
2839 * This is probably harmless, but if anyone wants to improve
2840 * it, we could introduce a variant that puts 64-bit nanoseconds
2841 * into the respective header bytes.
2842 * This would also be slightly faster to read.
2843 */
2844 ktime_get_real_ts64(×tamp);
2845 pgh->tv_sec = htonl(timestamp.tv_sec);
2846 pgh->tv_usec = htonl(timestamp.tv_nsec / NSEC_PER_USEC);
2847 }
2848 }
2849
Hi Hongbo,
kernel test robot noticed the following build errors:
[auto build test ERROR on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Hongbo-Li/net-mac80211-use-max-to-simplify-the-code/20240826-154029
base: net-next/main
patch link: https://lore.kernel.org/r/20240824074033.2134514-5-lihongbo22%40huawei.com
patch subject: [PATCH net-next 4/8] net/core: Use min()/max() to simplify the code
config: powerpc-allmodconfig (https://download.01.org/0day-ci/archive/20240828/202408281628.FtNGguag-lkp@intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240828/202408281628.FtNGguag-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408281628.FtNGguag-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from <command-line>:
net/core/pktgen.c: In function 'pktgen_finalize_skb':
>> include/linux/compiler_types.h:510:45: error: call to '__compiletime_assert_991' declared with attribute error: min(datalen/frags, (1UL << 16)) signedness error
510 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^
include/linux/compiler_types.h:491:25: note: in definition of macro '__compiletime_assert'
491 | prefix ## suffix(); \
| ^~~~~~
include/linux/compiler_types.h:510:9: note: in expansion of macro '_compiletime_assert'
510 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
| ^~~~~~~~~~~~~~~~~~
include/linux/minmax.h:100:9: note: in expansion of macro 'BUILD_BUG_ON_MSG'
100 | BUILD_BUG_ON_MSG(!__types_ok(x,y,ux,uy), \
| ^~~~~~~~~~~~~~~~
include/linux/minmax.h:105:9: note: in expansion of macro '__careful_cmp_once'
105 | __careful_cmp_once(op, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
| ^~~~~~~~~~~~~~~~~~
include/linux/minmax.h:129:25: note: in expansion of macro '__careful_cmp'
129 | #define min(x, y) __careful_cmp(min, x, y)
| ^~~~~~~~~~~~~
net/core/pktgen.c:2796:28: note: in expansion of macro 'min'
2796 | frag_len = min(datalen/frags, PAGE_SIZE);
| ^~~
vim +/__compiletime_assert_991 +510 include/linux/compiler_types.h
eb5c2d4b45e3d2 Will Deacon 2020-07-21 496
eb5c2d4b45e3d2 Will Deacon 2020-07-21 497 #define _compiletime_assert(condition, msg, prefix, suffix) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21 498 __compiletime_assert(condition, msg, prefix, suffix)
eb5c2d4b45e3d2 Will Deacon 2020-07-21 499
eb5c2d4b45e3d2 Will Deacon 2020-07-21 500 /**
eb5c2d4b45e3d2 Will Deacon 2020-07-21 501 * compiletime_assert - break build and emit msg if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21 502 * @condition: a compile-time constant condition to check
eb5c2d4b45e3d2 Will Deacon 2020-07-21 503 * @msg: a message to emit if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21 504 *
eb5c2d4b45e3d2 Will Deacon 2020-07-21 505 * In tradition of POSIX assert, this macro will break the build if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21 506 * supplied condition is *false*, emitting the supplied error message if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21 507 * compiler has support to do so.
eb5c2d4b45e3d2 Will Deacon 2020-07-21 508 */
eb5c2d4b45e3d2 Will Deacon 2020-07-21 509 #define compiletime_assert(condition, msg) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21 @510 _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
eb5c2d4b45e3d2 Will Deacon 2020-07-21 511
On Sat, Aug 24, 2024 at 03:40:29PM +0800, Hongbo Li wrote: > Let's use min()/max() to simplify the code and fix the > Coccinelle/coccicheck warning. > > Signed-off-by: Hongbo Li <lihongbo22@huawei.com> ... > diff --git a/net/core/sock.c b/net/core/sock.c > index bbe4c58470c3..c9910f48903f 100644 > --- a/net/core/sock.c > +++ b/net/core/sock.c > @@ -3800,7 +3800,7 @@ int sock_prot_inuse_get(struct net *net, struct proto *prot) > for_each_possible_cpu(cpu) > res += per_cpu_ptr(net->core.prot_inuse, cpu)->val[idx]; > > - return res >= 0 ? res : 0; > + return max(res, 0); > } > EXPORT_SYMBOL_GPL(sock_prot_inuse_get); As per my comment on 2/8 [*], I think you should drop this hunk. [*] https://lore.kernel.org/all/20240828135310.GC1368797@kernel.org/
diff --git a/net/core/pktgen.c b/net/core/pktgen.c index ea55a758a475..5d4d5ec4a126 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -2793,8 +2793,7 @@ static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb, } i = 0; - frag_len = (datalen/frags) < PAGE_SIZE ? - (datalen/frags) : PAGE_SIZE; + frag_len = min(datalen/frags, PAGE_SIZE); while (datalen > 0) { if (unlikely(!pkt_dev->page)) { int node = numa_node_id(); @@ -2811,8 +2810,7 @@ static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb, if (i == (frags - 1)) skb_frag_fill_page_desc(&skb_shinfo(skb)->frags[i], pkt_dev->page, 0, - (datalen < PAGE_SIZE ? - datalen : PAGE_SIZE)); + min(datalen, PAGE_SIZE)); else skb_frag_fill_page_desc(&skb_shinfo(skb)->frags[i], pkt_dev->page, 0, frag_len); diff --git a/net/core/sock.c b/net/core/sock.c index bbe4c58470c3..c9910f48903f 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -3800,7 +3800,7 @@ int sock_prot_inuse_get(struct net *net, struct proto *prot) for_each_possible_cpu(cpu) res += per_cpu_ptr(net->core.prot_inuse, cpu)->val[idx]; - return res >= 0 ? res : 0; + return max(res, 0); } EXPORT_SYMBOL_GPL(sock_prot_inuse_get);
Let's use min()/max() to simplify the code and fix the Coccinelle/coccicheck warning. Signed-off-by: Hongbo Li <lihongbo22@huawei.com> --- net/core/pktgen.c | 6 ++---- net/core/sock.c | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-)