Re: [PATCH] pktgen: use min() to make code cleaner

[kernel test robot <lkp@intel.com>]

Hi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]
[also build test WARNING on net/master horms-ipvs/master linus/master v5.16-rc5 next-20211213]
[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/cgel-zte-gmail-com/pktgen-use-min-to-make-code-cleaner/20211214-194646
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git a3c62a042237d1adeb0290dcb768e17edd6dcd25
config: x86_64-randconfig-r016-20211214 (https://download.01.org/0day-ci/archive/20211215/202112150130.jdr5Ps5z-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project b6a2ddb6c8ac29412b1361810972e15221fa021c)
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/7cc9aa88278e63d75ce0af120e4b6992bff00e7e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review cgel-zte-gmail-com/pktgen-use-min-to-make-code-cleaner/20211214-194646
        git checkout 7cc9aa88278e63d75ce0af120e4b6992bff00e7e
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/power/supply/ net/core/

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 >>):

>> net/core/pktgen.c:2781:14: warning: comparison of distinct pointer types ('typeof (datalen / frags) *' (aka 'int *') and 'typeof (((1UL) << 12)) *' (aka 'unsigned long *')) [-Wcompare-distinct-pointer-types]
                   frag_len = min(datalen / frags, PAGE_SIZE);
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:45:19: note: expanded from macro 'min'
   #define min(x, y)       __careful_cmp(x, y, <)
                           ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:36:24: note: expanded from macro '__careful_cmp'
           __builtin_choose_expr(__safe_cmp(x, y), \
                                 ^~~~~~~~~~~~~~~~
   include/linux/minmax.h:26:4: note: expanded from macro '__safe_cmp'
                   (__typecheck(x, y) && __no_side_effects(x, y))
                    ^~~~~~~~~~~~~~~~~
   include/linux/minmax.h:20:28: note: expanded from macro '__typecheck'
           (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
                      ~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~
>> net/core/pktgen.c:2798:9: warning: comparison of distinct pointer types ('typeof (datalen) *' (aka 'int *') and 'typeof (((1UL) << 12)) *' (aka 'unsigned long *')) [-Wcompare-distinct-pointer-types]
                                       min(datalen, PAGE_SIZE));
                                       ^~~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:45:19: note: expanded from macro 'min'
   #define min(x, y)       __careful_cmp(x, y, <)
                           ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:36:24: note: expanded from macro '__careful_cmp'
           __builtin_choose_expr(__safe_cmp(x, y), \
                                 ^~~~~~~~~~~~~~~~
   include/linux/minmax.h:26:4: note: expanded from macro '__safe_cmp'
                   (__typecheck(x, y) && __no_side_effects(x, y))
                    ^~~~~~~~~~~~~~~~~
   include/linux/minmax.h:20:28: note: expanded from macro '__typecheck'
           (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
                      ~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~
   2 warnings generated.


vim +2781 net/core/pktgen.c

  2754	
  2755	static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb,
  2756					int datalen)
  2757	{
  2758		struct timespec64 timestamp;
  2759		struct pktgen_hdr *pgh;
  2760	
  2761		pgh = skb_put(skb, sizeof(*pgh));
  2762		datalen -= sizeof(*pgh);
  2763	
  2764		if (pkt_dev->nfrags <= 0) {
  2765			skb_put_zero(skb, datalen);
  2766		} else {
  2767			int frags = pkt_dev->nfrags;
  2768			int i, len;
  2769			int frag_len;
  2770	
  2771	
  2772			if (frags > MAX_SKB_FRAGS)
  2773				frags = MAX_SKB_FRAGS;
  2774			len = datalen - frags * PAGE_SIZE;
  2775			if (len > 0) {
  2776				skb_put_zero(skb, len);
  2777				datalen = frags * PAGE_SIZE;
  2778			}
  2779	
  2780			i = 0;
> 2781			frag_len = min(datalen / frags, PAGE_SIZE);
  2782			while (datalen > 0) {
  2783				if (unlikely(!pkt_dev->page)) {
  2784					int node = numa_node_id();
  2785	
  2786					if (pkt_dev->node >= 0 && (pkt_dev->flags & F_NODE))
  2787						node = pkt_dev->node;
  2788					pkt_dev->page = alloc_pages_node(node, GFP_KERNEL | __GFP_ZERO, 0);
  2789					if (!pkt_dev->page)
  2790						break;
  2791				}
  2792				get_page(pkt_dev->page);
  2793				skb_frag_set_page(skb, i, pkt_dev->page);
  2794				skb_frag_off_set(&skb_shinfo(skb)->frags[i], 0);
  2795				/*last fragment, fill rest of data*/
  2796				if (i == (frags - 1))
  2797					skb_frag_size_set(&skb_shinfo(skb)->frags[i],
> 2798					    min(datalen, PAGE_SIZE));
  2799				else
  2800					skb_frag_size_set(&skb_shinfo(skb)->frags[i], frag_len);
  2801				datalen -= skb_frag_size(&skb_shinfo(skb)->frags[i]);
  2802				skb->len += skb_frag_size(&skb_shinfo(skb)->frags[i]);
  2803				skb->data_len += skb_frag_size(&skb_shinfo(skb)->frags[i]);
  2804				i++;
  2805				skb_shinfo(skb)->nr_frags = i;
  2806			}
  2807		}
  2808	
  2809		/* Stamp the time, and sequence number,
  2810		 * convert them to network byte order
  2811		 */
  2812		pgh->pgh_magic = htonl(PKTGEN_MAGIC);
  2813		pgh->seq_num = htonl(pkt_dev->seq_num);
  2814	
  2815		if (pkt_dev->flags & F_NO_TIMESTAMP) {
  2816			pgh->tv_sec = 0;
  2817			pgh->tv_usec = 0;
  2818		} else {
  2819			/*
  2820			 * pgh->tv_sec wraps in y2106 when interpreted as unsigned
  2821			 * as done by wireshark, or y2038 when interpreted as signed.
  2822			 * This is probably harmless, but if anyone wants to improve
  2823			 * it, we could introduce a variant that puts 64-bit nanoseconds
  2824			 * into the respective header bytes.
  2825			 * This would also be slightly faster to read.
  2826			 */
  2827			ktime_get_real_ts64(&timestamp);
  2828			pgh->tv_sec = htonl(timestamp.tv_sec);
  2829			pgh->tv_usec = htonl(timestamp.tv_nsec / NSEC_PER_USEC);
  2830		}
  2831	}
  2832	

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