Re: [PATCH] mm/vmstat: Fix -Wenum-enum-conversion warning in vmstat.h

[Nathan Chancellor <nathan@kernel.org>]

Hi Aleksei,

On Mon, Oct 07, 2024 at 08:13:41PM +0000, Aleksei Vetrov wrote:
> Hi Shivamurthy,
> 
> On Fri, Jun 21, 2024 at 01:16:04PM +0200, Shivamurthy Shastri wrote:
> > A W=1 build with -Wenum-enum-conversion enabled, results in the
> > following build warning due to an arithmetic operation between different
> > enumeration types 'enum node_stat_item' and 'enum lru_list':
> > 
> >   include/linux/vmstat.h:514:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
> >     514 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
> >         |                               ~~~~~~~~~~~ ^ ~~~
> > 
> > Address this by casting lru to the proper type.
> > 
> > Signed-off-by: Shivamurthy Shastri <shivamurthy.shastri@linutronix.de>
> > Reviewed-by: Anna-Maria Behnsen <anna-maria@linutronix.de>
> > ---
> >  include/linux/vmstat.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/include/linux/vmstat.h b/include/linux/vmstat.h
> > index 735eae6e272c..72ecd46fd0c4 100644
> > --- a/include/linux/vmstat.h
> > +++ b/include/linux/vmstat.h
> > @@ -511,7 +511,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 + lru) + 3; // skip "nr_"
> > +	return node_stat_name(NR_LRU_BASE + (enum node_stat_item)lru) + 3; // skip "nr_"
> >  }
> >  
> >  static inline const char *writeback_stat_name(enum writeback_stat_item item)
> > -- 
> > 2.34.1
> > 
> 
> We have encountered the same problem after trying to update Clang to the
> latest version and this is a blocker because we use W=1 to compile the
> kernel.
> 
> Do you plan to address review comments about casting to int instead of
> enum node_stat_item? Or I can submit another patch myself that addresses
> it.

For what it's worth, I never really saw Matthew's comment around what
value does this warning provide addressed. I was the one who originally
moved it into W=1 at the request of Arnd because he felt that instances
of this warning could be bugs and they should be audited.  However, I
have not seen many instances of this warning pop up in new code through
0day build reports and the ones that I have seen seem to be intentional,
as they are using enums like integral values, such as here. If that is
just going to result in a bunch of patches like this adding unnecessary
casts, I think it would just be better to consider disabling this
warning altogether or at the very least, moving it to W=2 (which is for
warnings that are noisy but might contain bugs), since more people are
using W=1 as their normal build configuration nowadays.

Cheers,
Nathan