linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Ivan Shapovalov <intelfx@intelfx.name>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Ivan Shapovalov <intelfx@intelfx.name>,
	Nathan Chancellor <nathan@kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Bill Wendling <morbo@google.com>,
	Justin Stitt <justinstitt@google.com>,
	Pasha Tatashin <pasha.tatashin@soleen.com>,
	David Rientjes <rientjes@google.com>,
	David Hildenbrand <david@redhat.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	Joel Granados <joel.granados@kernel.org>,
	Sourav Panda <souravpanda@google.com>,
	Bart Van Assche <bvanassche@acm.org>,
	Kaiyang Zhao <kaiyang2@cs.cmu.edu>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Konstantin Khlebnikov <koct9i@gmail.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	llvm@lists.linux.dev
Subject: Re: [PATCH] mm/vmstat: Fix a W=1 clang compiler warning
Date: Wed, 22 Jan 2025 04:57:15 +0300	[thread overview]
Message-ID: <20250122015818.3308696-1-intelfx@intelfx.name> (raw)
In-Reply-To: <20241212182425.ad1f7894cd0f00b2e34bbaed@linux-foundation.org>

> 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.

All of these *_name() functions do seem to expect arguments in range of
the corresponding enums, so perhaps keep those args typed as a form of
self-documenting code, and do this instead?

---
From f8960df01b7f4fdc8d09a366886563385aed7f18 Mon Sep 17 00:00:00 2001
From: Ivan Shapovalov <intelfx@intelfx.name>
Date: Wed, 22 Jan 2025 04:10:59 +0300
Subject: [PATCH] mm: fix more clang compiler warnings

In the similar vein as 30c2de0a267c ("mm/vmstat: fix a W=1 clang
compiler warning"), fix several more sites that generate warnings
with clang 19.1.7:

./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.

Similarly for mm_inline.h:

./include/linux/mm_inline.h:47:41: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
   47 |         __mod_lruvec_state(lruvec, NR_LRU_BASE + lru, nr_pages);
      |                                    ~~~~~~~~~~~ ^ ~~~
./include/linux/mm_inline.h:49:22: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
   49 |                                 NR_ZONE_LRU_BASE + lru, nr_pages);
      |                                 ~~~~~~~~~~~~~~~~ ^ ~~~
2 warnings generated.

Additionally, change the lru_list_name() and mm_inline.h sites to use
the same `(int)ENUMERATOR + ...` idiom to match the three first fixes
above for consistency. Everywhere else is's semantically more correct
thing to do because we are not really treating any of these enums as
other enums but rather treating all of those as indices.

Link: https://lore.kernel.org/all/20241212213126.1269116-1-bvanassche@acm.org/
Fixes: 9d7ea9a297e6 ("mm/vmstat: add helpers to get vmstat item names for each enum type")
Fixes: 30c2de0a267c ("mm/vmstat: fix a W=1 clang compiler warning")
Signed-off-by: Ivan Shapovalov <intelfx@intelfx.name>
---
 include/linux/mm_inline.h | 4 ++--
 include/linux/vmstat.h    | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
index 1b6a917fffa4..5a2328728b21 100644
--- a/include/linux/mm_inline.h
+++ b/include/linux/mm_inline.h
@@ -44,9 +44,9 @@ static __always_inline void __update_lru_size(struct lruvec *lruvec,
 	lockdep_assert_held(&lruvec->lru_lock);
 	WARN_ON_ONCE(nr_pages != (int)nr_pages);
 
-	__mod_lruvec_state(lruvec, NR_LRU_BASE + lru, nr_pages);
+	__mod_lruvec_state(lruvec, (int)NR_LRU_BASE + lru, nr_pages);
 	__mod_zone_page_state(&pgdat->node_zones[zid],
-				NR_ZONE_LRU_BASE + lru, nr_pages);
+				(int)NR_ZONE_LRU_BASE + lru, nr_pages);
 }
 
 static __always_inline void update_lru_size(struct lruvec *lruvec,
diff --git a/include/linux/vmstat.h b/include/linux/vmstat.h
index 9f3a04345b86..545568a4ea16 100644
--- a/include/linux/vmstat.h
+++ b/include/linux/vmstat.h
@@ -501,27 +501,27 @@ static inline const char *zone_stat_name(enum zone_stat_item item)
 #ifdef CONFIG_NUMA
 static inline const char *numa_stat_name(enum numa_stat_item item)
 {
-	return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
+	return vmstat_text[(int)NR_VM_ZONE_STAT_ITEMS +
 			   item];
 }
 #endif /* CONFIG_NUMA */
 
 static inline const char *node_stat_name(enum node_stat_item item)
 {
-	return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
+	return vmstat_text[(int)NR_VM_ZONE_STAT_ITEMS +
 			   NR_VM_NUMA_EVENT_ITEMS +
 			   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((int)NR_LRU_BASE + lru) + 3; // skip "nr_"
 }
 
 #if defined(CONFIG_VM_EVENT_COUNTERS) || defined(CONFIG_MEMCG)
 static inline const char *vm_event_name(enum vm_event_item item)
 {
-	return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
+	return vmstat_text[(int)NR_VM_ZONE_STAT_ITEMS +
 			   NR_VM_NUMA_EVENT_ITEMS +
 			   NR_VM_NODE_STAT_ITEMS +
 			   NR_VM_STAT_ITEMS +
-- 
2.48.1.5.g9188e14f140



  parent reply	other threads:[~2025-01-22  1:58 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-12 21:31 Bart Van Assche
2024-12-13  2:24 ` Andrew Morton
2024-12-13 22:01   ` Bart Van Assche
2025-01-22  1:57   ` Ivan Shapovalov [this message]
2025-01-22 21:55     ` Bart Van Assche
2025-01-28 21:36     ` Bart Van Assche
2025-01-29 10:22       ` Vlastimil Babka
2025-01-29 17:30         ` Bart Van Assche
2025-01-29 22:46         ` David Laight

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250122015818.3308696-1-intelfx@intelfx.name \
    --to=intelfx@intelfx.name \
    --cc=akpm@linux-foundation.org \
    --cc=bvanassche@acm.org \
    --cc=david@redhat.com \
    --cc=hannes@cmpxchg.org \
    --cc=joel.granados@kernel.org \
    --cc=justinstitt@google.com \
    --cc=kaiyang2@cs.cmu.edu \
    --cc=koct9i@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=llvm@lists.linux.dev \
    --cc=morbo@google.com \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=pasha.tatashin@soleen.com \
    --cc=rientjes@google.com \
    --cc=souravpanda@google.com \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox