From: Qi Zheng <zhengqi.arch@bytedance.com>
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.
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.1On Wed, Mar 25, 2026 at 10:13:22PM +0800, Qi Zheng wrote:
> From: Qi Zheng <zhengqi.arch@bytedance.com>
>
> 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@bytedance.com>
Anyway logic seems fine to me, so:
Reviewed-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org>
> ---
> 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
>
On 3/25/26 11:28 PM, Lorenzo Stoakes (Oracle) wrote: > On Wed, Mar 25, 2026 at 10:13:22PM +0800, Qi Zheng wrote: >> From: Qi Zheng <zhengqi.arch@bytedance.com> >> >> 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. Before LRU folio reparenting was introduced, we wouldn’t pass in such a large value, so this wasn’t a problem. Since LRU folio reparenting is still in mm-unstable, so I didn't add a Fixes tag in [4/4]. > >> >> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com> > > Anyway logic seems fine to me, so: > > Reviewed-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org> Thanks!
On Thu, Mar 26, 2026 at 10:32:43AM +0800, Qi Zheng wrote: > > > On 3/25/26 11:28 PM, Lorenzo Stoakes (Oracle) wrote: > > On Wed, Mar 25, 2026 at 10:13:22PM +0800, Qi Zheng wrote: > > > From: Qi Zheng <zhengqi.arch@bytedance.com> > > > > > > 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. > > Before LRU folio reparenting was introduced, we wouldn’t pass in such a > large value, so this wasn’t a problem. Since LRU folio reparenting is > still in mm-unstable, so I didn't add a Fixes tag in [4/4]. Ah, well these patches should be _before_ the LRU folio reparenting then? Andrew - can we ensure correct ordering here? > > > > > > > > > Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com> > > > > Anyway logic seems fine to me, so: > > > > Reviewed-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org> > > Thanks! > > Thanks, Lorenzo
On 3/26/26 4:05 PM, Lorenzo Stoakes (Oracle) wrote: > On Thu, Mar 26, 2026 at 10:32:43AM +0800, Qi Zheng wrote: >> >> >> On 3/25/26 11:28 PM, Lorenzo Stoakes (Oracle) wrote: >>> On Wed, Mar 25, 2026 at 10:13:22PM +0800, Qi Zheng wrote: >>>> From: Qi Zheng <zhengqi.arch@bytedance.com> >>>> >>>> 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. >> >> Before LRU folio reparenting was introduced, we wouldn’t pass in such a >> large value, so this wasn’t a problem. Since LRU folio reparenting is >> still in mm-unstable, so I didn't add a Fixes tag in [4/4]. > > Ah, well these patches should be _before_ the LRU folio reparenting then? > > Andrew - can we ensure correct ordering here? There are some dependencies for this. To be precise, it should be applied after: [PATCH v6 29/33] mm: memcontrol: refactor mod_memcg_state() and mod_memcg_lruvec_state() and before: [PATCH v6 32/33] mm: memcontrol: eliminate the problem of dying memory cgroup for LRU folios and there might be conflicts. Would it be okay to merge them together into v7.1-rcX? Otherwise, perhaps updating to v7 would be more convenient for Andrew. Hi Andrew, what do you think? Thanks, Qi > >> >>> >>>> >>>> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com> >>> >>> Anyway logic seems fine to me, so: >>> >>> Reviewed-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org> >> >> Thanks! >> >> > > Thanks, Lorenzo
On Thu, Mar 26, 2026 at 08:05:57AM +0000, Lorenzo Stoakes (Oracle) wrote: > On Thu, Mar 26, 2026 at 10:32:43AM +0800, Qi Zheng wrote: > > > > > > On 3/25/26 11:28 PM, Lorenzo Stoakes (Oracle) wrote: > > > On Wed, Mar 25, 2026 at 10:13:22PM +0800, Qi Zheng wrote: > > > > From: Qi Zheng <zhengqi.arch@bytedance.com> > > > > > > > > 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. > > > > Before LRU folio reparenting was introduced, we wouldn’t pass in such a > > large value, so this wasn’t a problem. Since LRU folio reparenting is > > still in mm-unstable, so I didn't add a Fixes tag in [4/4]. > > Ah, well these patches should be _before_ the LRU folio reparenting then? Yes. I think that'll be the best option. If that's too much of a headache at this point (I'm not sure), it should be at least part of 7.1-rcX, given that it's quite unlikely that people will notice it during bisection anyway... -- Cheers, Harry / Hyeonggon
© 2016 - 2026 Red Hat, Inc.