Re: [PATCH v3 2/3] mm: memcontrol: change val type to long in __mod_memcg_{lruvec_}state()

[Harry Yoo (Oracle)]

On Fri, Mar 27, 2026 at 06:16:29PM +0800, Qi Zheng wrote:
> From: Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx>
> 
> The __mod_memcg_state() and __mod_memcg_lruvec_state() functions are also
> used to reparent non-hierarchical stats. In this scenario, the values
> passed to them are accumulated statistics that might be extremely large
> and exceed the upper limit of a 32-bit integer.
> 
> Change the val parameter type from int to long in these functions and
> their corresponding tracepoints (memcg_rstat_stats) to prevent potential
> overflow issues.
> 
> After that, in memcg_state_val_in_pages(), if the passed val is negative,
> the expression val * unit / PAGE_SIZE could be implicitly converted to a
> massive positive number when compared with 1UL in the max() macro.
> This leads to returning an incorrect massive positive value.
> 
> Fix this by using abs(val) to calculate the magnitude first, and then
> restoring the sign of the value before returning the result. Additionally,
> use mult_frac() to prevent potential overflow during the multiplication of
> val and unit.
> 
> Reported-by: Harry Yoo (Oracle) <harry@xxxxxxxxxx>
> Signed-off-by: Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx>
> Reviewed-by: Lorenzo Stoakes (Oracle) <ljs@xxxxxxxxxx>
> ---

Looks good to me,
Reviewed-by: Harry Yoo (Oracle) <harry@xxxxxxxxxx>

> @@ -831,7 +837,7 @@ static inline void get_non_dying_memcg_end(void)
>  #endif
>  
>  static void __mod_memcg_state(struct mem_cgroup *memcg,
> -			      enum memcg_stat_item idx, int val)
> +			      enum memcg_stat_item idx, long val)
>  {
>  	int i = memcg_stats_index(idx);
>  	int cpu;
> @@ -896,7 +902,7 @@ void reparent_memcg_state_local(struct mem_cgroup *memcg,
>  #endif
>  
>  static void __mod_memcg_lruvec_state(struct mem_cgroup_per_node *pn,
> -				     enum node_stat_item idx, int val)
> +				     enum node_stat_item idx, long val)
>  {
>  	struct mem_cgroup *memcg = pn->memcg;
>  	int i = memcg_stats_index(idx);

Some of code paths that calls mod_memcg{,_lruvec}_state still passes
int values (which is quite subtle to notice), but it should be fine
as reparenting is not involved in the path and could be cleaned up later.

-- 
Cheers,
Harry / Hyeonggon