[PATCH v2] mm: vmscan: Avoid signedness error for GCC 5.4

[PATCH v2] mm: vmscan: Avoid signedness error for GCC 5.4

[WangYuli]

To the GCC 5.4 compiler, (MAX_NR_TIERS - 1) (i.e., (4U - 1)) is
unsigned, whereas tier is a signed integer.

Then, the __types_ok check within the __careful_cmp_once macro failed,
triggered BUILD_BUG_ON.

Use min_t instead of min to circumvent this compiler error.

Fix follow error with gcc 5.4:
  mm/vmscan.c: In function ‘read_ctrl_pos’:
  mm/vmscan.c:3166:728: error: call to ‘__compiletime_assert_887’ declared with attribute error: min(tier, 4U - 1) signedness error

Cc: Matthew Wilcox <willy@infradead.org>
Fixes: 37a260870f2c ("mm/mglru: rework type selection")
Signed-off-by: WangYuli <wangyuli@uniontech.com>
---
Changelog:
 *v1->v2:
    1. Fix commit msg.
    2. Use min_t instead of min.
---
 mm/vmscan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 3783e45bfc92..8d9a82621c4f 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -3163,7 +3163,7 @@ static void read_ctrl_pos(struct lruvec *lruvec, int type, int tier, int gain,
 	pos->gain = gain;
 	pos->refaulted = pos->total = 0;
 
-	for (i = tier % MAX_NR_TIERS; i <= min(tier, MAX_NR_TIERS - 1); i++) {
+	for (i = tier % MAX_NR_TIERS; i <= min_t(int, tier, MAX_NR_TIERS - 1); i++) {
 		pos->refaulted += lrugen->avg_refaulted[type][i] +
 				  atomic_long_read(&lrugen->refaulted[hist][type][i]);
 		pos->total += lrugen->avg_total[type][i] +
-- 
2.49.0

Re: [PATCH v2] mm: vmscan: Avoid signedness error for GCC 5.4

[David Laight]

On Wed,  7 May 2025 12:08:27 +0800
WangYuli <wangyuli@uniontech.com> wrote:

> To the GCC 5.4 compiler, (MAX_NR_TIERS - 1) (i.e., (4U - 1)) is
> unsigned, whereas tier is a signed integer.

That is true for all version of the compiler.
The warning depends on whether the function is inlined or not.

> 
> Then, the __types_ok check within the __careful_cmp_once macro failed,
> triggered BUILD_BUG_ON.
> 
> Use min_t instead of min to circumvent this compiler error.

It isn't a compiler error, it is a coding error.

The correct fix is to change tier to be unsigned.
(and probably 'i' at the same time.)

> 
> Fix follow error with gcc 5.4:
>   mm/vmscan.c: In function ‘read_ctrl_pos’:
>   mm/vmscan.c:3166:728: error: call to ‘__compiletime_assert_887’ declared with attribute error: min(tier, 4U - 1) signedness error
> 
> Cc: Matthew Wilcox <willy@infradead.org>
> Fixes: 37a260870f2c ("mm/mglru: rework type selection")
> Signed-off-by: WangYuli <wangyuli@uniontech.com>
> ---
> Changelog:
>  *v1->v2:
>     1. Fix commit msg.
>     2. Use min_t instead of min.
> ---
>  mm/vmscan.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 3783e45bfc92..8d9a82621c4f 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -3163,7 +3163,7 @@ static void read_ctrl_pos(struct lruvec *lruvec, int type, int tier, int gain,
>  	pos->gain = gain;
>  	pos->refaulted = pos->total = 0;
>  
> -	for (i = tier % MAX_NR_TIERS; i <= min(tier, MAX_NR_TIERS - 1); i++) {
> +	for (i = tier % MAX_NR_TIERS; i <= min_t(int, tier, MAX_NR_TIERS - 1); i++) {

That loop is atrocious.
There are two types of call to the function.
Either 'tier' is a constant 0, a variable 1..3 or a constant 4.
You really don't want it not inlined.
When inlined gcc tracks the domain of 'tier' and knows it is positive.
That stops the error from min().
If not inlined you get terrible code and the warning.

For the loop case (and that includes the 0, then 1..3 loop) you probably
want to ensure the [hist] and [type] array indexes are only done once.
I suspect gcc can't do that for you.

	David

>  		pos->refaulted += lrugen->avg_refaulted[type][i] +
>  				  atomic_long_read(&lrugen->refaulted[hist][type][i]);
>  		pos->total += lrugen->avg_total[type][i] +