[PATCH] mm: Fix clang W=1 compiler warnings

[PATCH] mm: Fix clang W=1 compiler warnings

[Bart Van Assche]

Commit 30c2de0a267c ("mm/vmstat: fix a W=1 clang compiler warning")
suppresses some but not all compiler warnings that are reported by clang
when building mm/ with W=1. Hence revert commit 30c2de0a267c and instead
make NR_LRU_BASE and NR_ZONE_LRU_BASE integer constants instead of
enumeration constants. Additionally, convert the item##_NORMAL constants
from type enum vm_event_item into int before subtracting ZONE_NORMAL. The
latter constant has type enum zone_type.

Cc: Ivan Shapovalov <intelfx@intelfx.name>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: David Laight <david.laight@aculab.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 include/linux/mmzone.h | 26 +++++++++++++++++++++-----
 include/linux/vmstat.h |  9 +++++++--
 2 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 9540b41894da..bc139c704538 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -135,11 +135,20 @@ enum numa_stat_item {
 #define NR_VM_NUMA_EVENT_ITEMS 0
 #endif
 
+/*
+ * NR_ZONE_LRU_BASE and NR_VM_ZONE_STAT_ITEMS are often added to enumeration
+ * constants of another type than enum_zone_stat_item. Define these constants
+ * as an integer instead of enum node_stat_item to prevent that the compiler
+ * warns about enumeration type mismatches when these constants are used.
+ */
+#define NR_ZONE_LRU_BASE	(__NR_ZONE_LRU_BASE + 0)
+#define NR_VM_ZONE_STAT_ITEMS	(__NR_VM_ZONE_STAT_ITEMS + 0)
+
 enum zone_stat_item {
 	/* First 128 byte cacheline (assuming 64 bit words) */
 	NR_FREE_PAGES,
-	NR_ZONE_LRU_BASE, /* Used only for compaction and reclaim retry */
-	NR_ZONE_INACTIVE_ANON = NR_ZONE_LRU_BASE,
+	__NR_ZONE_LRU_BASE, /* Used only for compaction and reclaim retry */
+	NR_ZONE_INACTIVE_ANON = __NR_ZONE_LRU_BASE,
 	NR_ZONE_ACTIVE_ANON,
 	NR_ZONE_INACTIVE_FILE,
 	NR_ZONE_ACTIVE_FILE,
@@ -155,11 +164,18 @@ enum zone_stat_item {
 #ifdef CONFIG_UNACCEPTED_MEMORY
 	NR_UNACCEPTED,
 #endif
-	NR_VM_ZONE_STAT_ITEMS };
+	__NR_VM_ZONE_STAT_ITEMS };
+
+/*
+ * enum lru_list constants are often added to NR_LRU_BASE. Define NR_LRU_BASE
+ * as an integer instead of enum node_stat_item to prevent that the compiler
+ * warns about enumeration type mismatches.
+ */
+#define NR_LRU_BASE (__NR_LRU_BASE + 0)
 
 enum node_stat_item {
-	NR_LRU_BASE,
-	NR_INACTIVE_ANON = NR_LRU_BASE, /* must match order of LRU_[IN]ACTIVE */
+	__NR_LRU_BASE,
+	NR_INACTIVE_ANON = __NR_LRU_BASE, /*must match order of LRU_[IN]ACTIVE*/
 	NR_ACTIVE_ANON,		/*  "     "     "   "       "         */
 	NR_INACTIVE_FILE,	/*  "     "     "   "       "         */
 	NR_ACTIVE_FILE,		/*  "     "     "   "       "         */
diff --git a/include/linux/vmstat.h b/include/linux/vmstat.h
index 9f3a04345b86..72d1974ee81f 100644
--- a/include/linux/vmstat.h
+++ b/include/linux/vmstat.h
@@ -135,8 +135,13 @@ static inline void vm_events_fold_cpu(int cpu)
 #define count_vm_vma_lock_event(x) do {} while (0)
 #endif
 
+/*
+ * item##_NORMAL has type enum vm_event_item while ZONE_NORMAL and zid have
+ * type enum zone_type. Suppress compiler warnings about mixing different
+ * enumeration types by converting item##_NORMAL into an int with '+ 0'.
+ */
 #define __count_zid_vm_events(item, zid, delta) \
-	__count_vm_events(item##_NORMAL - ZONE_NORMAL + zid, delta)
+	__count_vm_events((item##_NORMAL + 0) - ZONE_NORMAL + zid, delta)
 
 /*
  * Zone and node-based page accounting with per cpu differentials.
@@ -515,7 +520,7 @@ static inline const char *node_stat_name(enum node_stat_item item)
 
 static inline const char *lru_list_name(enum lru_list lru)
 {
-	return node_stat_name(NR_LRU_BASE + (enum node_stat_item)lru) + 3; // skip "nr_"
+	return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
 }
 
 #if defined(CONFIG_VM_EVENT_COUNTERS) || defined(CONFIG_MEMCG)

Re: [PATCH] mm: Fix clang W=1 compiler warnings

[Vlastimil Babka]

On 1/31/25 20:12, Bart Van Assche wrote:
> Commit 30c2de0a267c ("mm/vmstat: fix a W=1 clang compiler warning")
> suppresses some but not all compiler warnings that are reported by clang
> when building mm/ with W=1. Hence revert commit 30c2de0a267c and instead
> make NR_LRU_BASE and NR_ZONE_LRU_BASE integer constants instead of
> enumeration constants. Additionally, convert the item##_NORMAL constants
> from type enum vm_event_item into int before subtracting ZONE_NORMAL. The
> latter constant has type enum zone_type.
> 
> Cc: Ivan Shapovalov <intelfx@intelfx.name>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: David Laight <david.laight@aculab.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>

Acked-by: Vlastimil Babka <vbabka@suse.cz>

Thanks.

Re: [PATCH] mm: Fix clang W=1 compiler warnings

[Jakub Kicinski]

On Fri, 31 Jan 2025 11:12:23 -0800 Bart Van Assche wrote:
> Commit 30c2de0a267c ("mm/vmstat: fix a W=1 clang compiler warning")
> suppresses some but not all compiler warnings that are reported by clang
> when building mm/ with W=1. Hence revert commit 30c2de0a267c and instead
> make NR_LRU_BASE and NR_ZONE_LRU_BASE integer constants instead of
> enumeration constants. Additionally, convert the item##_NORMAL constants
> from type enum vm_event_item into int before subtracting ZONE_NORMAL. The
> latter constant has type enum zone_type.

Could we possibly please still consider taking this in for 6.14? :(
Since the warning comes from vmstat.h pretty much every object file
generates this warning. clang 19 is getting more widely used now, 
its making it hard to see new warnings.

> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 9540b41894da..bc139c704538 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -135,11 +135,20 @@ enum numa_stat_item {
>  #define NR_VM_NUMA_EVENT_ITEMS 0
>  #endif
>  
> +/*
> + * NR_ZONE_LRU_BASE and NR_VM_ZONE_STAT_ITEMS are often added to enumeration
> + * constants of another type than enum_zone_stat_item. Define these constants
> + * as an integer instead of enum node_stat_item to prevent that the compiler
> + * warns about enumeration type mismatches when these constants are used.
> + */
> +#define NR_ZONE_LRU_BASE	(__NR_ZONE_LRU_BASE + 0)
> +#define NR_VM_ZONE_STAT_ITEMS	(__NR_VM_ZONE_STAT_ITEMS + 0)
> +
>  enum zone_stat_item {
>  	/* First 128 byte cacheline (assuming 64 bit words) */
>  	NR_FREE_PAGES,
> -	NR_ZONE_LRU_BASE, /* Used only for compaction and reclaim retry */
> -	NR_ZONE_INACTIVE_ANON = NR_ZONE_LRU_BASE,
> +	__NR_ZONE_LRU_BASE, /* Used only for compaction and reclaim retry */
> +	NR_ZONE_INACTIVE_ANON = __NR_ZONE_LRU_BASE,
>  	NR_ZONE_ACTIVE_ANON,
>  	NR_ZONE_INACTIVE_FILE,
>  	NR_ZONE_ACTIVE_FILE,
> @@ -155,11 +164,18 @@ enum zone_stat_item {
>  #ifdef CONFIG_UNACCEPTED_MEMORY
>  	NR_UNACCEPTED,
>  #endif
> -	NR_VM_ZONE_STAT_ITEMS };
> +	__NR_VM_ZONE_STAT_ITEMS };
> +
> +/*
> + * enum lru_list constants are often added to NR_LRU_BASE. Define NR_LRU_BASE
> + * as an integer instead of enum node_stat_item to prevent that the compiler
> + * warns about enumeration type mismatches.
> + */
> +#define NR_LRU_BASE (__NR_LRU_BASE + 0)
>  
>  enum node_stat_item {
> -	NR_LRU_BASE,
> -	NR_INACTIVE_ANON = NR_LRU_BASE, /* must match order of LRU_[IN]ACTIVE */
> +	__NR_LRU_BASE,
> +	NR_INACTIVE_ANON = __NR_LRU_BASE, /*must match order of LRU_[IN]ACTIVE*/
>  	NR_ACTIVE_ANON,		/*  "     "     "   "       "         */
>  	NR_INACTIVE_FILE,	/*  "     "     "   "       "         */
>  	NR_ACTIVE_FILE,		/*  "     "     "   "       "         */
> diff --git a/include/linux/vmstat.h b/include/linux/vmstat.h
> index 9f3a04345b86..72d1974ee81f 100644
> --- a/include/linux/vmstat.h
> +++ b/include/linux/vmstat.h
> @@ -135,8 +135,13 @@ static inline void vm_events_fold_cpu(int cpu)
>  #define count_vm_vma_lock_event(x) do {} while (0)
>  #endif
>  
> +/*
> + * item##_NORMAL has type enum vm_event_item while ZONE_NORMAL and zid have
> + * type enum zone_type. Suppress compiler warnings about mixing different
> + * enumeration types by converting item##_NORMAL into an int with '+ 0'.
> + */
>  #define __count_zid_vm_events(item, zid, delta) \
> -	__count_vm_events(item##_NORMAL - ZONE_NORMAL + zid, delta)
> +	__count_vm_events((item##_NORMAL + 0) - ZONE_NORMAL + zid, delta)
>  
>  /*
>   * Zone and node-based page accounting with per cpu differentials.
> @@ -515,7 +520,7 @@ static inline const char *node_stat_name(enum node_stat_item item)
>  
>  static inline const char *lru_list_name(enum lru_list lru)
>  {
> -	return node_stat_name(NR_LRU_BASE + (enum node_stat_item)lru) + 3; // skip "nr_"
> +	return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
>  }
>  
>  #if defined(CONFIG_VM_EVENT_COUNTERS) || defined(CONFIG_MEMCG)
> 
> 

Re: [PATCH] mm: Fix clang W=1 compiler warnings

[Linus Torvalds]

On Fri, 7 Feb 2025 at 16:49, Jakub Kicinski <kuba@kernel.org> wrote:
>
> Could we possibly please still consider taking this in for 6.14? :(
> Since the warning comes from vmstat.h pretty much every object file
> generates this warning. clang 19 is getting more widely used now,
> its making it hard to see new warnings.

So:

 - I build the kernel with clang, but I don't have clang-19, so it's
kind of pointless sending patches that DO NOT EVEN EXPLAIN WHAT THE
WARNINGS ARE.

 - and even if you explain *WHAT* the warnings are, please also
explain *WHY* they are valid and should be cared about.

Because honestly, W=1 is literally meant for "warnings that aren't
necessarily valid". That's why they aren't on by default.

So no, I'm certainly not applying unexplained random patches that
don't bother to explain the what or the why. Not for 6.14, not ever.

Fix the patch. Explain the problem. And possibly just disable the warning.

               Linus

Re: [PATCH] mm: Fix clang W=1 compiler warnings

[Jakub Kicinski]

On Fri, 7 Feb 2025 17:01:00 -0800 Linus Torvalds wrote:
> On Fri, 7 Feb 2025 at 16:49, Jakub Kicinski <kuba@kernel.org> wrote:
> > Could we possibly please still consider taking this in for 6.14? :(
> > Since the warning comes from vmstat.h pretty much every object file
> > generates this warning. clang 19 is getting more widely used now,
> > its making it hard to see new warnings.  
> 
> So:
> 
>  - I build the kernel with clang, but I don't have clang-19, so it's
> kind of pointless sending patches that DO NOT EVEN EXPLAIN WHAT THE
> WARNINGS ARE.
> 
>  - and even if you explain *WHAT* the warnings are, please also
> explain *WHY* they are valid and should be cared about.
> 
> Because honestly, W=1 is literally meant for "warnings that aren't
> necessarily valid". That's why they aren't on by default.
> 
> So no, I'm certainly not applying unexplained random patches that
> don't bother to explain the what or the why. Not for 6.14, not ever.

I thought I'd ask.. :) FWIW this for every single object:

  CC      net/core/request_sock.o
In file included from ../net/core/request_sock.c:14:
In file included from ../include/linux/tcp.h:17:
In file included from ../include/linux/skbuff.h:17:
In file included from ../include/linux/bvec.h:10:
In file included from ../include/linux/highmem.h:8:
In file included from ../include/linux/cacheflush.h:5:
In file included from ../arch/x86/include/asm/cacheflush.h:5:
In file included from ../include/linux/mm.h:2224:
../include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
  504 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
      |                            ~~~~~~~~~~~~~~~~~~~~~ ^
  505 |                            item];
      |                            ~~~~
../include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
  511 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
      |                            ~~~~~~~~~~~~~~~~~~~~~ ^
  512 |                            NR_VM_NUMA_EVENT_ITEMS +
      |                            ~~~~~~~~~~~~~~~~~~~~~~
../include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
  524 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
      |                            ~~~~~~~~~~~~~~~~~~~~~ ^
  525 |                            NR_VM_NUMA_EVENT_ITEMS +
      |                            ~~~~~~~~~~~~~~~~~~~~~~
3 warnings generated.

> Fix the patch. Explain the problem. And possibly just disable the warning.

That'd be great. Nathan, would that be okay with you?

Re: [PATCH] mm: Fix clang W=1 compiler warnings

[Nathan Chancellor]

On Fri, Feb 07, 2025 at 05:38:13PM -0800, Jakub Kicinski wrote:
> On Fri, 7 Feb 2025 17:01:00 -0800 Linus Torvalds wrote:
> > On Fri, 7 Feb 2025 at 16:49, Jakub Kicinski <kuba@kernel.org> wrote:
> > > Could we possibly please still consider taking this in for 6.14? :(
> > > Since the warning comes from vmstat.h pretty much every object file
> > > generates this warning. clang 19 is getting more widely used now,
> > > its making it hard to see new warnings.  
> > 
> > So:
> > 
> >  - I build the kernel with clang, but I don't have clang-19, so it's
> > kind of pointless sending patches that DO NOT EVEN EXPLAIN WHAT THE
> > WARNINGS ARE.
> > 
> >  - and even if you explain *WHAT* the warnings are, please also
> > explain *WHY* they are valid and should be cared about.
> > 
> > Because honestly, W=1 is literally meant for "warnings that aren't
> > necessarily valid". That's why they aren't on by default.
> > 
> > So no, I'm certainly not applying unexplained random patches that
> > don't bother to explain the what or the why. Not for 6.14, not ever.
> 
> I thought I'd ask.. :) FWIW this for every single object:
> 
>   CC      net/core/request_sock.o
> In file included from ../net/core/request_sock.c:14:
> In file included from ../include/linux/tcp.h:17:
> In file included from ../include/linux/skbuff.h:17:
> In file included from ../include/linux/bvec.h:10:
> In file included from ../include/linux/highmem.h:8:
> In file included from ../include/linux/cacheflush.h:5:
> In file included from ../arch/x86/include/asm/cacheflush.h:5:
> In file included from ../include/linux/mm.h:2224:
> ../include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
>   504 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
>       |                            ~~~~~~~~~~~~~~~~~~~~~ ^
>   505 |                            item];
>       |                            ~~~~
> ../include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
>   511 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
>       |                            ~~~~~~~~~~~~~~~~~~~~~ ^
>   512 |                            NR_VM_NUMA_EVENT_ITEMS +
>       |                            ~~~~~~~~~~~~~~~~~~~~~~
> ../include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
>   524 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
>       |                            ~~~~~~~~~~~~~~~~~~~~~ ^
>   525 |                            NR_VM_NUMA_EVENT_ITEMS +
>       |                            ~~~~~~~~~~~~~~~~~~~~~~
> 3 warnings generated.
> 
> > Fix the patch. Explain the problem. And possibly just disable the warning.
> 
> That'd be great. Nathan, would that be okay with you?

FWIW, I sent a patch to move this warning to W=2 in October and pinged
it in December:

https://lore.kernel.org/20241017-disable-two-clang-enum-warnings-v2-1-163ac04346ae@kernel.org/

'b4 shazam' tells me that it is still applicable on Linus's tree so
maybe he can just apply it directly?

Cheers,
Nathan

Re: [PATCH] mm: Fix clang W=1 compiler warnings

[Linus Torvalds]

On Fri, 7 Feb 2025 at 17:38, Jakub Kicinski <kuba@kernel.org> wrote:
>
> ../include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]

Ok.

So my gut feel is that this warning is simply bogus. We have
situations where we very intentionally use enumerations instead of a
list of #define constants.

That NR_VM_ZONE_STAT_ITEMS case really *does* look like it's a great
example of how code that doesn't care about the actual numbers, and
just wants the compiler to generate a sequence for us is supposed to
look.

Because sometimes we do enums just because it's a nice way to get
automatically incrementing values when we don't care exactly what the
values are.

And sometimes we do it for actual namespace reasons - see for example
commit 1a251f52cfdc ("minmax: make generic MIN() and MAX() macros
available everywhere") where in order to avoid clashes with the
"MIN()" macro, I made a couple of drivers that had a 'MIN' value (much
too generic a name, but it was what it was) use an enum instead.

See

   git show 1a251f52cfdc drivers/hwmon/adt7475.c

for details.

Now, at least in that case I don't think there was any arithmetic with
those values, so clang-19 wouldn't complain about it either, but I
mention that commit as a case of "it's actually very valid to use an
enum for actual real honest-to-goodness integer value enumeration".

Which really makes me feel like the new clang warning is seriously misguided.

In sparse I actually wanted to have integer types that don't silently
cast to other integer types. So sparse knows the concept of "bitwise"
and "nocast" attributes.  The "nocast" thing doesn't work well in
practice, but "bitwise" was a huge success and is how a lot of our
endianness problems were solved.

So the *concept* of an enum - or any other integer type - that "stays
in the type" is good, but I think it's a major mistake to think they
have to do so without any sane escape, and to only limit it to enums.

That said, we may not have *many* of those enum cases in the kernel
(and we do tend to have a history of using long series of '#define'
sequences to do these constants), so maybe the warning is acceptable
if it's a case of "this is literally the *only* one in the kernel".

But not having clang-19, I can't see if this is a case of "this is so
rare that let's just avoid it", or "this is the case that causes the
most noise for every file build, but there are lots of other cases of
this".

                  Linus

Re: [PATCH] mm: Fix clang W=1 compiler warnings

[Nathan Chancellor]

On Fri, Feb 07, 2025 at 07:11:08PM -0800, Linus Torvalds wrote:
> That said, we may not have *many* of those enum cases in the kernel
> (and we do tend to have a history of using long series of '#define'
> sequences to do these constants), so maybe the warning is acceptable
> if it's a case of "this is literally the *only* one in the kernel".
> 
> But not having clang-19, I can't see if this is a case of "this is so
> rare that let's just avoid it", or "this is the case that causes the
> most noise for every file build, but there are lots of other cases of
> this".

When this warning was turned on for C in clang-19 (it was C++ only prior
to that IIRC), it was extremely noisy. Some of that was due to the
warning occurring in headers that are included everywhere such as these
couple of ones but even hiding the big ones, there were still a bunch of
locations that triggered it (I did not do the best at hiding all of the
header ones because I had given up trying to make leaving the warning on
for the default build at that point).

The diff to hide some of the really common ones:

https://github.com/ClangBuiltLinux/linux/issues/2002#issuecomment-1970004069

The build log with that:

https://gist.github.com/nathanchance/971e5abeba504d3017cd6ed4517bbda6

I looked at a number of them and none of them really seemed like bugs to
me.

Cheers,
Nathan

Re: [PATCH] mm: Fix clang W=1 compiler warnings

[Linus Torvalds]

On Fri, 7 Feb 2025 at 19:33, Nathan Chancellor <nathan@kernel.org> wrote:
>
> On Fri, Feb 07, 2025 at 07:11:08PM -0800, Linus Torvalds wrote:
> >
> > But not having clang-19, I can't see if this is a case of "this is so
> > rare that let's just avoid it", or "this is the case that causes the
> > most noise for every file build, but there are lots of other cases of
> > this".
>
> The diff to hide some of the really common ones:
>
> https://github.com/ClangBuiltLinux/linux/issues/2002#issuecomment-1970004069

Oh, ok, that's already more than I would have liked to see, and I do
think that the patch is not only bigger than I'd like, it makes the
code actively worse with adding random casts just to shut the compiler
up.

That is, of course, something that C++ people think is a good thing,
since they are used to their traditional NULL casting idiocy. But in
C, we have actual taste and grace.

So no, that warning is no good for the kernel.

> The build log with that:
>
> https://gist.github.com/nathanchance/971e5abeba504d3017cd6ed4517bbda6
>
> I looked at a number of them and none of them really seemed like bugs to
> me.

Yeah, those seem like more of the same: the kernel doing sane things,
using enum's the way they are meant to be used, and the compiler
warning is just simply plain wrong.

I will take your patch to move it to W=2.

Thanks,
                   Linus

Re: [PATCH] mm: Fix clang W=1 compiler warnings

[Linus Torvalds]

On Fri, 7 Feb 2025 at 19:49, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> I will take your patch to move it to W=2.

Ok, applied and pushed out as commit 8f6629c004b1 ("kbuild: Move
-Wenum-enum-conversion to W=2").

Jakub, hopefully this fixes your situation,

                Linus

Re: [PATCH] mm: Fix clang W=1 compiler warnings

[Jakub Kicinski]

On Fri, 7 Feb 2025 20:24:18 -0800 Linus Torvalds wrote:
> > I will take your patch to move it to W=2.  
> 
> Ok, applied and pushed out as commit 8f6629c004b1 ("kbuild: Move
> -Wenum-enum-conversion to W=2").
> 
> Jakub, hopefully this fixes your situation,

Much appreciated, thank you!

Re: [PATCH] mm: Fix clang W=1 compiler warnings

[Bart Van Assche]

On 2/7/25 5:01 PM, Linus Torvalds wrote:
> On Fri, 7 Feb 2025 at 16:49, Jakub Kicinski <kuba@kernel.org> wrote:
>>
>> Could we possibly please still consider taking this in for 6.14? :(
>> Since the warning comes from vmstat.h pretty much every object file
>> generates this warning. clang 19 is getting more widely used now,
>> its making it hard to see new warnings.
> 
> So:
> 
>   - I build the kernel with clang, but I don't have clang-19, so it's
> kind of pointless sending patches that DO NOT EVEN EXPLAIN WHAT THE
> WARNINGS ARE.
> 
>   - and even if you explain *WHAT* the warnings are, please also
> explain *WHY* they are valid and should be cared about.
> 
> Because honestly, W=1 is literally meant for "warnings that aren't
> necessarily valid". That's why they aren't on by default.
> 
> So no, I'm certainly not applying unexplained random patches that
> don't bother to explain the what or the why. Not for 6.14, not ever.
> 
> Fix the patch. Explain the problem. And possibly just disable the warning.

Got it.

Since the patch at the start of this e-mail thread is a follow-up for
commit 30c2de0a267c ("mm/vmstat: fix a W=1 clang compiler warning"), do
you perhaps want me to submit a revert for that commit?

Thanks,

Bart.

Re: [PATCH] mm: Fix clang W=1 compiler warnings

[Linus Torvalds]

On Fri, 7 Feb 2025 at 18:56, Bart Van Assche <bvanassche@acm.org> wrote:
>
> Since the patch at the start of this e-mail thread is a follow-up for
> commit 30c2de0a267c ("mm/vmstat: fix a W=1 clang compiler warning"), do
> you perhaps want me to submit a revert for that commit?

So I _think_ the proper fix is to just disable the warning (or move it
to W=2 like apparently Nathan did - crossing emails left and right
here), and once that is done, yes, reverting that commit too.

That said, I'll repeat that while I think using enums for values that
then get used for arithmetic can actually be a good thing, I'm not
entirely against the notion of saying "let's limit ourselves".

So if it's _so_ rare that these are the *only* cases of that warning
happening in the whole kernel build, then I'll just concede that while
I still think it's a fine pattern, if it's *so* rare that we only had
two cases of it, then it's worth fixing those two cases.

Because at some point "really really unusual" might be worth warning
for, even if the unusual case isn't wrong per se.

But I don't have any visibility into whether this is just one header
file fix and we're done, or whether it's actually more common, and the
one header file case is just a "this causes lots of noise because it's
included everywhere".

           Linus

Re: [PATCH] mm: Fix clang W=1 compiler warnings

[Bart Van Assche]

On 2/7/25 7:22 PM, Linus Torvalds wrote:
> So if it's _so_ rare that these are the *only* cases of that warning
> happening in the whole kernel build, then I'll just concede that while
> I still think it's a fine pattern, if it's *so* rare that we only had
> two cases of it, then it's worth fixing those two cases.
> 
> Because at some point "really really unusual" might be worth warning
> for, even if the unusual case isn't wrong per se.
> 
> But I don't have any visibility into whether this is just one header
> file fix and we're done, or whether it's actually more common, and the
> one header file case is just a "this causes lots of noise because it's
> included everywhere".

Hi Linus,

That's an interesting question: how often -Wenum-enum-conversion
warnings are triggered. I ran into these warnings while building the
code in the block/ directory with W=1. For almost every source file in
the block directory that was built, -Wenum-enum-conversion warnings
appeared for the header files include/linux/mmzone.h or
include/linux/vmstat.h. This is why I posted the patch at the start
of this email thread. Of course, moving -Wenum-enum-conversion from
W=1 to W=2 is also fine with me.

Thanks,

Bart.

Re: [PATCH] mm: Fix clang W=1 compiler warnings

[Matthew Wilcox]

On Fri, Jan 31, 2025 at 11:12:23AM -0800, Bart Van Assche wrote:
> Commit 30c2de0a267c ("mm/vmstat: fix a W=1 clang compiler warning")
> suppresses some but not all compiler warnings that are reported by clang
> when building mm/ with W=1. Hence revert commit 30c2de0a267c and instead
> make NR_LRU_BASE and NR_ZONE_LRU_BASE integer constants instead of
> enumeration constants. Additionally, convert the item##_NORMAL constants
> from type enum vm_event_item into int before subtracting ZONE_NORMAL. The
> latter constant has type enum zone_type.

this patch was NAKed last year

https://lore.kernel.org/all/ZnXC5Xa4R0Mp7FCB@casper.infradead.org/

Re: [PATCH] mm: Fix clang W=1 compiler warnings

[Vlastimil Babka]

On 2/8/25 11:28, Matthew Wilcox wrote:
> On Fri, Jan 31, 2025 at 11:12:23AM -0800, Bart Van Assche wrote:
>> Commit 30c2de0a267c ("mm/vmstat: fix a W=1 clang compiler warning")
>> suppresses some but not all compiler warnings that are reported by clang
>> when building mm/ with W=1. Hence revert commit 30c2de0a267c and instead
>> make NR_LRU_BASE and NR_ZONE_LRU_BASE integer constants instead of
>> enumeration constants. Additionally, convert the item##_NORMAL constants
>> from type enum vm_event_item into int before subtracting ZONE_NORMAL. The
>> latter constant has type enum zone_type.
> 
> this patch was NAKed last year

Looks like not strongly enough :)

> https://lore.kernel.org/all/ZnXC5Xa4R0Mp7FCB@casper.infradead.org/

With the warning now in W=2, want to submit the revert of above, Bart?

Re: [PATCH] mm: Fix clang W=1 compiler warnings

[Bart Van Assche]

On 2/11/25 6:34 AM, Vlastimil Babka wrote:
> On 2/8/25 11:28, Matthew Wilcox wrote:
>> this patch was NAKed last year
 >>
>> https://lore.kernel.org/all/ZnXC5Xa4R0Mp7FCB@casper.infradead.org/

I wasn't Cc-ed on that email thread so I hadn't seen it yet.

> With the warning now in W=2, want to submit the revert of above, Bart?

OK, I will submit a revert of commit 30c2de0a267c ("mm/vmstat: fix a W=1
clang compiler warning"). I see that Nathan's change already made it in
Linus' tree as commit 8f6629c004b1 ("kbuild: Move -Wenum-enum-conversion
to W=2").

Thanks,

Bart.

Re: [PATCH] mm: Fix clang W=1 compiler warnings

[Matthew Wilcox]

On Tue, Feb 11, 2025 at 10:48:47AM -0800, Bart Van Assche wrote:
> On 2/11/25 6:34 AM, Vlastimil Babka wrote:
> > On 2/8/25 11:28, Matthew Wilcox wrote:
> > > this patch was NAKed last year
> >>
> > > https://lore.kernel.org/all/ZnXC5Xa4R0Mp7FCB@casper.infradead.org/
> 
> I wasn't Cc-ed on that email thread so I hadn't seen it yet.

Yes, but email lists have archives for a reason.  You can always look to
see if you're the first person to encounter a problem.

> > With the warning now in W=2, want to submit the revert of above, Bart?
> 
> OK, I will submit a revert of commit 30c2de0a267c ("mm/vmstat: fix a W=1
> clang compiler warning"). I see that Nathan's change already made it in
> Linus' tree as commit 8f6629c004b1 ("kbuild: Move -Wenum-enum-conversion
> to W=2").

Thank you.