Re: [PATCH] mm/vmstat: Fix a W=1 clang compiler warning

[Bart Van Assche <bvanassche@acm.org>]

On 12/12/24 6:24 PM, Andrew Morton wrote:
> On Thu, 12 Dec 2024 13:31:26 -0800 Bart Van Assche <bvanassche@acm.org> wrote:
> 
>> Fix the following clang compiler warning that is reported if the kernel is
>> built with W=1:
>>
>> ./include/linux/vmstat.h:518:36: error: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Werror,-Wenum-enum-conversion]
>>    518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
>>        |                               ~~~~~~~~~~~ ^ ~~~
>>
>> ...
>>
>> --- a/include/linux/vmstat.h
>> +++ b/include/linux/vmstat.h
>> @@ -515,7 +515,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_"
>>   }
>>   
> 
> Spose so.  One always suspects that adding a typecast is a sign that we
> screwed things up somehow.  The relationship between enums lru_list and
> node_stat_item is foggy, and I'm unsure whether this is the place to
> make the transition it.  Perhaps lru_list_name() should take an
> `unsigned int' arg instead.

Could the (untested) patch below be a better approach?

Thanks,

Bart.

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index b36124145a16..a361cac06a85 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -135,10 +135,16 @@ enum numa_stat_item {
  #define NR_VM_NUMA_EVENT_ITEMS 0
  #endif

+/*
+ * enum lru_list constants are often added to NR_ZONE_LRU_BASE. Define
+ * NR_ZONE_LRU_BASE as an integer instead of enum node_stat_item to prevent
+ * that the compiler warns about enumeration type mismatches.
+ */
+#define NR_ZONE_LRU_BASE 1 /* Used only for compaction and reclaim retry */
+
  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_ACTIVE_ANON,
  	NR_ZONE_INACTIVE_FILE,
@@ -157,8 +163,14 @@ enum zone_stat_item {
  #endif
  	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 0
+
  enum node_stat_item {
-	NR_LRU_BASE,
  	NR_INACTIVE_ANON = NR_LRU_BASE, /* must match order of LRU_[IN]ACTIVE */
  	NR_ACTIVE_ANON,		/*  "     "     "   "       "         */
  	NR_INACTIVE_FILE,	/*  "     "     "   "       "         */