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

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

On 3/26/26 10:37 PM, David Laight wrote:
> On Thu, 26 Mar 2026 09:19:29 +0000
> "Lorenzo Stoakes (Oracle)" <ljs@kernel.org> wrote:
> 
>> On Wed, Mar 25, 2026 at 10:13:23PM +0800, Qi Zheng wrote:
>>> From: Qi Zheng <zhengqi.arch@bytedance.com>
>>>
>>> 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.
> 
> Won't that are be true on 32bit systems?
> Which means the underlying values need to be u64 not long.

On a 32-bit system, such a large number of values ​​should not accumulate.

Furthermore, changing it to u64/s64 would mean modifying
memcg_vmstats_percpu->state and lruvec_stats_percpu->state,
which involves significant changes, so I think it might be unnecessary.

Thanks,
Qi

> 
> 	David
> 
>>>
>>> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
>>
>> LGTM, so:
>>
>> Reviewed-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org>
>>
>>> ---
>>>   include/trace/events/memcg.h | 10 +++++-----
>>>   mm/memcontrol.c              |  8 ++++----
>>>   2 files changed, 9 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/include/trace/events/memcg.h b/include/trace/events/memcg.h
>>> index dfe2f51019b4c..51b62c5931fc2 100644
>>> --- a/include/trace/events/memcg.h
>>> +++ b/include/trace/events/memcg.h
>>> @@ -11,14 +11,14 @@
>>>
>>>   DECLARE_EVENT_CLASS(memcg_rstat_stats,
>>>
>>> -	TP_PROTO(struct mem_cgroup *memcg, int item, int val),
>>> +	TP_PROTO(struct mem_cgroup *memcg, int item, long val),
>>>
>>>   	TP_ARGS(memcg, item, val),
>>>
>>>   	TP_STRUCT__entry(
>>>   		__field(u64, id)
>>>   		__field(int, item)
>>> -		__field(int, val)
>>> +		__field(long, val)
>>>   	),
>>>
>>>   	TP_fast_assign(
>>> @@ -27,20 +27,20 @@ DECLARE_EVENT_CLASS(memcg_rstat_stats,
>>>   		__entry->val = val;
>>>   	),
>>>
>>> -	TP_printk("memcg_id=%llu item=%d val=%d",
>>> +	TP_printk("memcg_id=%llu item=%d val=%ld",
>>>   		  __entry->id, __entry->item, __entry->val)
>>>   );
>>>
>>>   DEFINE_EVENT(memcg_rstat_stats, mod_memcg_state,
>>>
>>> -	TP_PROTO(struct mem_cgroup *memcg, int item, int val),
>>> +	TP_PROTO(struct mem_cgroup *memcg, int item, long val),
>>>
>>>   	TP_ARGS(memcg, item, val)
>>>   );
>>>
>>>   DEFINE_EVENT(memcg_rstat_stats, mod_memcg_lruvec_state,
>>>
>>> -	TP_PROTO(struct mem_cgroup *memcg, int item, int val),
>>> +	TP_PROTO(struct mem_cgroup *memcg, int item, long val),
>>>
>>>   	TP_ARGS(memcg, item, val)
>>>   );
>>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>>> index 7fb9cbc10dfbb..4a78550f6174e 100644
>>> --- a/mm/memcontrol.c
>>> +++ b/mm/memcontrol.c
>>> @@ -527,7 +527,7 @@ unsigned long lruvec_page_state_local(struct lruvec *lruvec,
>>>
>>>   #ifdef CONFIG_MEMCG_V1
>>>   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);
>>>
>>>   void reparent_memcg_lruvec_state_local(struct mem_cgroup *memcg,
>>>   				       struct mem_cgroup *parent, int idx)
>>> @@ -784,7 +784,7 @@ static int memcg_page_state_unit(int item);
>>>    * Normalize the value passed into memcg_rstat_updated() to be in pages. Round
>>>    * up non-zero sub-page updates to 1 page as zero page updates are ignored.
>>>    */
>>> -static int memcg_state_val_in_pages(int idx, int val)
>>> +static long memcg_state_val_in_pages(int idx, long val)
>>>   {
>>>   	int unit = memcg_page_state_unit(idx);
>>>
>>> @@ -831,7 +831,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 +896,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);
>>> --
>>> 2.20.1
>>>   
>>
>