[PATCH v2] Fix the data type inconsistency issue of min (tier, MAX_CR_TIERS-1) in read_ctrl_pos

[PATCH v2] Fix the data type inconsistency issue of min (tier, MAX_CR_TIERS-1) in read_ctrl_pos

[Qingshuang Fu]

From: Qingshuang Fu <fuqingshuang@kylinos.cn>

Due to the fact that the tier data type in min (tier, MAX_CR_TIERS -1)
is int,but MAX_CR_TIERS is an unsigned type, directly using
the min function for comparison will result in an error:

from mm/vmscan.c:15:
mm/vmscan.c: In function ‘read_ctrl_pos’:
./include/linux/build_bug.h:78:41: error: static assertion failed:
"min(tier, 4U - 1) signedness error, fix types or
consider umin() before min_t()"

Fixes: 37a260870f2c ("mm/mglru: rework type selection")
Suggested-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Qingshuang Fu <fuqingshuang@kylinos.cn>
---
 mm/vmscan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 7de11524a936..f991196fd8e5 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -3194,7 +3194,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.25.1

Re: [PATCH v2] Fix the data type inconsistency issue of min (tier, MAX_CR_TIERS-1) in read_ctrl_pos

[David Hildenbrand]

On 08.08.25 11:54, Qingshuang Fu wrote:
> From: Qingshuang Fu <fuqingshuang@kylinos.cn>
> 
> Due to the fact that the tier data type in min (tier, MAX_CR_TIERS -1)
> is int,but MAX_CR_TIERS is an unsigned type, directly using
> the min function for comparison will result in an error:
> 
> from mm/vmscan.c:15:
> mm/vmscan.c: In function ‘read_ctrl_pos’:
> ./include/linux/build_bug.h:78:41: error: static assertion failed:
> "min(tier, 4U - 1) signedness error, fix types or
> consider umin() before min_t()"
> 
> Fixes: 37a260870f2c ("mm/mglru: rework type selection")
> Suggested-by: David Hildenbrand <david@redhat.com>

You keep ignoring my feedback and I can only reproduce this with W=2 
where there are like a bunch of other issues, this here doesn't move the 
needle.

So I'm afraid I cannot spend any more time on reviewing this patch.

NAK

-- 
Cheers,

David / dhildenb

Re:Re: [PATCH v2] Fix the data type inconsistency issue of min (tier, MAX_CR_TIERS-1) in read_ctrl_pos

[fffsqian]

