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

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

[Eliav Farber]

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.

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
min()/max()/clamp() if the arguments have the same signedness.")

Fix the issue by using max_t(unsigned int, ...) to explicitly cast both
operands to the same type, avoiding the type mismatch and ensuring
correctness.

Fixes: b53d6e9525af ("tcp: bring back NUMA dispersion in inet_ehash_locks_alloc()")
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
 net/ipv4/inet_hashtables.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index fea74ab2a4be..ac2d185c04ef 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -943,7 +943,7 @@ int inet_ehash_locks_alloc(struct inet_hashinfo *hashinfo)
 	nblocks = max(2U * L1_CACHE_BYTES / locksz, 1U) * num_possible_cpus();
 
 	/* At least one page per NUMA node. */
-	nblocks = max(nblocks, num_online_nodes() * PAGE_SIZE / locksz);
+	nblocks = max_t(unsigned int, nblocks, num_online_nodes() * PAGE_SIZE / locksz);
 
 	nblocks = roundup_pow_of_two(nblocks);
 
-- 
2.47.1

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

[Kuniyuki Iwashima]

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.

> 
> 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.

But why not backport it to stable instead ?

In the first place, f8ece40786c9 does not have Fixes: and seems
to be cherry-picked accidentally by AUTOSEL.


> min()/max()/clamp() if the arguments have the same signedness.")
> 
> Fix the issue by using max_t(unsigned int, ...) to explicitly cast both
> operands to the same type, avoiding the type mismatch and ensuring
> correctness.

The cover letter of d03eba99f5bf says:
https://lore.kernel.org/all/b97faef60ad24922b530241c5d7c933c@AcuMS.aculab.com/

---8<---
The min() (etc) functions in minmax.h require that the arguments have
exactly the same types.

However when the type check fails, rather than look at the types and
fix the type of a variable/constant, everyone seems to jump on min_t().
In reality min_t() ought to be rare - when something unusual is being
done, not normality.
---8<---

So, the typecheck variant should be rare, and it was merged 2 years ago,
so there should be more places depending on the commit.

Once someone backported such a change to stable trees again and required
this type of "fix", it would revert the less-typecheck effor gradually,
which should be avoided.


> 
> Fixes: b53d6e9525af ("tcp: bring back NUMA dispersion in inet_ehash_locks_alloc()")
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> ---
>  net/ipv4/inet_hashtables.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
> index fea74ab2a4be..ac2d185c04ef 100644
> --- a/net/ipv4/inet_hashtables.c
> +++ b/net/ipv4/inet_hashtables.c
> @@ -943,7 +943,7 @@ int inet_ehash_locks_alloc(struct inet_hashinfo *hashinfo)
>  	nblocks = max(2U * L1_CACHE_BYTES / locksz, 1U) * num_possible_cpus();
>  
>  	/* At least one page per NUMA node. */
> -	nblocks = max(nblocks, num_online_nodes() * PAGE_SIZE / locksz);
> +	nblocks = max_t(unsigned int, nblocks, num_online_nodes() * PAGE_SIZE / locksz);
>  
>  	nblocks = roundup_pow_of_two(nblocks);
>  
> -- 
> 2.47.1

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

[Kuniyuki Iwashima]

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.
> 
> > 
> > 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.
> 
> 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 ?

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, ...

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

[Farber, Eliav]

> 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