[PATCH 0/3] correct the parameter type of some mm functions

[PATCH 0/3] correct the parameter type of some mm functions

[Qi Zheng]

From: Qi Zheng <zhengqi.arch@bytedance.com>

Hi all,

As Harry Yoo pointed out [1], in the case of reparenting LRU folios, the value
passed to functions such as __mod_memcg{_lruvec}_state() may exceed the upper
limit of int, so this series aims to correct the parameter type of these
functions.

This series is based on the next-20260323.

Comments and suggestions are welcome!

Thanks,
Qi

[1]. https://lore.kernel.org/all/acDxaEgnqPI-Z4be@hyeyoo/

Qi Zheng (3):
  mm: memcontrol: correct the type of stats_updates to unsigned long
  mm: memcontrol: correct the parameter type of
    __mod_memcg{_lruvec}_state()
  mm: memcontrol: correct the nr_pages parameter type of
    mem_cgroup_update_lru_size()

 include/linux/memcontrol.h   |  2 +-
 include/trace/events/memcg.h | 10 +++++-----
 mm/memcontrol.c              | 28 ++++++++++++++--------------
 3 files changed, 20 insertions(+), 20 deletions(-)

-- 
2.20.1

[PATCH] fix: mm: memcontrol: convert objcg to be per-memcg per-node type

[Qi Zheng]

From: Qi Zheng <zhengqi.arch@bytedance.com>

Reset pn->orig_objcg to NULL to prevent obj_cgroup_put()
from being called agagin in __mem_cgroup_free().

Reported-by: Usama Arif <usama.arif@linux.dev>
Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
---
 mm/memcontrol.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 992a3f5caa62b..ad32639ea5959 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4140,8 +4140,14 @@ static int mem_cgroup_css_online(struct cgroup_subsys_state *css)
 	for_each_node(nid) {
 		struct mem_cgroup_per_node *pn = memcg->nodeinfo[nid];
 
-		if (pn && pn->orig_objcg)
+		if (pn && pn->orig_objcg) {
 			obj_cgroup_put(pn->orig_objcg);
+			/*
+			 * Reset pn->orig_objcg to NULL to prevent obj_cgroup_put()
+			 * from being called agagin in __mem_cgroup_free().
+			 */
+			pn->orig_objcg = NULL;
+		}
 	}
 	free_shrinker_info(memcg);
 offline_kmem:
-- 
2.20.1

Re: [PATCH] fix: mm: memcontrol: convert objcg to be per-memcg per-node type

[Qi Zheng]

Oops, Please ignore this patch.

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

[Qi Zheng]

From: Qi Zheng <zhengqi.arch@bytedance.com>

Now, the memcg_rstat_updated() is for vmstats_percpu->state and
lruvec_stats_percpu->state, which are both of type long, so let's change
the type of stats_updates to unsigned long as well.

Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
---
 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

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

[Lorenzo Stoakes (Oracle)]

(-cc old mail)

On Tue, Mar 24, 2026 at 07:31:27PM +0800, Qi Zheng wrote:
> From: Qi Zheng <zhengqi.arch@bytedance.com>
>
> Now, the memcg_rstat_updated() is for vmstats_percpu->state and

Now? Did this change? If so, please put commit here in commit xxx ("blah")
format, I don't want to have to try and look it up :P

> lruvec_stats_percpu->state, which are both of type long, so let's change
> the type of stats_updates to unsigned long as well.

Is this a bug whereby before this could be overflowed?

Or are you deciding to make these long now? Because it seems that are
proactively making this change _yourself_.

This comment message needs a lot more explanation.

>
> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
> ---
>  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
>

[PATCH 2/3] mm: memcontrol: correct the parameter type of __mod_memcg{_lruvec}_state()

[Qi Zheng]

From: Qi Zheng <zhengqi.arch@bytedance.com>

The __mod_memcg_state() and __mod_memcg_lruvec_state() were used to
reparent non-hierarchical stats, the values passed to them might exceed
the upper limit of the type int, so correct the val parameter type of them
to long.

Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
---
 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

Re: [PATCH 2/3] mm: memcontrol: correct the parameter type of __mod_memcg{_lruvec}_state()

[Lorenzo Stoakes (Oracle)]

On Tue, Mar 24, 2026 at 07:31:28PM +0800, Qi Zheng wrote:
> From: Qi Zheng <zhengqi.arch@bytedance.com>
>
> The __mod_memcg_state() and __mod_memcg_lruvec_state() were used to
> reparent non-hierarchical stats, the values passed to them might exceed
> the upper limit of the type int, so correct the val parameter type of them

Why might they? What precipitated this change?

> to long.

Why is it a signed value?

>
> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
> ---
>  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
>

Cheers, Lorenzo

Re: [PATCH 2/3] mm: memcontrol: correct the parameter type of __mod_memcg{_lruvec}_state()

[Matthew Wilcox]

On Tue, Mar 24, 2026 at 12:21:06PM +0000, Lorenzo Stoakes (Oracle) wrote:
> On Tue, Mar 24, 2026 at 07:31:28PM +0800, Qi Zheng wrote:
> > From: Qi Zheng <zhengqi.arch@bytedance.com>
> >
> > The __mod_memcg_state() and __mod_memcg_lruvec_state() were used to
> > reparent non-hierarchical stats, the values passed to them might exceed
> > the upper limit of the type int, so correct the val parameter type of them
> 
> Why might they? What precipitated this change?
> 
> > to long.
> 
> Why is it a signed value?

Because 'mod' can both increase and decrease this counter.  It must be
signed.

Re: [PATCH 2/3] mm: memcontrol: correct the parameter type of __mod_memcg{_lruvec}_state()

[Lorenzo Stoakes (Oracle)]

On Tue, Mar 24, 2026 at 02:12:00PM +0000, Matthew Wilcox wrote:
> On Tue, Mar 24, 2026 at 12:21:06PM +0000, Lorenzo Stoakes (Oracle) wrote:
> > On Tue, Mar 24, 2026 at 07:31:28PM +0800, Qi Zheng wrote:
> > > From: Qi Zheng <zhengqi.arch@bytedance.com>
> > >
> > > The __mod_memcg_state() and __mod_memcg_lruvec_state() were used to
> > > reparent non-hierarchical stats, the values passed to them might exceed
> > > the upper limit of the type int, so correct the val parameter type of them
> >
> > Why might they? What precipitated this change?
> >
> > > to long.
> >
> > Why is it a signed value?
>
> Because 'mod' can both increase and decrease this counter.  It must be
> signed.

Right yeah noticed that in another patch :)

Cheers, Lorenzo

Re: [PATCH 2/3] mm: memcontrol: correct the parameter type of __mod_memcg{_lruvec}_state()

[Harry Yoo (Oracle)]

On Tue, Mar 24, 2026 at 07:31:28PM +0800, Qi Zheng wrote:
> From: Qi Zheng <zhengqi.arch@bytedance.com>
> 
> The __mod_memcg_state() and __mod_memcg_lruvec_state() were used to
> reparent non-hierarchical stats, the values passed to them might exceed
> the upper limit of the type int, so correct the val parameter type of them
> to long.
> 
> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
> ---
>  include/trace/events/memcg.h | 10 +++++-----
>  mm/memcontrol.c              |  8 ++++----
>  2 files changed, 9 insertions(+), 9 deletions(-)
> 
> 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);

Sashiko AI made an interesting argument [1] that this could lead to
incorrectly returning a very large positive number. Let me verify that.

[1] https://sashiko.dev/#/patchset/cover.1774342371.git.zhengqi.arch%40bytedance.com

Sashiko wrote:
> Does this change inadvertently break the handling of negative byte-sized
> updates?
> Looking at the rest of the function:
> 	if (!val || unit == PAGE_SIZE)
> 		return val;
> 	else
> 		return max(val * unit / PAGE_SIZE, 1UL);

> PAGE_SIZE is defined as an unsigned long.

Right, it's defined as 1UL << PAGE_SHIFT.

> When val is negative, such as during uncharging of byte-sized stats like
> MEMCG_ZSWAP_B, the expression val * unit is a negative long.

Right.

> Dividing a signed long by an unsigned long causes the signed long to be
> promoted to unsigned before division,

Right.

> resulting in a massive positive
> number instead of a small negative one.

Let's look at an example (assuming unit is 1).

val = val * unit = -16384 (-16 KiB)
val * unit / PAGE_SIZE = 0xFFFFFFFFFFFFC000 / PAGE_SIZE = 0x3FFFFFFFFFFFFF
max(0x3FFFFFFFFFFFFF, 1UL) = 0x3FFFFFFFFFF

Yeah, that's a massive positive number.

Hmm but how did it work when it was int?

val = val * unit = -16384 (-16KiB)
val * unit / PAGE_SIZE = 0xFFFFFFFFFFFFC000 / PAGE_SIZE = 0x3FFFFFFFFFFFFF
max(val * unit / PAGE_SIZE, 1UL) = 0x3FFFFFFFFFFFFF
(int)0x3FFFFFFFFFFFFF = 0xFFFFFFFF = (-1)

That's incorrect. It should have been -4?

> Before this change, the function returned an int, which implicitly truncated
> the massive unsigned 64-bit result to a 32-bit int, accidentally yielding the
> correct negative arithmetic value.

So... "accidentally yielding the correct negative arithemetic value"
is wrong.

Sounds like it's been subtly broken even before this patch and nobody
noticed.

> By changing the return type to long, this implicit truncation is removed,
> and the huge positive value is returned unaltered.

That's true.

> Could this corrupt tracepoint logs when passed to trace_mod_memcg_state?

I'm not sure if that's critical but yeah that's true.

> Also, would passing this huge positive value to memcg_rstat_updated instantly
> exceed the charge batch threshold and trigger endless, expensive global
> cgroup_rstat flushing, severely degrading system performance?

It would lead to more frequent flushes, at least.

-- 
Cheers,
Harry / Hyeonggon

Re: [PATCH 2/3] mm: memcontrol: correct the parameter type of __mod_memcg{_lruvec}_state()

[Qi Zheng]

On 3/25/26 9:43 AM, Harry Yoo (Oracle) wrote:
> On Tue, Mar 24, 2026 at 07:31:28PM +0800, Qi Zheng wrote:
>> From: Qi Zheng <zhengqi.arch@bytedance.com>
>>
>> The __mod_memcg_state() and __mod_memcg_lruvec_state() were used to
>> reparent non-hierarchical stats, the values passed to them might exceed
>> the upper limit of the type int, so correct the val parameter type of them
>> to long.
>>
>> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
>> ---
>>   include/trace/events/memcg.h | 10 +++++-----
>>   mm/memcontrol.c              |  8 ++++----
>>   2 files changed, 9 insertions(+), 9 deletions(-)
>>
>> 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);
> 
> Sashiko AI made an interesting argument [1] that this could lead to
> incorrectly returning a very large positive number. Let me verify that.
> 
> [1] https://sashiko.dev/#/patchset/cover.1774342371.git.zhengqi.arch%40bytedance.com
> 
> Sashiko wrote:
>> Does this change inadvertently break the handling of negative byte-sized
>> updates?
>> Looking at the rest of the function:
>> 	if (!val || unit == PAGE_SIZE)
>> 		return val;
>> 	else
>> 		return max(val * unit / PAGE_SIZE, 1UL);
> 
>> PAGE_SIZE is defined as an unsigned long.
> 
> Right, it's defined as 1UL << PAGE_SHIFT.
> 
>> When val is negative, such as during uncharging of byte-sized stats like
>> MEMCG_ZSWAP_B, the expression val * unit is a negative long.
> 
> Right.
> 
>> Dividing a signed long by an unsigned long causes the signed long to be
>> promoted to unsigned before division,
> 
> Right.
> 
>> resulting in a massive positive
>> number instead of a small negative one.
> 
> Let's look at an example (assuming unit is 1).
> 
> val = val * unit = -16384 (-16 KiB)
> val * unit / PAGE_SIZE = 0xFFFFFFFFFFFFC000 / PAGE_SIZE = 0x3FFFFFFFFFFFFF
> max(0x3FFFFFFFFFFFFF, 1UL) = 0x3FFFFFFFFFF
> 
> Yeah, that's a massive positive number.
> 
> Hmm but how did it work when it was int?
> 
> val = val * unit = -16384 (-16KiB)
> val * unit / PAGE_SIZE = 0xFFFFFFFFFFFFC000 / PAGE_SIZE = 0x3FFFFFFFFFFFFF
> max(val * unit / PAGE_SIZE, 1UL) = 0x3FFFFFFFFFFFFF
> (int)0x3FFFFFFFFFFFFF = 0xFFFFFFFF = (-1)
> 
> That's incorrect. It should have been -4?
> 
>> Before this change, the function returned an int, which implicitly truncated
>> the massive unsigned 64-bit result to a 32-bit int, accidentally yielding the
>> correct negative arithmetic value.
> 
> So... "accidentally yielding the correct negative arithemetic value"
> is wrong.
> 
> Sounds like it's been subtly broken even before this patch and nobody
> noticed.

Thank you for such a detailed analysis! And I think you are right.

The memcg_state_val_in_pages() is only to make @val to be in pages, so
perhaps we can avoid the above problem by taking the absolute value
first?

Thanks,
Qi

> 
>> By changing the return type to long, this implicit truncation is removed,
>> and the huge positive value is returned unaltered.
> 
> That's true.
> 
>> Could this corrupt tracepoint logs when passed to trace_mod_memcg_state?
> 
> I'm not sure if that's critical but yeah that's true.
> 
>> Also, would passing this huge positive value to memcg_rstat_updated instantly
>> exceed the charge batch threshold and trigger endless, expensive global
>> cgroup_rstat flushing, severely degrading system performance?
> 
> It would lead to more frequent flushes, at least.
> 

Re: [PATCH 2/3] mm: memcontrol: correct the parameter type of __mod_memcg{_lruvec}_state()

[Harry Yoo (Oracle)]

On Wed, Mar 25, 2026 at 11:25:06AM +0800, Qi Zheng wrote:
> On 3/25/26 9:43 AM, Harry Yoo (Oracle) wrote:
> > On Tue, Mar 24, 2026 at 07:31:28PM +0800, Qi Zheng wrote:
> > > From: Qi Zheng <zhengqi.arch@bytedance.com>
> > > 
> > > The __mod_memcg_state() and __mod_memcg_lruvec_state() were used to
> > > reparent non-hierarchical stats, the values passed to them might exceed
> > > the upper limit of the type int, so correct the val parameter type of them
> > > to long.
> > > 
> > > Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
> > > ---
> > >   include/trace/events/memcg.h | 10 +++++-----
> > >   mm/memcontrol.c              |  8 ++++----
> > >   2 files changed, 9 insertions(+), 9 deletions(-)
> > > 
> > > 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);
> > 
> > Sashiko AI made an interesting argument [1] that this could lead to
> > incorrectly returning a very large positive number. Let me verify that.
> > 
> > [1] https://sashiko.dev/#/patchset/cover.1774342371.git.zhengqi.arch%40bytedance.com
> > 
> > Sashiko wrote:
> > > Does this change inadvertently break the handling of negative byte-sized
> > > updates?
> > > Looking at the rest of the function:
> > > 	if (!val || unit == PAGE_SIZE)
> > > 		return val;
> > > 	else
> > > 		return max(val * unit / PAGE_SIZE, 1UL);
> > 
> > > PAGE_SIZE is defined as an unsigned long.
> > 
> > Right, it's defined as 1UL << PAGE_SHIFT.
> > 
> > > When val is negative, such as during uncharging of byte-sized stats like
> > > MEMCG_ZSWAP_B, the expression val * unit is a negative long.
> > 
> > Right.
> > 
> > > Dividing a signed long by an unsigned long causes the signed long to be
> > > promoted to unsigned before division,
> > 
> > Right.
> > 
> > > resulting in a massive positive
> > > number instead of a small negative one.
> > 
> > Let's look at an example (assuming unit is 1).
> > 
> > val = val * unit = -16384 (-16 KiB)
> > val * unit / PAGE_SIZE = 0xFFFFFFFFFFFFC000 / PAGE_SIZE = 0x3FFFFFFFFFFFFF
> > max(0x3FFFFFFFFFFFFF, 1UL) = 0x3FFFFFFFFFF
> > 
> > Yeah, that's a massive positive number.
> > 
> > Hmm but how did it work when it was int?
> > 
> > val = val * unit = -16384 (-16KiB)
> > val * unit / PAGE_SIZE = 0xFFFFFFFFFFFFC000 / PAGE_SIZE = 0x3FFFFFFFFFFFFF
> > max(val * unit / PAGE_SIZE, 1UL) = 0x3FFFFFFFFFFFFF
> > (int)0x3FFFFFFFFFFFFF = 0xFFFFFFFF = (-1)
> > 
> > That's incorrect. It should have been -4?
> > 
> > > Before this change, the function returned an int, which implicitly truncated
> > > the massive unsigned 64-bit result to a 32-bit int, accidentally yielding the
> > > correct negative arithmetic value.
> > 
> > So... "accidentally yielding the correct negative arithemetic value"
> > is wrong.
> > 
> > Sounds like it's been subtly broken even before this patch and nobody
> > noticed.
> 
> Thank you for such a detailed analysis! And I think you are right.

No problem ;)

> The memcg_state_val_in_pages() is only to make @val to be in pages, so
> perhaps we can avoid the above problem by taking the absolute value
> first?

That would work for memcg_rstat_updated(),
but not for trace_mod_memcg_state()?

-- 
Cheers,
Harry / Hyeonggon

Re: [PATCH 2/3] mm: memcontrol: correct the parameter type of __mod_memcg{_lruvec}_state()

[Qi Zheng]

On 3/25/26 1:17 PM, Harry Yoo (Oracle) wrote:
> On Wed, Mar 25, 2026 at 11:25:06AM +0800, Qi Zheng wrote:
>> On 3/25/26 9:43 AM, Harry Yoo (Oracle) wrote:
>>> On Tue, Mar 24, 2026 at 07:31:28PM +0800, Qi Zheng wrote:
>>>> From: Qi Zheng <zhengqi.arch@bytedance.com>
>>>>
>>>> The __mod_memcg_state() and __mod_memcg_lruvec_state() were used to
>>>> reparent non-hierarchical stats, the values passed to them might exceed
>>>> the upper limit of the type int, so correct the val parameter type of them
>>>> to long.
>>>>
>>>> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
>>>> ---
>>>>    include/trace/events/memcg.h | 10 +++++-----
>>>>    mm/memcontrol.c              |  8 ++++----
>>>>    2 files changed, 9 insertions(+), 9 deletions(-)
>>>>
>>>> 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);
>>>
>>> Sashiko AI made an interesting argument [1] that this could lead to
>>> incorrectly returning a very large positive number. Let me verify that.
>>>
>>> [1] https://sashiko.dev/#/patchset/cover.1774342371.git.zhengqi.arch%40bytedance.com
>>>
>>> Sashiko wrote:
>>>> Does this change inadvertently break the handling of negative byte-sized
>>>> updates?
>>>> Looking at the rest of the function:
>>>> 	if (!val || unit == PAGE_SIZE)
>>>> 		return val;
>>>> 	else
>>>> 		return max(val * unit / PAGE_SIZE, 1UL);
>>>
>>>> PAGE_SIZE is defined as an unsigned long.
>>>
>>> Right, it's defined as 1UL << PAGE_SHIFT.
>>>
>>>> When val is negative, such as during uncharging of byte-sized stats like
>>>> MEMCG_ZSWAP_B, the expression val * unit is a negative long.
>>>
>>> Right.
>>>
>>>> Dividing a signed long by an unsigned long causes the signed long to be
>>>> promoted to unsigned before division,
>>>
>>> Right.
>>>
>>>> resulting in a massive positive
>>>> number instead of a small negative one.
>>>
>>> Let's look at an example (assuming unit is 1).
>>>
>>> val = val * unit = -16384 (-16 KiB)
>>> val * unit / PAGE_SIZE = 0xFFFFFFFFFFFFC000 / PAGE_SIZE = 0x3FFFFFFFFFFFFF
>>> max(0x3FFFFFFFFFFFFF, 1UL) = 0x3FFFFFFFFFF
>>>
>>> Yeah, that's a massive positive number.
>>>
>>> Hmm but how did it work when it was int?
>>>
>>> val = val * unit = -16384 (-16KiB)
>>> val * unit / PAGE_SIZE = 0xFFFFFFFFFFFFC000 / PAGE_SIZE = 0x3FFFFFFFFFFFFF
>>> max(val * unit / PAGE_SIZE, 1UL) = 0x3FFFFFFFFFFFFF
>>> (int)0x3FFFFFFFFFFFFF = 0xFFFFFFFF = (-1)
>>>
>>> That's incorrect. It should have been -4?
>>>
>>>> Before this change, the function returned an int, which implicitly truncated
>>>> the massive unsigned 64-bit result to a 32-bit int, accidentally yielding the
>>>> correct negative arithmetic value.
>>>
>>> So... "accidentally yielding the correct negative arithemetic value"
>>> is wrong.
>>>
>>> Sounds like it's been subtly broken even before this patch and nobody
>>> noticed.
>>
>> Thank you for such a detailed analysis! And I think you are right.
> 
> No problem ;)
> 
>> The memcg_state_val_in_pages() is only to make @val to be in pages, so
>> perhaps we can avoid the above problem by taking the absolute value
>> first?

For more clearly, here is the diff:

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 6491ca74b3398..2b34805b01476 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -787,11 +787,17 @@ static int memcg_page_state_unit(int item);
  static long memcg_state_val_in_pages(int idx, long val)
  {
         int unit = memcg_page_state_unit(idx);
+       unsigned long uval;
+       long res;

         if (!val || unit == PAGE_SIZE)
                 return val;
-       else
-               return max(val * unit / PAGE_SIZE, 1UL);
+
+       uval = val < 0 ? -(unsigned long)val : (unsigned long)val;
+
+       res = max(mult_frac(uval, unit, PAGE_SIZE), 1UL);
+
+       return val < 0 ? -res : res;
  }

  #ifdef CONFIG_MEMCG_V1

> 
> That would work for memcg_rstat_updated(),
> but not for trace_mod_memcg_state()?

Why? The trace_mod_memcg_state() seems to accept 'long' type,
am I missing something?

Thanks,
Qi


> 

Re: [PATCH 2/3] mm: memcontrol: correct the parameter type of __mod_memcg{_lruvec}_state()

[Harry Yoo (Oracle)]

On Wed, Mar 25, 2026 at 03:26:46PM +0800, Qi Zheng wrote:
> 
> 
> On 3/25/26 1:17 PM, Harry Yoo (Oracle) wrote:
> > On Wed, Mar 25, 2026 at 11:25:06AM +0800, Qi Zheng wrote:
> > > On 3/25/26 9:43 AM, Harry Yoo (Oracle) wrote:
> > > > On Tue, Mar 24, 2026 at 07:31:28PM +0800, Qi Zheng wrote:
> > > > > From: Qi Zheng <zhengqi.arch@bytedance.com>
> > > > > 
> > > > > The __mod_memcg_state() and __mod_memcg_lruvec_state() were used to
> > > > > reparent non-hierarchical stats, the values passed to them might exceed
> > > > > the upper limit of the type int, so correct the val parameter type of them
> > > > > to long.
> > > > > 
> > > > > Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
> > > > > ---
> > > > >    include/trace/events/memcg.h | 10 +++++-----
> > > > >    mm/memcontrol.c              |  8 ++++----
> > > > >    2 files changed, 9 insertions(+), 9 deletions(-)
> > > > > 
> > > > > 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);
> > > > 
> > > > Sashiko AI made an interesting argument [1] that this could lead to
> > > > incorrectly returning a very large positive number. Let me verify that.
> > > > 
> > > > [1] https://sashiko.dev/#/patchset/cover.1774342371.git.zhengqi.arch%40bytedance.com
> > > > 
> > > > Sashiko wrote:
> > > > > Does this change inadvertently break the handling of negative byte-sized
> > > > > updates?
> > > > > Looking at the rest of the function:
> > > > > 	if (!val || unit == PAGE_SIZE)
> > > > > 		return val;
> > > > > 	else
> > > > > 		return max(val * unit / PAGE_SIZE, 1UL);
> > > > 
> > > > > PAGE_SIZE is defined as an unsigned long.
> > > > 
> > > > Right, it's defined as 1UL << PAGE_SHIFT.
> > > > 
> > > > > When val is negative, such as during uncharging of byte-sized stats like
> > > > > MEMCG_ZSWAP_B, the expression val * unit is a negative long.
> > > > 
> > > > Right.
> > > > 
> > > > > Dividing a signed long by an unsigned long causes the signed long to be
> > > > > promoted to unsigned before division,
> > > > 
> > > > Right.
> > > > 
> > > > > resulting in a massive positive
> > > > > number instead of a small negative one.
> > > > 
> > > > Let's look at an example (assuming unit is 1).
> > > > 
> > > > val = val * unit = -16384 (-16 KiB)
> > > > val * unit / PAGE_SIZE = 0xFFFFFFFFFFFFC000 / PAGE_SIZE = 0x3FFFFFFFFFFFFF
> > > > max(0x3FFFFFFFFFFFFF, 1UL) = 0x3FFFFFFFFFF
> > > > 
> > > > Yeah, that's a massive positive number.
> > > > 
> > > > Hmm but how did it work when it was int?
> > > > 
> > > > val = val * unit = -16384 (-16KiB)
> > > > val * unit / PAGE_SIZE = 0xFFFFFFFFFFFFC000 / PAGE_SIZE = 0x3FFFFFFFFFFFFF
> > > > max(val * unit / PAGE_SIZE, 1UL) = 0x3FFFFFFFFFFFFF
> > > > (int)0x3FFFFFFFFFFFFF = 0xFFFFFFFF = (-1)
> > > > 
> > > > That's incorrect. It should have been -4?
> > > > 
> > > > > Before this change, the function returned an int, which implicitly truncated
> > > > > the massive unsigned 64-bit result to a 32-bit int, accidentally yielding the
> > > > > correct negative arithmetic value.
> > > > 
> > > > So... "accidentally yielding the correct negative arithemetic value"
> > > > is wrong.
> > > > 
> > > > Sounds like it's been subtly broken even before this patch and nobody
> > > > noticed.
> > > 
> > > Thank you for such a detailed analysis! And I think you are right.
> > 
> > No problem ;)
> > 
> > > The memcg_state_val_in_pages() is only to make @val to be in pages, so
> > > perhaps we can avoid the above problem by taking the absolute value
> > > first?
> 
> For more clearly, here is the diff:
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 6491ca74b3398..2b34805b01476 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -787,11 +787,17 @@ static int memcg_page_state_unit(int item);
>  static long memcg_state_val_in_pages(int idx, long val)
>  {
>         int unit = memcg_page_state_unit(idx);
> +       unsigned long uval;
> +       long res;
> 
>         if (!val || unit == PAGE_SIZE)
>                 return val;
> -       else
> -               return max(val * unit / PAGE_SIZE, 1UL);
> +
> +       uval = val < 0 ? -(unsigned long)val : (unsigned long)val;

We could use abs() macro in linux/math.h here :)

val should not be LONG_MIN, but it should be fine
to assume it won't reach LONG_MIN.

> +
> +       res = max(mult_frac(uval, unit, PAGE_SIZE), 1UL);
> +
> +       return val < 0 ? -res : res;
>  }
> 
>  #ifdef CONFIG_MEMCG_V1
> 
> > That would work for memcg_rstat_updated(),
> > but not for trace_mod_memcg_state()?
> 
> Why? The trace_mod_memcg_state() seems to accept 'long' type,
> am I missing something?

Nevermind. Without the diff, I misunderstood what you meant.

Thanks!

-- 
Cheers,
Harry / Hyeonggon

Re: [PATCH 2/3] mm: memcontrol: correct the parameter type of __mod_memcg{_lruvec}_state()

[Qi Zheng]

On 3/25/26 3:36 PM, Harry Yoo (Oracle) wrote:
> On Wed, Mar 25, 2026 at 03:26:46PM +0800, Qi Zheng wrote:
>>
>>
>> On 3/25/26 1:17 PM, Harry Yoo (Oracle) wrote:
>>> On Wed, Mar 25, 2026 at 11:25:06AM +0800, Qi Zheng wrote:
>>>> On 3/25/26 9:43 AM, Harry Yoo (Oracle) wrote:
>>>>> On Tue, Mar 24, 2026 at 07:31:28PM +0800, Qi Zheng wrote:
>>>>>> From: Qi Zheng <zhengqi.arch@bytedance.com>
>>>>>>
>>>>>> The __mod_memcg_state() and __mod_memcg_lruvec_state() were used to
>>>>>> reparent non-hierarchical stats, the values passed to them might exceed
>>>>>> the upper limit of the type int, so correct the val parameter type of them
>>>>>> to long.
>>>>>>
>>>>>> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
>>>>>> ---
>>>>>>     include/trace/events/memcg.h | 10 +++++-----
>>>>>>     mm/memcontrol.c              |  8 ++++----
>>>>>>     2 files changed, 9 insertions(+), 9 deletions(-)
>>>>>>
>>>>>> 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);
>>>>>
>>>>> Sashiko AI made an interesting argument [1] that this could lead to
>>>>> incorrectly returning a very large positive number. Let me verify that.
>>>>>
>>>>> [1] https://sashiko.dev/#/patchset/cover.1774342371.git.zhengqi.arch%40bytedance.com
>>>>>
>>>>> Sashiko wrote:
>>>>>> Does this change inadvertently break the handling of negative byte-sized
>>>>>> updates?
>>>>>> Looking at the rest of the function:
>>>>>> 	if (!val || unit == PAGE_SIZE)
>>>>>> 		return val;
>>>>>> 	else
>>>>>> 		return max(val * unit / PAGE_SIZE, 1UL);
>>>>>
>>>>>> PAGE_SIZE is defined as an unsigned long.
>>>>>
>>>>> Right, it's defined as 1UL << PAGE_SHIFT.
>>>>>
>>>>>> When val is negative, such as during uncharging of byte-sized stats like
>>>>>> MEMCG_ZSWAP_B, the expression val * unit is a negative long.
>>>>>
>>>>> Right.
>>>>>
>>>>>> Dividing a signed long by an unsigned long causes the signed long to be
>>>>>> promoted to unsigned before division,
>>>>>
>>>>> Right.
>>>>>
>>>>>> resulting in a massive positive
>>>>>> number instead of a small negative one.
>>>>>
>>>>> Let's look at an example (assuming unit is 1).
>>>>>
>>>>> val = val * unit = -16384 (-16 KiB)
>>>>> val * unit / PAGE_SIZE = 0xFFFFFFFFFFFFC000 / PAGE_SIZE = 0x3FFFFFFFFFFFFF
>>>>> max(0x3FFFFFFFFFFFFF, 1UL) = 0x3FFFFFFFFFF
>>>>>
>>>>> Yeah, that's a massive positive number.
>>>>>
>>>>> Hmm but how did it work when it was int?
>>>>>
>>>>> val = val * unit = -16384 (-16KiB)
>>>>> val * unit / PAGE_SIZE = 0xFFFFFFFFFFFFC000 / PAGE_SIZE = 0x3FFFFFFFFFFFFF
>>>>> max(val * unit / PAGE_SIZE, 1UL) = 0x3FFFFFFFFFFFFF
>>>>> (int)0x3FFFFFFFFFFFFF = 0xFFFFFFFF = (-1)
>>>>>
>>>>> That's incorrect. It should have been -4?
>>>>>
>>>>>> Before this change, the function returned an int, which implicitly truncated
>>>>>> the massive unsigned 64-bit result to a 32-bit int, accidentally yielding the
>>>>>> correct negative arithmetic value.
>>>>>
>>>>> So... "accidentally yielding the correct negative arithemetic value"
>>>>> is wrong.
>>>>>
>>>>> Sounds like it's been subtly broken even before this patch and nobody
>>>>> noticed.
>>>>
>>>> Thank you for such a detailed analysis! And I think you are right.
>>>
>>> No problem ;)
>>>
>>>> The memcg_state_val_in_pages() is only to make @val to be in pages, so
>>>> perhaps we can avoid the above problem by taking the absolute value
>>>> first?
>>
>> For more clearly, here is the diff:
>>
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index 6491ca74b3398..2b34805b01476 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -787,11 +787,17 @@ static int memcg_page_state_unit(int item);
>>   static long memcg_state_val_in_pages(int idx, long val)
>>   {
>>          int unit = memcg_page_state_unit(idx);
>> +       unsigned long uval;
>> +       long res;
>>
>>          if (!val || unit == PAGE_SIZE)
>>                  return val;
>> -       else
>> -               return max(val * unit / PAGE_SIZE, 1UL);
>> +
>> +       uval = val < 0 ? -(unsigned long)val : (unsigned long)val;
> 
> We could use abs() macro in linux/math.h here :)
> 
> val should not be LONG_MIN, but it should be fine
> to assume it won't reach LONG_MIN.

OK, will use abs() macro.

> 
>> +
>> +       res = max(mult_frac(uval, unit, PAGE_SIZE), 1UL);
>> +
>> +       return val < 0 ? -res : res;
>>   }
>>
>>   #ifdef CONFIG_MEMCG_V1
>>
>>> That would work for memcg_rstat_updated(),
>>> but not for trace_mod_memcg_state()?
>>
>> Why? The trace_mod_memcg_state() seems to accept 'long' type,
>> am I missing something?
> 
> Nevermind. Without the diff, I misunderstood what you meant.

OK, will apply this diff and send the v2.

Thanks,
Qi

> 
> Thanks!
> 

Re: [PATCH 2/3] mm: memcontrol: correct the parameter type of __mod_memcg{_lruvec}_state()

[Harry Yoo (Oracle)]

On Wed, Mar 25, 2026 at 10:43:58AM +0900, Harry Yoo (Oracle) wrote:
> On Tue, Mar 24, 2026 at 07:31:28PM +0800, Qi Zheng wrote:
> > @@ -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);
> 
> Sashiko AI made an interesting argument [1] that this could lead to
> incorrectly returning a very large positive number. Let me verify that.
> 
> [1] https://sashiko.dev/#/patchset/cover.1774342371.git.zhengqi.arch%40bytedance.com
> 
> Sashiko wrote:
> > Does this change inadvertently break the handling of negative byte-sized
> > updates?
> > Looking at the rest of the function:
> > 	if (!val || unit == PAGE_SIZE)
> > 		return val;
> > 	else
> > 		return max(val * unit / PAGE_SIZE, 1UL);
> 
> > PAGE_SIZE is defined as an unsigned long.
> 
> Right, it's defined as 1UL << PAGE_SHIFT.
> 
> > When val is negative, such as during uncharging of byte-sized stats like
> > MEMCG_ZSWAP_B, the expression val * unit is a negative long.
> 
> Right.
> 
> > Dividing a signed long by an unsigned long causes the signed long to be
> > promoted to unsigned before division,
> 
> Right.
> 
> > resulting in a massive positive
> > number instead of a small negative one.
> 
> Let's look at an example (assuming unit is 1).
> 
> val = val * unit = -16384 (-16 KiB)
> val * unit / PAGE_SIZE = 0xFFFFFFFFFFFFC000 / PAGE_SIZE = 0x3FFFFFFFFFFFFF
> max(0x3FFFFFFFFFFFFF, 1UL) = 0x3FFFFFFFFFF
> 
> Yeah, that's a massive positive number.
> 
> Hmm but how did it work when it was int?

Oops, I was about to say... "Oh, doesn't patch 4/4 in v2 need to have
Fixes: 7bd5bc3ce963 ("mm: memcg: normalize the value passed into memcg_rstat_updated()")
???"

but then I realized that I made a silly mistake here.

> val = val * unit = -16384 (-16KiB)
> val * unit / PAGE_SIZE = 0xFFFFFFFFFFFFC000 / PAGE_SIZE = 0x3FFFFFFFFFFFFF

Err, I should have divided it by 0x1000, not 0x4096.

val * unit / PAGE_SIZE = 0xFFFFFFFFFFFFC000 / 0x1000 = 0xFFFFFFFFFFFFC
max(val * unit / PAGE_SIZE, 1UL) = 0xFFFFFFFFFFFFC
(int)0xFFFFFFFFFFFFC = -4.

> max(val * unit / PAGE_SIZE, 1UL) = 0x3FFFFFFFFFFFFF
> (int)0x3FFFFFFFFFFFFF = 0xFFFFFFFF = (-1)
> 
> That's incorrect. It should have been -4?

So that was correct.

The existing logic produces an accurate number (intended or not) as it
is right-shifted to only PAGE_SHIFT bits and truncated to int.

The existing logic is fine, it'll only be a problem when it's not
truncated to int.

> > Before this change, the function returned an int, which implicitly truncated
> > the massive unsigned 64-bit result to a 32-bit int, accidentally yielding the
> > correct negative arithmetic value.
> 
> So... "accidentally yielding the correct negative arithemetic value"
> is wrong.

I was wrong, not sashiko!

> Sounds like it's been subtly broken even before this patch and nobody
> noticed.

No, it's not.

-- 
Cheers,
Harry / Hyeonggon

[PATCH 3/3] mm: memcontrol: correct the nr_pages parameter type of mem_cgroup_update_lru_size()

[Qi Zheng]

From: Qi Zheng <zhengqi.arch@bytedance.com>

The nr_pages parameter of mem_cgroup_update_lru_size() should clearly
be long instead of int, let's correct it.

Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
---
 include/linux/memcontrol.h | 2 +-
 mm/memcontrol.c            | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 0861589695298..dc3fa687759b4 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -878,7 +878,7 @@ static inline bool mem_cgroup_online(struct mem_cgroup *memcg)
 }
 
 void mem_cgroup_update_lru_size(struct lruvec *lruvec, enum lru_list lru,
-		int zid, int nr_pages);
+		int zid, long nr_pages);
 
 static inline
 unsigned long mem_cgroup_get_zone_lru_size(struct lruvec *lruvec,
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 4a78550f6174e..6491ca74b3398 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1466,7 +1466,7 @@ struct lruvec *folio_lruvec_lock_irqsave(struct folio *folio,
  * to or just after a page is removed from an lru list.
  */
 void mem_cgroup_update_lru_size(struct lruvec *lruvec, enum lru_list lru,
-				int zid, int nr_pages)
+				int zid, long nr_pages)
 {
 	struct mem_cgroup_per_node *mz;
 	unsigned long *lru_size;
-- 
2.20.1

Re: [PATCH 3/3] mm: memcontrol: correct the nr_pages parameter type of mem_cgroup_update_lru_size()

[Lorenzo Stoakes (Oracle)]

On Tue, Mar 24, 2026 at 07:31:29PM +0800, Qi Zheng wrote:
> From: Qi Zheng <zhengqi.arch@bytedance.com>
>
> The nr_pages parameter of mem_cgroup_update_lru_size() should clearly
> be long instead of int, let's correct it.

Hmm, but are you ever going to be adding that many pages? I guess technically
correct though.

You should list all the callers and how they use long or unsigned long,
e.g. update_lru_sizes(), lruvec_reparent_lru(), lru_gen_reparent_memcg().

And maybe look a bit into why this was int, if there's any reason for it at
all :)

I'm assuming there's no weird 'let's intentionally let this overflow'
behaviour here.

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

This one looks correct overall, so, with commit message improved:

Reviewed-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org>

> ---
>  include/linux/memcontrol.h | 2 +-
>  mm/memcontrol.c            | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index 0861589695298..dc3fa687759b4 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -878,7 +878,7 @@ static inline bool mem_cgroup_online(struct mem_cgroup *memcg)
>  }
>
>  void mem_cgroup_update_lru_size(struct lruvec *lruvec, enum lru_list lru,
> -		int zid, int nr_pages);
> +		int zid, long nr_pages);
>
>  static inline
>  unsigned long mem_cgroup_get_zone_lru_size(struct lruvec *lruvec,
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 4a78550f6174e..6491ca74b3398 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1466,7 +1466,7 @@ struct lruvec *folio_lruvec_lock_irqsave(struct folio *folio,
>   * to or just after a page is removed from an lru list.
>   */
>  void mem_cgroup_update_lru_size(struct lruvec *lruvec, enum lru_list lru,
> -				int zid, int nr_pages)
> +				int zid, long nr_pages)
>  {
>  	struct mem_cgroup_per_node *mz;
>  	unsigned long *lru_size;
> --
> 2.20.1
>

Re: [PATCH 3/3] mm: memcontrol: correct the nr_pages parameter type of mem_cgroup_update_lru_size()

[Harry Yoo (Oracle)]

On Tue, Mar 24, 2026 at 12:28:08PM +0000, Lorenzo Stoakes (Oracle) wrote:
> On Tue, Mar 24, 2026 at 07:31:29PM +0800, Qi Zheng wrote:
> > From: Qi Zheng <zhengqi.arch@bytedance.com>
> >
> > The nr_pages parameter of mem_cgroup_update_lru_size() should clearly
> > be long instead of int, let's correct it.
> 
> Hmm, but are you ever going to be adding that many pages? I guess technically
> correct though.

It has been fine as-is, but reparenting changes that.
Reparenting in theory could add billions of pages at once!

And yeah the commit message could be improved.

-- 
Cheers,
Harry / Hyeonggon

Re: [PATCH 3/3] mm: memcontrol: correct the nr_pages parameter type of mem_cgroup_update_lru_size()

[Qi Zheng]

On 3/25/26 8:27 AM, Harry Yoo (Oracle) wrote:
> On Tue, Mar 24, 2026 at 12:28:08PM +0000, Lorenzo Stoakes (Oracle) wrote:
>> On Tue, Mar 24, 2026 at 07:31:29PM +0800, Qi Zheng wrote:
>>> From: Qi Zheng <zhengqi.arch@bytedance.com>
>>>
>>> The nr_pages parameter of mem_cgroup_update_lru_size() should clearly
>>> be long instead of int, let's correct it.
>>
>> Hmm, but are you ever going to be adding that many pages? I guess technically
>> correct though.
> 
> It has been fine as-is, but reparenting changes that.
> Reparenting in theory could add billions of pages at once!

Right. Thanks for the explanation!

> 
> And yeah the commit message could be improved.

Yes, I've omitted a lot of background information, which might seem
abrupt to some reviewers unfamiliar with it. I will improve this in the
next version.

Thanks,
Qi

> 

Re: [PATCH 3/3] mm: memcontrol: correct the nr_pages parameter type of mem_cgroup_update_lru_size()

[kernel test robot]

Hi Qi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on akpm-mm/mm-everything]
[also build test WARNING on next-20260325]
[cannot apply to trace/for-next linus/master v7.0-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Qi-Zheng/mm-memcontrol-correct-the-type-of-stats_updates-to-unsigned-long/20260325-230337
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/2cf06f9faf51900ce6acbb4740fc60355a2842ed.1774342371.git.zhengqi.arch%40bytedance.com
patch subject: [PATCH 3/3] mm: memcontrol: correct the nr_pages parameter type of mem_cgroup_update_lru_size()
config: riscv-allnoconfig-bpf (https://download.01.org/0day-ci/archive/20260326/202603260338.GfCrsaVO-lkp@intel.com/config)
compiler: riscv64-linux-gnu-gcc (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260326/202603260338.GfCrsaVO-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603260338.GfCrsaVO-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from ./arch/riscv/include/asm/bug.h:90,
                    from ./include/linux/bug.h:5,
                    from ./arch/riscv/include/asm/cmpxchg.h:9,
                    from ./arch/riscv/include/asm/barrier.h:14,
                    from ./include/linux/list.h:11,
                    from ./include/linux/cgroup-defs.h:12,
                    from mm/memcontrol.c:28:
   mm/memcontrol.c: In function 'mem_cgroup_update_lru_size':
>> mm/memcontrol.c:1486:17: warning: format '%d' expects argument of type 'int', but argument 5 has type 'long int' [-Wformat=]
    1486 |                 "%s(%p, %d, %d): lru_size %ld\n",
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    1487 |                 __func__, lruvec, lru, nr_pages, size)) {
         |                                        ~~~~~~~~
         |                                        |
         |                                        long int
   ./include/asm-generic/bug.h:133:31: note: in definition of macro '__WARN_printf'
     133 |                 __warn_printk(arg);                                     \
         |                               ^~~
   ./include/linux/once_lite.h:31:25: note: in expansion of macro 'WARN'
      31 |                         func(__VA_ARGS__);                              \
         |                         ^~~~
   ./include/asm-generic/bug.h:185:9: note: in expansion of macro 'DO_ONCE_LITE_IF'
     185 |         DO_ONCE_LITE_IF(condition, WARN, 1, format)
         |         ^~~~~~~~~~~~~~~
   mm/memcontrol.c:1485:13: note: in expansion of macro 'WARN_ONCE'
    1485 |         if (WARN_ONCE(size < 0,
         |             ^~~~~~~~~
   mm/memcontrol.c:1486:30: note: format string is defined here
    1486 |                 "%s(%p, %d, %d): lru_size %ld\n",
         |                             ~^
         |                              |
         |                              int
         |                             %ld


vim +1486 mm/memcontrol.c

6168d0da2b479ce Alex Shi           2020-12-15  1457  
925b7673cce3911 Johannes Weiner    2012-01-12  1458  /**
fa9add641b1b1c5 Hugh Dickins       2012-05-29  1459   * mem_cgroup_update_lru_size - account for adding or removing an lru page
fa9add641b1b1c5 Hugh Dickins       2012-05-29  1460   * @lruvec: mem_cgroup per zone lru vector
fa9add641b1b1c5 Hugh Dickins       2012-05-29  1461   * @lru: index of lru list the page is sitting on
b4536f0c829c858 Michal Hocko       2017-01-10  1462   * @zid: zone id of the accounted pages
fa9add641b1b1c5 Hugh Dickins       2012-05-29  1463   * @nr_pages: positive when adding or negative when removing
925b7673cce3911 Johannes Weiner    2012-01-12  1464   *
ca707239e8a7958 Hugh Dickins       2016-05-19  1465   * This function must be called under lru_lock, just before a page is added
07ca760673088f2 Hugh Dickins       2022-02-14  1466   * to or just after a page is removed from an lru list.
925b7673cce3911 Johannes Weiner    2012-01-12  1467   */
fa9add641b1b1c5 Hugh Dickins       2012-05-29  1468  void mem_cgroup_update_lru_size(struct lruvec *lruvec, enum lru_list lru,
6a3cda9fd4d0f0b Qi Zheng           2026-03-24  1469  				int zid, long nr_pages)
08e552c69c6930d KAMEZAWA Hiroyuki  2009-01-07  1470  {
ef8f2327996b5c2 Mel Gorman         2016-07-28  1471  	struct mem_cgroup_per_node *mz;
fa9add641b1b1c5 Hugh Dickins       2012-05-29  1472  	unsigned long *lru_size;
ca707239e8a7958 Hugh Dickins       2016-05-19  1473  	long size;
08e552c69c6930d KAMEZAWA Hiroyuki  2009-01-07  1474  
f8d665422603ee1 Hirokazu Takahashi 2009-01-07  1475  	if (mem_cgroup_disabled())
08e552c69c6930d KAMEZAWA Hiroyuki  2009-01-07  1476  		return;
6d12e2d8ddbe653 KAMEZAWA Hiroyuki  2008-02-07  1477  
ef8f2327996b5c2 Mel Gorman         2016-07-28  1478  	mz = container_of(lruvec, struct mem_cgroup_per_node, lruvec);
b4536f0c829c858 Michal Hocko       2017-01-10  1479  	lru_size = &mz->lru_zone_size[zid][lru];
ca707239e8a7958 Hugh Dickins       2016-05-19  1480  
ca707239e8a7958 Hugh Dickins       2016-05-19  1481  	if (nr_pages < 0)
ca707239e8a7958 Hugh Dickins       2016-05-19  1482  		*lru_size += nr_pages;
ca707239e8a7958 Hugh Dickins       2016-05-19  1483  
ca707239e8a7958 Hugh Dickins       2016-05-19  1484  	size = *lru_size;
b4536f0c829c858 Michal Hocko       2017-01-10  1485  	if (WARN_ONCE(size < 0,
b4536f0c829c858 Michal Hocko       2017-01-10 @1486  		"%s(%p, %d, %d): lru_size %ld\n",
b4536f0c829c858 Michal Hocko       2017-01-10  1487  		__func__, lruvec, lru, nr_pages, size)) {
ca707239e8a7958 Hugh Dickins       2016-05-19  1488  		VM_BUG_ON(1);
ca707239e8a7958 Hugh Dickins       2016-05-19  1489  		*lru_size = 0;
ca707239e8a7958 Hugh Dickins       2016-05-19  1490  	}
ca707239e8a7958 Hugh Dickins       2016-05-19  1491  
ca707239e8a7958 Hugh Dickins       2016-05-19  1492  	if (nr_pages > 0)
fa9add641b1b1c5 Hugh Dickins       2012-05-29  1493  		*lru_size += nr_pages;
08e552c69c6930d KAMEZAWA Hiroyuki  2009-01-07  1494  }
544122e5e0ee27d KAMEZAWA Hiroyuki  2009-01-07  1495  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 3/3] mm: memcontrol: correct the nr_pages parameter type of mem_cgroup_update_lru_size()

[Andrew Morton]

On Thu, 26 Mar 2026 03:35:39 +0100 kernel test robot <lkp@intel.com> wrote:

> Hi Qi,
> 
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on akpm-mm/mm-everything]
> [also build test WARNING on next-20260325]
> [cannot apply to trace/for-next linus/master v7.0-rc5]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Qi-Zheng/mm-memcontrol-correct-the-type-of-stats_updates-to-unsigned-long/20260325-230337
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
> patch link:    https://lore.kernel.org/r/2cf06f9faf51900ce6acbb4740fc60355a2842ed.1774342371.git.zhengqi.arch%40bytedance.com
> patch subject: [PATCH 3/3] mm: memcontrol: correct the nr_pages parameter type of mem_cgroup_update_lru_size()
> config: riscv-allnoconfig-bpf (https://download.01.org/0day-ci/archive/20260326/202603260338.GfCrsaVO-lkp@intel.com/config)
> compiler: riscv64-linux-gnu-gcc (Debian 14.2.0-19) 14.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260326/202603260338.GfCrsaVO-lkp@intel.com/reproduce)

Thanks.  Can't reproduce. (there are meds for this!)

> b4536f0c829c858 Michal Hocko       2017-01-10 @1486  		"%s(%p, %d, %d): lru_size %ld\n",

We presently have

		"%s(%p, %d, %ld): lru_size %ld\n",

so it appears we fixed this recently.

Re: [PATCH 0/3] correct the parameter type of some mm functions

[Lorenzo Stoakes (Oracle)]

-cc old mail

(just a gentle nudge to send stuff to ljs@kernel.org instead thanks :)

On Tue, Mar 24, 2026 at 07:31:25PM +0800, Qi Zheng wrote:
> From: Qi Zheng <zhengqi.arch@bytedance.com>
>
> Hi all,
>
> As Harry Yoo pointed out [1], in the case of reparenting LRU folios, the value
> passed to functions such as __mod_memcg{_lruvec}_state() may exceed the upper
> limit of int, so this series aims to correct the parameter type of these
> functions.
>
> This series is based on the next-20260323.
>
> Comments and suggestions are welcome!
>
> Thanks,
> Qi
>
> [1]. https://lore.kernel.org/all/acDxaEgnqPI-Z4be@hyeyoo/
>
> Qi Zheng (3):
>   mm: memcontrol: correct the type of stats_updates to unsigned long
>   mm: memcontrol: correct the parameter type of
>     __mod_memcg{_lruvec}_state()
>   mm: memcontrol: correct the nr_pages parameter type of
>     mem_cgroup_update_lru_size()
>
>  include/linux/memcontrol.h   |  2 +-
>  include/trace/events/memcg.h | 10 +++++-----
>  mm/memcontrol.c              | 28 ++++++++++++++--------------
>  3 files changed, 20 insertions(+), 20 deletions(-)
>
> --
> 2.20.1
>

[PATCH v6 31/33] mm: memcontrol: convert objcg to be per-memcg per-node type

[Qi Zheng]

From: Qi Zheng <zhengqi.arch@bytedance.com>

Convert objcg to be per-memcg per-node type, so that when reparent LRU
folios later, we can hold the lru lock at the node level, thus avoiding
holding too many lru locks at once.

Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
---
 include/linux/memcontrol.h | 23 +++++------
 include/linux/sched.h      |  2 +-
 mm/memcontrol.c            | 79 +++++++++++++++++++++++---------------
 3 files changed, 62 insertions(+), 42 deletions(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index d2748e672fd88..57d86decf2830 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -116,6 +116,16 @@ struct mem_cgroup_per_node {
 	unsigned long		lru_zone_size[MAX_NR_ZONES][NR_LRU_LISTS];
 	struct mem_cgroup_reclaim_iter	iter;
 
+	/*
+	 * objcg is wiped out as a part of the objcg repaprenting process.
+	 * orig_objcg preserves a pointer (and a reference) to the original
+	 * objcg until the end of live of memcg.
+	 */
+	struct obj_cgroup __rcu	*objcg;
+	struct obj_cgroup	*orig_objcg;
+	/* list of inherited objcgs, protected by objcg_lock */
+	struct list_head objcg_list;
+
 #ifdef CONFIG_MEMCG_NMI_SAFETY_REQUIRES_ATOMIC
 	/* slab stats for nmi context */
 	atomic_t		slab_reclaimable;
@@ -180,6 +190,7 @@ struct obj_cgroup {
 		struct list_head list; /* protected by objcg_lock */
 		struct rcu_head rcu;
 	};
+	bool is_root;
 };
 
 /*
@@ -258,15 +269,6 @@ struct mem_cgroup {
 	seqlock_t		socket_pressure_seqlock;
 #endif
 	int kmemcg_id;
-	/*
-	 * memcg->objcg is wiped out as a part of the objcg repaprenting
-	 * process. memcg->orig_objcg preserves a pointer (and a reference)
-	 * to the original objcg until the end of live of memcg.
-	 */
-	struct obj_cgroup __rcu	*objcg;
-	struct obj_cgroup	*orig_objcg;
-	/* list of inherited objcgs, protected by objcg_lock */
-	struct list_head objcg_list;
 
 	struct memcg_vmstats_percpu __percpu *vmstats_percpu;
 
@@ -333,7 +335,6 @@ struct mem_cgroup {
 #define MEMCG_CHARGE_BATCH 64U
 
 extern struct mem_cgroup *root_mem_cgroup;
-extern struct obj_cgroup *root_obj_cgroup;
 
 enum page_memcg_data_flags {
 	/* page->memcg_data is a pointer to an slabobj_ext vector */
@@ -552,7 +553,7 @@ static inline bool mem_cgroup_is_root(struct mem_cgroup *memcg)
 
 static inline bool obj_cgroup_is_root(const struct obj_cgroup *objcg)
 {
-	return objcg == root_obj_cgroup;
+	return objcg->is_root;
 }
 
 static inline bool mem_cgroup_disabled(void)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index a7b4a980eb2f0..7b63b7b74f414 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1533,7 +1533,7 @@ struct task_struct {
 	/* Used by memcontrol for targeted memcg charge: */
 	struct mem_cgroup		*active_memcg;
 
-	/* Cache for current->cgroups->memcg->objcg lookups: */
+	/* Cache for current->cgroups->memcg->nodeinfo[nid]->objcg lookups: */
 	struct obj_cgroup		*objcg;
 #endif
 
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index b0519a16f5684..e31c58bc89188 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -84,8 +84,6 @@ EXPORT_SYMBOL(memory_cgrp_subsys);
 struct mem_cgroup *root_mem_cgroup __read_mostly;
 EXPORT_SYMBOL(root_mem_cgroup);
 
-struct obj_cgroup *root_obj_cgroup __read_mostly;
-
 /* Active memory cgroup to use from an interrupt context */
 DEFINE_PER_CPU(struct mem_cgroup *, int_active_memcg);
 EXPORT_PER_CPU_SYMBOL_GPL(int_active_memcg);
@@ -210,18 +208,21 @@ static struct obj_cgroup *obj_cgroup_alloc(void)
 }
 
 static inline struct obj_cgroup *__memcg_reparent_objcgs(struct mem_cgroup *memcg,
-							 struct mem_cgroup *parent)
+							 struct mem_cgroup *parent,
+							 int nid)
 {
 	struct obj_cgroup *objcg, *iter;
+	struct mem_cgroup_per_node *pn = memcg->nodeinfo[nid];
+	struct mem_cgroup_per_node *parent_pn = parent->nodeinfo[nid];
 
-	objcg = rcu_replace_pointer(memcg->objcg, NULL, true);
+	objcg = rcu_replace_pointer(pn->objcg, NULL, true);
 	/* 1) Ready to reparent active objcg. */
-	list_add(&objcg->list, &memcg->objcg_list);
+	list_add(&objcg->list, &pn->objcg_list);
 	/* 2) Reparent active objcg and already reparented objcgs to parent. */
-	list_for_each_entry(iter, &memcg->objcg_list, list)
+	list_for_each_entry(iter, &pn->objcg_list, list)
 		WRITE_ONCE(iter->memcg, parent);
 	/* 3) Move already reparented objcgs to the parent's list */
-	list_splice(&memcg->objcg_list, &parent->objcg_list);
+	list_splice(&pn->objcg_list, &parent_pn->objcg_list);
 
 	return objcg;
 }
@@ -268,14 +269,17 @@ static void memcg_reparent_objcgs(struct mem_cgroup *memcg)
 {
 	struct obj_cgroup *objcg;
 	struct mem_cgroup *parent = parent_mem_cgroup(memcg);
+	int nid;
 
-	reparent_locks(memcg, parent);
+	for_each_node(nid) {
+		reparent_locks(memcg, parent);
 
-	objcg = __memcg_reparent_objcgs(memcg, parent);
+		objcg = __memcg_reparent_objcgs(memcg, parent, nid);
 
-	reparent_unlocks(memcg, parent);
+		reparent_unlocks(memcg, parent);
 
-	percpu_ref_kill(&objcg->refcnt);
+		percpu_ref_kill(&objcg->refcnt);
+	}
 }
 
 /*
@@ -2877,8 +2881,10 @@ struct mem_cgroup *mem_cgroup_from_virt(void *p)
 
 static struct obj_cgroup *__get_obj_cgroup_from_memcg(struct mem_cgroup *memcg)
 {
+	int nid = numa_node_id();
+
 	for (; memcg; memcg = parent_mem_cgroup(memcg)) {
-		struct obj_cgroup *objcg = rcu_dereference(memcg->objcg);
+		struct obj_cgroup *objcg = rcu_dereference(memcg->nodeinfo[nid]->objcg);
 
 		if (likely(objcg && obj_cgroup_tryget(objcg)))
 			return objcg;
@@ -2942,6 +2948,7 @@ __always_inline struct obj_cgroup *current_obj_cgroup(void)
 {
 	struct mem_cgroup *memcg;
 	struct obj_cgroup *objcg;
+	int nid = numa_node_id();
 
 	if (IS_ENABLED(CONFIG_MEMCG_NMI_UNSAFE) && in_nmi())
 		return NULL;
@@ -2958,14 +2965,14 @@ __always_inline struct obj_cgroup *current_obj_cgroup(void)
 		 * Objcg reference is kept by the task, so it's safe
 		 * to use the objcg by the current task.
 		 */
-		return objcg ? : root_obj_cgroup;
+		return objcg ? : rcu_dereference_check(root_mem_cgroup->nodeinfo[nid]->objcg, 1);
 	}
 
 	memcg = this_cpu_read(int_active_memcg);
 	if (unlikely(memcg))
 		goto from_memcg;
 
-	return root_obj_cgroup;
+	return rcu_dereference_check(root_mem_cgroup->nodeinfo[nid]->objcg, 1);
 
 from_memcg:
 	for (; memcg; memcg = parent_mem_cgroup(memcg)) {
@@ -2975,12 +2982,12 @@ __always_inline struct obj_cgroup *current_obj_cgroup(void)
 		 * away and can be used within the scope without any additional
 		 * protection.
 		 */
-		objcg = rcu_dereference_check(memcg->objcg, 1);
+		objcg = rcu_dereference_check(memcg->nodeinfo[nid]->objcg, 1);
 		if (likely(objcg))
 			return objcg;
 	}
 
-	return root_obj_cgroup;
+	return rcu_dereference_check(root_mem_cgroup->nodeinfo[nid]->objcg, 1);
 }
 
 struct obj_cgroup *get_obj_cgroup_from_folio(struct folio *folio)
@@ -3877,6 +3884,8 @@ static bool alloc_mem_cgroup_per_node_info(struct mem_cgroup *memcg, int node)
 	if (!pn->lruvec_stats_percpu)
 		goto fail;
 
+	INIT_LIST_HEAD(&pn->objcg_list);
+
 	lruvec_init(&pn->lruvec);
 	pn->memcg = memcg;
 
@@ -3891,10 +3900,12 @@ static void __mem_cgroup_free(struct mem_cgroup *memcg)
 {
 	int node;
 
-	obj_cgroup_put(memcg->orig_objcg);
+	for_each_node(node) {
+		struct mem_cgroup_per_node *pn = memcg->nodeinfo[node];
 
-	for_each_node(node)
-		free_mem_cgroup_per_node_info(memcg->nodeinfo[node]);
+		obj_cgroup_put(pn->orig_objcg);
+		free_mem_cgroup_per_node_info(pn);
+	}
 	memcg1_free_events(memcg);
 	kfree(memcg->vmstats);
 	free_percpu(memcg->vmstats_percpu);
@@ -3965,7 +3976,6 @@ static struct mem_cgroup *mem_cgroup_alloc(struct mem_cgroup *parent)
 #endif
 	memcg1_memcg_init(memcg);
 	memcg->kmemcg_id = -1;
-	INIT_LIST_HEAD(&memcg->objcg_list);
 #ifdef CONFIG_CGROUP_WRITEBACK
 	INIT_LIST_HEAD(&memcg->cgwb_list);
 	for (i = 0; i < MEMCG_CGWB_FRN_CNT; i++)
@@ -4042,6 +4052,7 @@ static int mem_cgroup_css_online(struct cgroup_subsys_state *css)
 {
 	struct mem_cgroup *memcg = mem_cgroup_from_css(css);
 	struct obj_cgroup *objcg;
+	int nid;
 
 	memcg_online_kmem(memcg);
 
@@ -4053,17 +4064,19 @@ static int mem_cgroup_css_online(struct cgroup_subsys_state *css)
 	if (alloc_shrinker_info(memcg))
 		goto offline_kmem;
 
-	objcg = obj_cgroup_alloc();
-	if (!objcg)
-		goto free_shrinker;
+	for_each_node(nid) {
+		objcg = obj_cgroup_alloc();
+		if (!objcg)
+			goto free_objcg;
 
-	if (unlikely(mem_cgroup_is_root(memcg)))
-		root_obj_cgroup = objcg;
+		if (unlikely(mem_cgroup_is_root(memcg)))
+			objcg->is_root = true;
 
-	objcg->memcg = memcg;
-	rcu_assign_pointer(memcg->objcg, objcg);
-	obj_cgroup_get(objcg);
-	memcg->orig_objcg = objcg;
+		objcg->memcg = memcg;
+		rcu_assign_pointer(memcg->nodeinfo[nid]->objcg, objcg);
+		obj_cgroup_get(objcg);
+		memcg->nodeinfo[nid]->orig_objcg = objcg;
+	}
 
 	if (unlikely(mem_cgroup_is_root(memcg)) && !mem_cgroup_disabled())
 		queue_delayed_work(system_dfl_wq, &stats_flush_dwork,
@@ -4087,7 +4100,13 @@ static int mem_cgroup_css_online(struct cgroup_subsys_state *css)
 	xa_store(&mem_cgroup_private_ids, memcg->id.id, memcg, GFP_KERNEL);
 
 	return 0;
-free_shrinker:
+free_objcg:
+	for_each_node(nid) {
+		struct mem_cgroup_per_node *pn = memcg->nodeinfo[nid];
+
+		if (pn && pn->orig_objcg)
+			obj_cgroup_put(pn->orig_objcg);
+	}
 	free_shrinker_info(memcg);
 offline_kmem:
 	memcg_offline_kmem(memcg);
-- 
2.20.1

[PATCH] fix: mm: memcontrol: convert objcg to be per-memcg per-node type

[Qi Zheng]

From: Qi Zheng <zhengqi.arch@bytedance.com>

Reset pn->orig_objcg to NULL to prevent obj_cgroup_put()
from being called agagin in __mem_cgroup_free().

Reported-by: Usama Arif <usama.arif@linux.dev>
Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
---
 mm/memcontrol.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 992a3f5caa62b..ad32639ea5959 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4140,8 +4140,14 @@ static int mem_cgroup_css_online(struct cgroup_subsys_state *css)
 	for_each_node(nid) {
 		struct mem_cgroup_per_node *pn = memcg->nodeinfo[nid];
 
-		if (pn && pn->orig_objcg)
+		if (pn && pn->orig_objcg) {
 			obj_cgroup_put(pn->orig_objcg);
+			/*
+			 * Reset pn->orig_objcg to NULL to prevent obj_cgroup_put()
+			 * from being called agagin in __mem_cgroup_free().
+			 */
+			pn->orig_objcg = NULL;
+		}
 	}
 	free_shrinker_info(memcg);
 offline_kmem:
-- 
2.20.1

Re: [PATCH] fix: mm: memcontrol: convert objcg to be per-memcg per-node type

[Usama Arif]

On 09/03/2026 14:29, Qi Zheng wrote:
> From: Qi Zheng <zhengqi.arch@bytedance.com>
> 
> Reset pn->orig_objcg to NULL to prevent obj_cgroup_put()
> from being called agagin in __mem_cgroup_free().
> 
> Reported-by: Usama Arif <usama.arif@linux.dev>
> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
> ---
>  mm/memcontrol.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 992a3f5caa62b..ad32639ea5959 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -4140,8 +4140,14 @@ static int mem_cgroup_css_online(struct cgroup_subsys_state *css)
>  	for_each_node(nid) {
>  		struct mem_cgroup_per_node *pn = memcg->nodeinfo[nid];
>  
> -		if (pn && pn->orig_objcg)
> +		if (pn && pn->orig_objcg) {
>  			obj_cgroup_put(pn->orig_objcg);
> +			/*
> +			 * Reset pn->orig_objcg to NULL to prevent obj_cgroup_put()
> +			 * from being called agagin in __mem_cgroup_free().

nit: s/agagin/again/

Apart from the nit.

Acked-by: Usama Arif <usama.arif@linux.dev>

> +			 */
> +			pn->orig_objcg = NULL;
> +		}
>  	}
>  	free_shrinker_info(memcg);
>  offline_kmem:

Re: [PATCH] fix: mm: memcontrol: convert objcg to be per-memcg per-node type

[Qi Zheng]

On 3/9/26 7:33 PM, Usama Arif wrote:
> 
> 
> On 09/03/2026 14:29, Qi Zheng wrote:
>> From: Qi Zheng <zhengqi.arch@bytedance.com>
>>
>> Reset pn->orig_objcg to NULL to prevent obj_cgroup_put()
>> from being called agagin in __mem_cgroup_free().
>>
>> Reported-by: Usama Arif <usama.arif@linux.dev>
>> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
>> ---
>>   mm/memcontrol.c | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index 992a3f5caa62b..ad32639ea5959 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -4140,8 +4140,14 @@ static int mem_cgroup_css_online(struct cgroup_subsys_state *css)
>>   	for_each_node(nid) {
>>   		struct mem_cgroup_per_node *pn = memcg->nodeinfo[nid];
>>   
>> -		if (pn && pn->orig_objcg)
>> +		if (pn && pn->orig_objcg) {
>>   			obj_cgroup_put(pn->orig_objcg);
>> +			/*
>> +			 * Reset pn->orig_objcg to NULL to prevent obj_cgroup_put()
>> +			 * from being called agagin in __mem_cgroup_free().
> 
> nit: s/agagin/again/

Ouch, my bad.

Hi Andrew, can you help squash the following diff:

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index ad32639ea5959..5fcbb651846a4 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4144,7 +4144,7 @@ static int mem_cgroup_css_online(struct 
cgroup_subsys_state *css)
                         obj_cgroup_put(pn->orig_objcg);
                         /*
                          * Reset pn->orig_objcg to NULL to prevent 
obj_cgroup_put()
-                        * from being called agagin in __mem_cgroup_free().
+                        * from being called again in __mem_cgroup_free().
                          */
                         pn->orig_objcg = NULL;
                 }

> 
> Apart from the nit.
> 
> Acked-by: Usama Arif <usama.arif@linux.dev>

Thanks!

> 
>> +			 */
>> +			pn->orig_objcg = NULL;
>> +		}
>>   	}
>>   	free_shrinker_info(memcg);
>>   offline_kmem:
>