[-- Attachment #1: Type: text/plain, Size: 5503 bytes --]

Hi David,

I'm really sorry, I just submitted the patch for version V2 and didn't notice your reply email. Here is an explanation about the environment used for compilation:

Firstly, I opened the configuration startup option CONFIG_WERROR=y

Secondly, when I tested with versions 9.4.0 and 12.3.1 of gcc, I encountered errors:




$ gcc --version
gcc (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0
$ make mm/vmscan.o
  CALL    scripts/checksyscalls.sh
  DESCEND bpf/resolve_btfids
  INSTALL libsubcmd_headers
  CC      mm/vmscan.o
In file included from ./include/linux/init.h:5,
                 from ./include/linux/printk.h:6,
                 from ./include/asm-generic/bug.h:22,
                 from ./arch/arm64/include/asm/bug.h:26,
                 from ./include/linux/bug.h:5,
                 from ./include/linux/mmdebug.h:5,
                 from ./include/linux/mm.h:6,
                 from mm/vmscan.c:15:
mm/vmscan.c: In function ‘read_ctrl_pos’:
./include/linux/build_bug.h:78:41: error: static assertion failed: "min(tier, 4U - 1) signedness error, fix types or consider umin() before min_t()"
   78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
      |                                         ^~~~~~~~~~~~~~
./include/linux/build_bug.h:77:34: note: in expansion of macro ‘__static_assert’
   77 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
      |                                  ^~~~~~~~~~~~~~~
./include/linux/minmax.h:51:2: note: in expansion of macro ‘static_assert’
   51 |  static_assert(__types_ok(x, y),   \
      |  ^~~~~~~~~~~~~
./include/linux/minmax.h:58:3: note: in expansion of macro ‘__cmp_once’
   58 |   __cmp_once(op, x, y, __UNIQUE_ID(__x), __UNIQUE_ID(__y)))
      |   ^~~~~~~~~~
./include/linux/minmax.h:85:19: note: in expansion of macro ‘__careful_cmp’
   85 | #define min(x, y) __careful_cmp(min, x, y)
      |                   ^~~~~~~~~~~~~
mm/vmscan.c:3098:37: note: in expansion of macro ‘min’
 3098 |  for (i = tier % MAX_NR_TIERS; i <= min(tier, MAX_NR_TIERS - 1); i++) {
      |                                     ^~~
make[3]: *** [scripts/Makefile.build:243: mm/vmscan.o] Error 1
make[2]: *** [scripts/Makefile.build:480: mm] Error 2
make[1]: *** [/home/user/fffs/klinux/Makefile:1944: .] Error 2
make: *** [Makefile:236: __sub-make] Error 2





$ gcc --version
gcc (GCC) 12.3.1 (kylin 12.3.1-31.p02.ky11)
$ make mm/vmscan.o
  CALL    scripts/checksyscalls.sh
  DESCEND bpf/resolve_btfids
make[4]: “install_headers”is already the latest.
  CC      mm/vmscan.o
In file included from ./include/linux/init.h:5,
                 from ./include/linux/printk.h:6,
                 from ./include/asm-generic/bug.h:22,
                 from ./arch/arm64/include/asm/bug.h:26,
                 from ./include/linux/bug.h:5,
                 from ./include/linux/mmdebug.h:5,
                 from ./include/linux/mm.h:6,
                 from mm/vmscan.c:15:
mm/vmscan.c: In function ‘read_ctrl_pos’:
./include/linux/build_bug.h:78:41: error：static assertion failed："min(tier, 4U - 1) signedness error, fix types or consider umin() before min_t()"
   78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
      |                                         ^~~~~~~~~~~~~~
./include/linux/build_bug.h:77:34: note：in expansion of macro ‘__static_assert’
   77 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
      |                                  ^~~~~~~~~~~~~~~
./include/linux/minmax.h:51:9: note：in expansion of macro ‘static_assert’
   51 |         static_assert(__types_ok(x, y),                 \
      |         ^~~~~~~~~~~~~
./include/linux/minmax.h:58:17: note：in expansion of macro ‘__cmp_once’
   58 |                 __cmp_once(op, x, y, __UNIQUE_ID(__x), __UNIQUE_ID(__y)))
      |                 ^~~~~~~~~~
./include/linux/minmax.h:85:25: note：in expansion of macro ‘__careful_cmp’
   85 | #define min(x, y)       __careful_cmp(min, x, y)
      |                         ^~~~~~~~~~~~~
mm/vmscan.c:3098:44: note：in expansion of macro ‘min’
 3098 |         for (i = tier % MAX_NR_TIERS; i <= min(tier, MAX_NR_TIERS - 1); i++) {
      |                                            ^~~
make[3]: *** [scripts/Makefile.build:243：mm/vmscan.o] Error 1
make[2]: *** [scripts/Makefile.build:480：mm] Error 2
make[1]: *** [/root/fffs/klinux/Makefile:1944：.] Error2
make: *** [Makefile:236：__sub-make] Error 2




Sorry again.

















At 2025-08-08 17:58:31, "David Hildenbrand" <david@redhat.com> wrote:
>On 08.08.25 11:54, Qingshuang Fu wrote:
>> From: Qingshuang Fu <fuqingshuang@kylinos.cn>
>> 
>> Due to the fact that the tier data type in min (tier, MAX_CR_TIERS -1)
>> is int,but MAX_CR_TIERS is an unsigned type, directly using
>> the min function for comparison will result in an error:
>> 
>> from mm/vmscan.c:15:
>> mm/vmscan.c: In function ‘read_ctrl_pos’:
>> ./include/linux/build_bug.h:78:41: error: static assertion failed:
>> "min(tier, 4U - 1) signedness error, fix types or
>> consider umin() before min_t()"
>> 
>> Fixes: 37a260870f2c ("mm/mglru: rework type selection")
>> Suggested-by: David Hildenbrand <david@redhat.com>
>
>You keep ignoring my feedback and I can only reproduce this with W=2 
>where there are like a bunch of other issues, this here doesn't move the 
>needle.
>
>So I'm afraid I cannot spend any more time on reviewing this patch.
>
>NAK
>
>-- 
>Cheers,
>
>David / dhildenb

[-- Attachment #2: Type: text/html, Size: 72815 bytes --]