Re: [PATCH 2/4] memcg: remove __mod_lruvec_kmem_state

[Qi Zheng <qi.zheng@linux.dev>]

Hi Shakeel,

On 11/11/25 7:20 AM, Shakeel Butt wrote:
> The __mod_lruvec_kmem_state is already safe against irqs, so there is no
> need to have a separate interface (i.e. mod_lruvec_kmem_state) which
> wraps calls to it with irq disabling and reenabling. Let's rename
> __mod_lruvec_kmem_state to mod_lruvec_kmem_state.
> 
> Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>

Reviewed-by: Qi Zheng <zhengqi.arch@bytedance.com>

One nit below:

> ---
>   include/linux/memcontrol.h | 28 +++++-----------------------
>   mm/memcontrol.c            |  2 +-
>   mm/workingset.c            |  2 +-
>   3 files changed, 7 insertions(+), 25 deletions(-)
> 
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index f82fac2fd988..1384a9d305e1 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -957,17 +957,7 @@ unsigned long lruvec_page_state_local(struct lruvec *lruvec,
>   void mem_cgroup_flush_stats(struct mem_cgroup *memcg);
>   void mem_cgroup_flush_stats_ratelimited(struct mem_cgroup *memcg);
>   
> -void __mod_lruvec_kmem_state(void *p, enum node_stat_item idx, int val);
> -
> -static inline void mod_lruvec_kmem_state(void *p, enum node_stat_item idx,
> -					 int val)
> -{
> -	unsigned long flags;
> -
> -	local_irq_save(flags);
> -	__mod_lruvec_kmem_state(p, idx, val);
> -	local_irq_restore(flags);
> -}
> +void mod_lruvec_kmem_state(void *p, enum node_stat_item idx, int val);
>   
>   void count_memcg_events(struct mem_cgroup *memcg, enum vm_event_item idx,
>   			unsigned long count);
> @@ -1403,14 +1393,6 @@ static inline void mem_cgroup_flush_stats_ratelimited(struct mem_cgroup *memcg)
>   {
>   }
>   
> -static inline void __mod_lruvec_kmem_state(void *p, enum node_stat_item idx,
> -					   int val)
> -{
> -	struct page *page = virt_to_head_page(p);
> -
> -	mod_node_page_state(page_pgdat(page), idx, val);
> -}
> -
>   static inline void mod_lruvec_kmem_state(void *p, enum node_stat_item idx,
>   					 int val)
>   {
> @@ -1470,14 +1452,14 @@ struct slabobj_ext {
>   #endif
>   } __aligned(8);
>   
> -static inline void __inc_lruvec_kmem_state(void *p, enum node_stat_item idx)
> +static inline void inc_lruvec_kmem_state(void *p, enum node_stat_item idx)
>   {
> -	__mod_lruvec_kmem_state(p, idx, 1);
> +	mod_lruvec_kmem_state(p, idx, 1);
>   }

The inc_lruvec_kmem_state() has only one user.

>   
> -static inline void __dec_lruvec_kmem_state(void *p, enum node_stat_item idx)
> +static inline void dec_lruvec_kmem_state(void *p, enum node_stat_item idx)
>   {
> -	__mod_lruvec_kmem_state(p, idx, -1);
> +	mod_lruvec_kmem_state(p, idx, -1);
>   }

The dec_lruvec_kmem_state() has no user.

Not sure whether inc_lruvec_kmem_state() and dec_lruvec_kmem_state()
also need to be removed.

Thanks,
Qi

>   
>   static inline struct lruvec *parent_lruvec(struct lruvec *lruvec)
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index f4b8a6414ed3..3a59d3ee92a7 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -799,7 +799,7 @@ void __lruvec_stat_mod_folio(struct folio *folio, enum node_stat_item idx,
>   }
>   EXPORT_SYMBOL(__lruvec_stat_mod_folio);
>   
> -void __mod_lruvec_kmem_state(void *p, enum node_stat_item idx, int val)
> +void mod_lruvec_kmem_state(void *p, enum node_stat_item idx, int val)
>   {
>   	pg_data_t *pgdat = page_pgdat(virt_to_page(p));
>   	struct mem_cgroup *memcg;
> diff --git a/mm/workingset.c b/mm/workingset.c
> index d32dc2e02a61..892f6fe94ea9 100644
> --- a/mm/workingset.c
> +++ b/mm/workingset.c
> @@ -749,7 +749,7 @@ static enum lru_status shadow_lru_isolate(struct list_head *item,
>   	if (WARN_ON_ONCE(node->count != node->nr_values))
>   		goto out_invalid;
>   	xa_delete_node(node, workingset_update_node);
> -	__inc_lruvec_kmem_state(node, WORKINGSET_NODERECLAIM);
> +	inc_lruvec_kmem_state(node, WORKINGSET_NODERECLAIM);
>   
>   out_invalid:
>   	xa_unlock_irq(&mapping->i_pages);