Re: [PATCH 1/4] memcg: keep swap charging under RCU protection

[Bingfang Guo <bfguo@icloud.com>]

On Fri, Sep 18, 2026 at 09:46:47AM -0700, Shakeel Butt wrote:
> On Fri, Sep 18, 2026 at 05:18:40PM +0800, Bingfang Guo via B4 Relay wrote:
> > From: Bingfang Guo <bingfangguo@tencent.com>
> > 
> > This is a preparatory work for unbinding memcgid from memcg. No
> > functional change.
> > 
> > The swap charging path currently drops its RCU read lock after acquiring
> > a private ID reference. This is safe because the ID reference pins the
> > memcg's CSS.
> > 
> > Moving private ID references to objcgs will remove that lifetime
> > guarantee. Keep the RCU read lock held while accessing the memcg for
> > counter charging, statistics and failure handling. (This matches what
> > __memcg1_swapout() already does.).
> > 
> > Save the private ID before dropping the RCU read lock, and use the saved
> > value when recording the swap entry. The swap cluster locking remains
> > outside the RCU read-side critical section.
> > 
> > Signed-off-by: Bingfang Guo <bingfangguo@tencent.com>
> > ---
> >  mm/memcontrol.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index 791e536efaebe..72522ec827c9a 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -5954,6 +5954,7 @@ int __mem_cgroup_try_charge_swap(struct folio *folio)
> >  	struct page_counter *counter;
> >  	struct mem_cgroup *memcg;
> >  	struct obj_cgroup *objcg;
> > +	unsigned short private_id;
> >  
> >  	if (do_memsw_account())
> >  		return 0;
> > @@ -5973,20 +5974,21 @@ int __mem_cgroup_try_charge_swap(struct folio *folio)
> >  
> 
> If there is one more spin of this patch, I think scoped_guard(rcu) would be more
> readable here, so please use that here.
> 

Hi, Shakeel. Thanks for your review and suggestion!

It looks like a good idea. I will include that in the next version!

> >  	memcg = mem_cgroup_private_id_get_online(memcg, nr_pages);
> >  	/* memcg is pined by memcg ID. */
> > -	rcu_read_unlock();
> > +	private_id = mem_cgroup_private_id(memcg);
> >  
> >  	if (!mem_cgroup_is_root(memcg) &&
> >  	    !page_counter_try_charge(&memcg->swap, nr_pages, &counter)) {
> >  		memcg_memory_event(memcg, MEMCG_SWAP_MAX);
> >  		memcg_memory_event(memcg, MEMCG_SWAP_FAIL);
> >  		mem_cgroup_private_id_put(memcg, nr_pages);
> > +		rcu_read_unlock();
> >  		return -ENOMEM;
> >  	}
> >  	mod_memcg_state(memcg, MEMCG_SWAP, nr_pages);
> > +	rcu_read_unlock();
> >  
> >  	ci = swap_cluster_get_and_lock(folio);
> > -	__swap_cgroup_set(ci, swp_cluster_offset(folio->swap), nr_pages,
> > -			  mem_cgroup_private_id(memcg));
> > +	__swap_cgroup_set(ci, swp_cluster_offset(folio->swap), nr_pages, private_id);
> >  	swap_cluster_unlock(ci);
> >  
> >  	return 0;
> > 
> > -- 
> > 2.43.7
> > 
> >