Re: + mm-vmscan-avoid-signedness-error-for-gcc-54.patch added to mm-hotfixes-unstable branch

[David Laight <david.laight.linux@gmail.com>]

On Wed, 07 May 2025 11:08:17 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> The patch titled
>      Subject: mm: vmscan: avoid signedness error for GCC 5.4
> has been added to the -mm mm-hotfixes-unstable branch.  Its filename is
>      mm-vmscan-avoid-signedness-error-for-gcc-54.patch
> 
> This patch will shortly appear at
>      https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/mm-vmscan-avoid-signedness-error-for-gcc-54.patch
> 
> This patch will later appear in the mm-hotfixes-unstable branch at
>     git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> 
> Before you just go and hit "reply", please:
>    a) Consider who else should be cc'ed
>    b) Prefer to cc a suitable mailing list as well
>    c) Ideally: find the original patch on the mailing list and do a
>       reply-to-all to that, adding suitable additional cc's
> 
> *** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
> 
> The -mm tree is included into linux-next via the mm-everything
> branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> and is updated there every 2-3 working days
> 
> ------------------------------------------------------
> From: WangYuli <wangyuli@uniontech.com>
> Subject: mm: vmscan: avoid signedness error for GCC 5.4
> Date: Wed, 7 May 2025 12:08:27 +0800
> 
> To the GCC 5.4 compiler, (MAX_NR_TIERS - 1) (i.e., (4U - 1)) is unsigned,
> whereas tier is a signed integer.

As I've said before this is independent of the compiler.

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

The test passes when the function is inlined because the compiler knows
that 'tier' is always positive - even though it is a signed type.
Most likely the compilation with GCC 5.4 isn't inlining the function.

I rather hope this isn't a common code path - the generated code
for the function will be pretty horrid even when inlined.

> Use min_t instead of min to circumvent this compiler error.

It is a compilation error not a compiler error.

If every complain about min() reporting a signedness error is fixed
by using min_t() we might as remove the check.
The fixes should change the types of the variables so the error isn't
reported.

In this case just changing 'tier' to unsigned int is enough.
(Although the code is still horrid.)

	David

> 
> 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
> 
> Link: https://lkml.kernel.org/r/62726950F697595A+20250507040827.1147510-1-wangyuli@uniontech.com
> Fixes: 37a260870f2c ("mm/mglru: rework type selection")
> Signed-off-by: WangYuli <wangyuli@uniontech.com>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>  mm/vmscan.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/mm/vmscan.c~mm-vmscan-avoid-signedness-error-for-gcc-54
> +++ a/mm/vmscan.c
> @@ -3163,7 +3163,7 @@ static void read_ctrl_pos(struct lruvec
>  	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] +
> _
> 
> Patches currently in -mm which might be from wangyuli@uniontech.com are
> 
> mm-vmscan-avoid-signedness-error-for-gcc-54.patch
> ocfs2-o2net_idle_timer-rename-del_timer_sync-in-comment.patch
> treewide-fix-typo-previlege.patch
> 
>