RE: [PATCH] net/ipv4: fix type mismatch in inet_ehash_locks_alloc() causing build failure

["Farber, Eliav" <farbere@amazon.com>]

> From: Kuniyuki Iwashima <kuni1840@gmail.com>
> Date: Sun,  8 Jun 2025 13:11:51 -0700
> > From: Eliav Farber <farbere@amazon.com>
> > Date: Sun, 8 Jun 2025 06:07:26 +0000
> > > Fix compilation warning:
> > >
> > > In file included from ./include/linux/kernel.h:15,
> > >                  from ./include/linux/list.h:9,
> > >                  from ./include/linux/module.h:12,
> > >                  from net/ipv4/inet_hashtables.c:12:
> > > net/ipv4/inet_hashtables.c: In function ‘inet_ehash_locks_alloc’:
> > > ./include/linux/minmax.h:20:35: warning: comparison of distinct pointer types lacks a cast
> > >    20 |         (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
> > >       |                                   ^~
> > > ./include/linux/minmax.h:26:18: note: in expansion of macro ‘__typecheck’
> > >    26 |                 (__typecheck(x, y) && __no_side_effects(x, y))
> > >       |                  ^~~~~~~~~~~
> > > ./include/linux/minmax.h:36:31: note: in expansion of macro ‘__safe_cmp’
> > >    36 |         __builtin_choose_expr(__safe_cmp(x, y), \
> > >       |                               ^~~~~~~~~~
> > > ./include/linux/minmax.h:52:25: note: in expansion of macro ‘__careful_cmp’
> > >    52 | #define max(x, y)       __careful_cmp(x, y, >)
> > >       |                         ^~~~~~~~~~~~~
> > > net/ipv4/inet_hashtables.c:946:19: note: in expansion of macro ‘max’
> > >   946 |         nblocks = max(nblocks, num_online_nodes() * PAGE_SIZE / locksz);
> > >       |                   ^~~
> > >   CC      block/badblocks.o
> > >
> > > When warnings are treated as errors, this causes the build to fail.
> > >
> > > The issue is a type mismatch between the operands passed to the max()
> > > macro. Here, nblocks is an unsigned int, while the expression
> > > num_online_nodes() * PAGE_SIZE / locksz is promoted to unsigned long.
> > >
> > > This happens because:
> > >  - num_online_nodes() returns int
> > >  - PAGE_SIZE is typically defined as an unsigned long (depending on the
> > >    architecture)
> > >  - locksz is unsigned int
> > >
> > > The resulting arithmetic expression is promoted to unsigned long.
> > >
> > > Thus, the max() macro compares values of different types: unsigned int
> > > vs unsigned long.
> > >
> > > This issue was introduced in commit b53d6e9525af ("tcp: bring back NUMA
> > > dispersion in inet_ehash_locks_alloc()") during the update from kernel
> > > v5.10.237 to v5.10.238.
> >
> > Please use the upstream SHA1, f8ece40786c9.
Fixed in V2

> > > It does not exist in newer kernel branches (e.g., v5.15.185 and all 6.x
> > > branches), because they include commit d53b5d862acd ("minmax: allow
> >
> > Same here, d03eba99f5bf.
Fixed in V2

> > But why not backport it to stable instead ?
>
> I just checked the 5.10.238 thread.
> https://lore.kernel.org/stable/2025060412-cursor-navigate-126d@gregkh/
>
> ---8<---
> > > For both of these, I'll just let them be as they are ok, it's just the
> > > mess of our min/max macro unwinding causes these issues.
> > >
> > > Unless they really bother someone, and in that case, a patch to add the
> > > correct type to the backport to make the noise go away would be greatly
> > > appreciated.
> >
> > Yeah that's a reasonable resolution, I will try to track down the missing
> > patches for minmax.h so we are warning free for the stable kernels.
>
> I tried in the past, it's non-trivial.  What would be easier is to just
> properly cast the variables in the places where this warning is showing
> up to get rid of that warning.  We've done that in some backports in the
> past as well.
> ---8<---
>
> So this should be fixed up in the backport, and I guess this patch
> targeted the stable trees ?
This patch is for v5.10.238. Newer kernels don't need it.

> If so, please clarify that by specifying the stable version in the
> subject and CCing the stable mainling list:
>
>   Subject: [PATCH 5.10.y] tcp: ...
>   Cc: stable@vger.kernel.org, ...
Done in v2