Re: [PATCH 4/4] memcg: move memcg private ID refcount to objcg

[Bingfang Guo]

On Fri, Sep 18, 2026 at 11:19:08AM +0800, Shakeel Butt wrote:
> On Fri, Sep 18, 2026 at 05:18:43PM +0800, Bingfang Guo via B4 Relay wrote:
> > From: Bingfang Guo <bingfangguo@xxxxxxxxxxx>
> > 
> > diff --git a/mm/workingset.c b/mm/workingset.c
> > index 8412f4840ae35..7e4fbc5a786d6 100644
> > --- a/mm/workingset.c
> > +++ b/mm/workingset.c
> > @@ -281,6 +281,9 @@ static bool lru_gen_test_recent(void *shadow, struct lruvec **lruvec,
> >  	unpack_shadow(shadow, &memcg_id, &pgdat, token, workingset);
> >  
> >  	memcg = mem_cgroup_from_private_id(memcg_id);
> > +	if (!memcg)
> > +		return false;
> 
> You can not return here without initializing *lruvec as it might be used in the
> caller. Also mem_cgroup_lruvec() can handle null memcg and will substitute with
> root_mem_cgroup.
> 

Oops. Thanks! Nice catch!

I made a mistake here and it managed to work so I didn't notice
it...

In the previous patch I left here unchanged and used the root.
But then it struck me that maybe it is more consistent to make
the classical LRU and MGLRU behave similarly by skipping the
recency check for both of them.

I'd like to hear about your suggestions:  Whether to simply
remove the if statement and use the root only for MGLRU? Or to
use the root and keep the test speculative for both the two?

Thanks
Bingfang

> > +
> >  	*lruvec = mem_cgroup_lruvec(memcg, pgdat);
> >  
> >  	max_seq = READ_ONCE((*lruvec)->lrugen.max_seq);
> > @@ -470,7 +473,7 @@ bool workingset_test_recent(void *shadow, bool file, bool *workingset,
> >  	 * configurations instead.
> >  	 */
> >  	eviction_memcg = mem_cgroup_from_private_id(memcgid);
> > -	if (!mem_cgroup_tryget(eviction_memcg))
> > +	if (eviction_memcg && !mem_cgroup_tryget(eviction_memcg))
> >  		eviction_memcg = NULL;
> >  	rcu_read_unlock();
> >  
> > 
> > -- 
> > 2.43.7
> > 
> >