Re: [PATCH] mm: memcontrol: fix forget to obtain the ref to objcg in split_page_memcg

[Roman Gushchin <guro@fb.com>]

On Thu, Apr 01, 2021 at 11:31:16AM +0800, Miaohe Lin wrote:
> On 2021/4/1 11:01, Muchun Song wrote:
> > Christian Borntraeger reported a warning about "percpu ref
> > (obj_cgroup_release) <= 0 (-1) after switching to atomic".
> > Because we forgot to obtain the reference to the objcg and
> > wrongly obtain the reference of memcg.
> > 
> > Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> 
> Thanks for the patch.
> Is a Fixes tag needed?

No, as the original patch hasn't been merged into the Linus's tree yet.
So the fix can be simply squashed.

Btw, the fix looks good to me.

Acked-by: Roman Gushchin <guro@fb.com>

> 
> > ---
> >  include/linux/memcontrol.h | 6 ++++++
> >  mm/memcontrol.c            | 6 +++++-
> >  2 files changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> > index 0e8907957227..c960fd49c3e8 100644
> > --- a/include/linux/memcontrol.h
> > +++ b/include/linux/memcontrol.h
> > @@ -804,6 +804,12 @@ static inline void obj_cgroup_get(struct obj_cgroup *objcg)
> >  	percpu_ref_get(&objcg->refcnt);
> >  }
> >  
> > +static inline void obj_cgroup_get_many(struct obj_cgroup *objcg,
> > +				       unsigned long nr)
> > +{
> > +	percpu_ref_get_many(&objcg->refcnt, nr);
> > +}
> > +
> >  static inline void obj_cgroup_put(struct obj_cgroup *objcg)
> >  {
> >  	percpu_ref_put(&objcg->refcnt);
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index c0b83a396299..64ada9e650a5 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -3133,7 +3133,11 @@ void split_page_memcg(struct page *head, unsigned int nr)
> >  
> >  	for (i = 1; i < nr; i++)
> >  		head[i].memcg_data = head->memcg_data;
> > -	css_get_many(&memcg->css, nr - 1);
> > +
> > +	if (PageMemcgKmem(head))
> > +		obj_cgroup_get_many(__page_objcg(head), nr - 1);
> > +	else
> > +		css_get_many(&memcg->css, nr - 1);
> >  }
> >  
> >  #ifdef CONFIG_MEMCG_SWAP
> > 
>