[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aXrBiPlpEOOC3cMZ@hyeyoo>
Date: Thu, 29 Jan 2026 11:10:16 +0900
From: Harry Yoo <harry.yoo@...cle.com>
To: Qi Zheng <qi.zheng@...ux.dev>
Cc: Shakeel Butt <shakeel.butt@...ux.dev>, hannes@...xchg.org,
hughd@...gle.com, mhocko@...e.com, roman.gushchin@...ux.dev,
muchun.song@...ux.dev, david@...nel.org, lorenzo.stoakes@...cle.com,
ziy@...dia.com, yosry.ahmed@...ux.dev, imran.f.khan@...cle.com,
kamalesh.babulal@...cle.com, axelrasmussen@...gle.com,
yuanchu@...gle.com, weixugc@...gle.com, chenridong@...weicloud.com,
mkoutny@...e.com, akpm@...ux-foundation.org,
hamzamahfooz@...ux.microsoft.com, apais@...ux.microsoft.com,
lance.yang@...ux.dev, linux-mm@...ck.org, linux-kernel@...r.kernel.org,
cgroups@...r.kernel.org, Qi Zheng <zhengqi.arch@...edance.com>
Subject: Re: [PATCH v3 28/30] mm: memcontrol: prepare for reparenting
state_local
On Mon, Jan 19, 2026 at 11:34:53AM +0800, Qi Zheng wrote:
>
>
> On 1/18/26 11:20 AM, Shakeel Butt wrote:
> > On Wed, Jan 14, 2026 at 07:32:55PM +0800, Qi Zheng wrote:
> > > From: Qi Zheng <zhengqi.arch@...edance.com>
> > >
> > > To resolve the dying memcg issue, we need to reparent LRU folios of child
> > > memcg to its parent memcg. The following counts are all non-hierarchical
> > > and need to be reparented to prevent the counts of parent memcg overflow.
> > >
> > > 1. memcg->vmstats->state_local[i]
> > > 2. pn->lruvec_stats->state_local[i]
> > >
> > > This commit implements the specific function, which will be used during
> > > the reparenting process.
> >
> > Please add more explanation which was discussed in the email chain at
> > https://lore.kernel.org/all/5dsb6q2r4xsi24kk5gcnckljuvgvvp6nwifwvc4wuho5hsifeg@5ukg2dq6ini5/
>
> OK, will do.
>
> > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > > index 70583394f421f..7aa32b97c9f17 100644
> > > --- a/mm/memcontrol.c
> > > +++ b/mm/memcontrol.c
> > > @@ -225,6 +225,28 @@ static inline struct obj_cgroup *__memcg_reparent_objcgs(struct mem_cgroup *memc
> > > return objcg;
> > > }
> > > +#ifdef CONFIG_MEMCG_V1
> > > +static void __mem_cgroup_flush_stats(struct mem_cgroup *memcg, bool force);
> > > +
> > > +static inline void reparent_state_local(struct mem_cgroup *memcg, struct mem_cgroup *parent)
> > > +{
> > > + if (cgroup_subsys_on_dfl(memory_cgrp_subsys))
> > > + return;
> > > +
> > > + synchronize_rcu();
> >
> > Hmm synchrinuze_rcu() is a heavy hammer here. Also you would need rcu
> > read lock in mod_memcg_state() & mod_memcg_lruvec_state() for this
> > synchronize_rcu().
>
> Since these two functions require memcg or lruvec, they are already
> within the critical section of the RCU lock.
What happens if someone grabbed a refcount and then release the rcu read
lock before percpu refkill and then call mod_memcg[_lruvec]_state()?
In this case, can we end up reparenting in the middle of non-hierarchical
stat update because they don't have RCU grace period?
Something like
T1 T2
- rcu_read_lock()
- get memcg refcnt
- rcu_read_unlock()
- call mod_memcg_state()
- CSS_IS_DYING is not set
- Set CSS_IS_DYING
- Trigger percpu refkill
- Trigger offline_css()
-> reparent non-hierarchical - update non-hierarchical stats
stats
- put memcg refcount
> > Hmm instead of synchronize_rcu() here, we can use queue_rcu_work() in
> > css_killed_ref_fn(). It would be as simple as the following:
>
> It does look much simpler, will do.
>
> Thanks,
> Qi
--
Cheers,
Harry / Hyeonggon
Powered by blists - more mailing lists