Re: [PATCH v2] mm: memcg/slab: Fix return child memcg objcg for root memcg

[Roman Gushchin]

On Thu, Oct 29, 2020 at 01:34:57PM -0700, Shakeel Butt wrote:
> On Thu, Oct 29, 2020 at 10:10 AM Roman Gushchin <guro@xxxxxx> wrote:
> >
> > On Thu, Oct 29, 2020 at 08:48:45AM -0700, Shakeel Butt wrote:
> > > On Tue, Oct 27, 2020 at 8:50 PM Muchun Song <songmuchun@xxxxxxxxxxxxx> wrote:
> > > >
> > > > Consider the following memcg hierarchy.
> > > >
> > > >                     root
> > > >                    /    \
> > > >                   A      B
> > > >
> > > > If we get the objcg of memcg A failed,
> > >
> > > Please fix the above statement.
> > >
> > > > the get_obj_cgroup_from_current
> > > > can return the wrong objcg for the root memcg.
> > > >
> > > > Fixes: bf4f059954dc ("mm: memcg/slab: obj_cgroup API")
> > > > Signed-off-by: Muchun Song <songmuchun@xxxxxxxxxxxxx>
> > > > ---
> > > >  changelog in v2:
> > > >  1. Do not use a comparison with the root_mem_cgroup
> > > >
> > > >  mm/memcontrol.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > > > index 1337775b04f3..8c8b4c3ed5a0 100644
> > > > --- a/mm/memcontrol.c
> > > > +++ b/mm/memcontrol.c
> > > > @@ -2961,6 +2961,7 @@ __always_inline struct obj_cgroup *get_obj_cgroup_from_current(void)
> > > >                 objcg = rcu_dereference(memcg->objcg);
> > > >                 if (objcg && obj_cgroup_tryget(objcg))
> > > >                         break;
> > > > +               objcg = NULL;
> > >
> > > Roman, in your cleanup, are you planning to have objcg for root memcg as well?
> >
> > Yes. I'll just change the for loop to include the root_mem_cgroup.
> >
> 
> Then do we really need this patch since it's not tagged for stable?

A good question.

Honestly, I really doubt that obj_cgroup_tryget() can fail even with the current code.
But, formally speaking, it's possible. So there is likely no practical difference if
we'll apply this patch or not, even without the root handling cleanup.

But you're right, with the root handling cleanup, it will make even less of a difference.

Thanks!