Re: [PATCH v2 1/4] mm: memcontrol: correct the type of stats_updates to unsigned long

[Lorenzo Stoakes (Oracle)]

On Wed, Mar 25, 2026 at 10:13:22PM +0800, Qi Zheng wrote:
> From: Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx>
>
> The memcg_rstat_updated() tracks updates for vmstats_percpu->state
> and lruvec_stats_percpu->state. Since these state values are of type long,
> change the val parameter passed to memcg_rstat_updated() to long as well.
>
> Correspondingly, change the type of stats_updates in struct
> memcg_vmstats_percpu and struct memcg_vmstats from unsigned int and
> atomic_t to unsigned long and atomic_long_t respectively to prevent
> potential overflow when handling large state updates during the
> reparenting of LRU folios.

Do we need a Fixes, possibly cc: stable for that? Apologies if already
asked + answered.

>
> Signed-off-by: Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx>

Anyway logic seems fine to me, so:

Reviewed-by: Lorenzo Stoakes (Oracle) <ljs@xxxxxxxxxx>

> ---
>  mm/memcontrol.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index a47fb68dd65f1..7fb9cbc10dfbb 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -608,7 +608,7 @@ static inline int memcg_events_index(enum vm_event_item idx)
>
>  struct memcg_vmstats_percpu {
>  	/* Stats updates since the last flush */
> -	unsigned int			stats_updates;
> +	unsigned long			stats_updates;
>
>  	/* Cached pointers for fast iteration in memcg_rstat_updated() */
>  	struct memcg_vmstats_percpu __percpu	*parent_pcpu;
> @@ -639,7 +639,7 @@ struct memcg_vmstats {
>  	unsigned long		events_pending[NR_MEMCG_EVENTS];
>
>  	/* Stats updates since the last flush */
> -	atomic_t		stats_updates;
> +	atomic_long_t		stats_updates;
>  };
>
>  /*
> @@ -665,16 +665,16 @@ static u64 flush_last_time;
>
>  static bool memcg_vmstats_needs_flush(struct memcg_vmstats *vmstats)
>  {
> -	return atomic_read(&vmstats->stats_updates) >
> +	return atomic_long_read(&vmstats->stats_updates) >
>  		MEMCG_CHARGE_BATCH * num_online_cpus();
>  }
>
> -static inline void memcg_rstat_updated(struct mem_cgroup *memcg, int val,
> +static inline void memcg_rstat_updated(struct mem_cgroup *memcg, long val,
>  				       int cpu)
>  {
>  	struct memcg_vmstats_percpu __percpu *statc_pcpu;
>  	struct memcg_vmstats_percpu *statc;
> -	unsigned int stats_updates;
> +	unsigned long stats_updates;
>
>  	if (!val)
>  		return;
> @@ -697,7 +697,7 @@ static inline void memcg_rstat_updated(struct mem_cgroup *memcg, int val,
>  			continue;
>
>  		stats_updates = this_cpu_xchg(statc_pcpu->stats_updates, 0);
> -		atomic_add(stats_updates, &statc->vmstats->stats_updates);
> +		atomic_long_add(stats_updates, &statc->vmstats->stats_updates);
>  	}
>  }
>
> @@ -705,7 +705,7 @@ static void __mem_cgroup_flush_stats(struct mem_cgroup *memcg, bool force)
>  {
>  	bool needs_flush = memcg_vmstats_needs_flush(memcg->vmstats);
>
> -	trace_memcg_flush_stats(memcg, atomic_read(&memcg->vmstats->stats_updates),
> +	trace_memcg_flush_stats(memcg, atomic_long_read(&memcg->vmstats->stats_updates),
>  		force, needs_flush);
>
>  	if (!force && !needs_flush)
> @@ -4406,8 +4406,8 @@ static void mem_cgroup_css_rstat_flush(struct cgroup_subsys_state *css, int cpu)
>  	}
>  	WRITE_ONCE(statc->stats_updates, 0);
>  	/* We are in a per-cpu loop here, only do the atomic write once */
> -	if (atomic_read(&memcg->vmstats->stats_updates))
> -		atomic_set(&memcg->vmstats->stats_updates, 0);
> +	if (atomic_long_read(&memcg->vmstats->stats_updates))
> +		atomic_long_set(&memcg->vmstats->stats_updates, 0);
>  }
>
>  static void mem_cgroup_fork(struct task_struct *task)
> --
> 2.20.1
>