linux-kernel - Re: [PATCH v3 28/30] mm: memcontrol: prepare for reparenting state

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aXtRWdwwmi7G-Hlg@hyeyoo>
Date: Thu, 29 Jan 2026 21:23:53 +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 Thu, Jan 29, 2026 at 04:50:39PM +0800, Qi Zheng wrote:
> 
> 
> On 1/29/26 10:10 AM, Harry Yoo wrote:
> > 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
> 
> Good catch, I think you are right.
> 
> The rcu lock should be added to mod_memcg_state() and
> mod_memcg_lruvec_state().

Thanks for confirming!

Because it's quite confusing, let me ask few more questions...

Q1. Yosry mentioned [1] [2] that stat updates should be done in the same
RCU section that is used to grab a refcount of the cgroup.

But I don't think your work is relying on that. Instead, I guess, it's
relying on the CSS_DYING check from reader side to determine whether it
should update stats of the child or parent memcg, right?

-> That being said, when rcu_read_lock() is called _after_ stats are
   reparented, the reader must observe that the CSS_DYING flag is set.

[1] https://lore.kernel.org/all/utl6esq7jz5e4f7kwgrpwdjc2rm3yi33ljb6xkm5nxzufa4o7s@hblq2piu3vnz 
[2] https://lore.kernel.org/all/ebdhvcwygvnfejai5azhg3sjudsjorwmlcvmzadpkhexoeq3tb@5gj5y2exdhpn

Q2. When a reader checks CSS_DYING flag, how is the flag change
guaranteed to be visible to the reader without any lock, memory barrier,
or atomic ops involved?

As Shakeel mentioned elsewhere, I hope some explanations for correctness
to be included in the commit message :)

> I will update to v4 as soon as possible.

Thanks a lot!

I'll wait for that and will review carefully to make sure it's correct ;)

> Thanks,
> Qi
> 
> > > > 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