Re: [PATCH 3/4] mm/vmstat: remove unused __node_stat_* wrappers

[Joshua Hahn <joshua.hahnjy@gmail.com>]

On Wed, 15 Apr 2026 08:50:54 +0800 Ye Liu <ye.liu@linux.dev> wrote:

> 
> 
> 在 2026/4/14 22:59, Joshua Hahn 写道:
> > On Tue, 14 Apr 2026 17:15:20 +0800 Ye Liu <ye.liu@linux.dev> wrote:
> > 
> >> From: Ye Liu <liuye@kylinos.cn>
> >>
> >> Replace the single call to __node_stat_mod_folio()
> >> with node_stat_mod_folio(), and remove the dead inline __node_stat_*
> >> wrapper definitions from include/linux/vmstat.h.
> >>
> >> Signed-off-by: Ye Liu <liuye@kylinos.cn>
> >> ---
> >>  include/linux/vmstat.h | 18 ------------------
> >>  mm/page-writeback.c    |  2 +-
> >>  2 files changed, 1 insertion(+), 19 deletions(-)
> >>
> >> diff --git a/include/linux/vmstat.h b/include/linux/vmstat.h
> >> index 3c9c266cf782..54da7d820f78 100644
> >> --- a/include/linux/vmstat.h
> >> +++ b/include/linux/vmstat.h
> >> @@ -440,24 +440,6 @@ static inline void zone_stat_sub_folio(struct folio *folio,
> >>  	mod_zone_page_state(folio_zone(folio), item, -folio_nr_pages(folio));
> >>  }
> >>  
> >> -static inline void __node_stat_mod_folio(struct folio *folio,
> >> -		enum node_stat_item item, long nr)
> >> -{
> >> -	__mod_node_page_state(folio_pgdat(folio), item, nr);
> >> -}
> >> -
> >> -static inline void __node_stat_add_folio(struct folio *folio,
> >> -		enum node_stat_item item)
> >> -{
> >> -	__mod_node_page_state(folio_pgdat(folio), item, folio_nr_pages(folio));
> >> -}
> >> -
> >> -static inline void __node_stat_sub_folio(struct folio *folio,
> >> -		enum node_stat_item item)
> >> -{
> >> -	__mod_node_page_state(folio_pgdat(folio), item, -folio_nr_pages(folio));
> >> -}
> >> -
> >>  static inline void node_stat_mod_folio(struct folio *folio,
> >>  		enum node_stat_item item, long nr)
> >>  {
> >> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> >> index 6f9b7b081ab7..ed3301753e89 100644
> >> --- a/mm/page-writeback.c
> >> +++ b/mm/page-writeback.c
> >> @@ -2627,7 +2627,7 @@ static void folio_account_dirtied(struct folio *folio,
> >>  
> >>  		lruvec_stat_mod_folio(folio, NR_FILE_DIRTY, nr);
> >>  		__zone_stat_mod_folio(folio, NR_ZONE_WRITE_PENDING, nr);
> >> -		__node_stat_mod_folio(folio, NR_DIRTIED, nr);
> >> +		node_stat_mod_folio(folio, NR_DIRTIED, nr);
> > 
> > Hi Ye, thank you for the patch,
> > 
> > In addition to what Matthew has pointed out, I also wanted to note that this
> > substitution isn't trivial; there are differences between the __ prefixed
> > version of node_stat_mod_folio and the one without. Even though the correctness
> > of the two versions might be the same, I think that a change like this should
> > be supplemented by a description of what side effects this change has
> > (i.e. introducing additional overhead from the cmpxchg loop).
> > 
> > Thank you, I hope you have a great day!
> > Joshua
> 
> Thank you for your review, Joshua. Regarding the difference between
> __node_stat_mod_folio and node_stat_mod_folio: in the current implementation,
> both functions ultimately use __mod_node_page_state, as mod_node_page_state
> is defined as __mod_node_page_state. There is no functional difference
> between them in terms of atomicity or overhead. The __ prefixed versions
> were wrappers that are now unused, which is why we're removing them.

Hello Ye,

Thanks for the quick response. However, I'm not sure that's true.
For #ifdef CONFIG_HAVE_CMPXCHG_LOCAL in mm/vmstat.c, I can see that
mod_node_page_state calls mod_node_state, which includes the cmpxchg loop.
I think many users enalbe this config, including x86. IMHO, we should document
this effect of the changes.

Thank you, I hope you have a great day!
Joshua