| Message ID | 20241212213126.1269116-1-bvanassche@acm.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/vmstat: Fix a W=1 clang compiler warning | expand |
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.
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, /* " " " " " */
diff --git a/include/linux/vmstat.h b/include/linux/vmstat.h index d2761bf8ff32..9f3a04345b86 100644 --- 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_" } #if defined(CONFIG_VM_EVENT_COUNTERS) || defined(CONFIG_MEMCG)
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_" | ~~~~~~~~~~~ ^ ~~~ Cc: Konstantin Khlebnikov <koct9i@gmail.com> Cc: Nathan Chancellor <nathan@kernel.org> Fixes: 9d7ea9a297e6 ("mm/vmstat: add helpers to get vmstat item names for each enum type") Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- include/linux/vmstat.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)