Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210413065153.63431-1-songmuchun@bytedance.com>
 <20210413065153.63431-3-songmuchun@bytedance.com> <YHa00lx7ACWcS1/h@dhcp22.suse.cz>
In-Reply-To: <YHa00lx7ACWcS1/h@dhcp22.suse.cz>
From:   Muchun Song <songmuchun@bytedance.com>
Date:   Wed, 14 Apr 2021 18:04:35 +0800
Message-ID: <CAMZfGtVpUW1=Zvys7J=gW1xqkuUVLuPYcQbwJoe=TEkBa6qqQw@mail.gmail.com>
Subject: Re: [External] Re: [PATCH 2/7] mm: memcontrol: bail out early when
 !mm in get_mem_cgroup_from_mm
To:     Michal Hocko <mhocko@suse.com>
Cc:     guro@fb.com, hannes@cmpxchg.org, akpm@linux-foundation.org,
        shakeelb@google.com, vdavydov.dev@gmail.com,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        duanxiongchun@bytedance.com, fam.zheng@bytedance.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Apr 14, 2021 at 5:24 PM Michal Hocko <mhocko@suse.com> wrote:
>
> On Tue 13-04-21 14:51:48, Muchun Song wrote:
> > When mm is NULL, we do not need to hold rcu lock and call css_tryget for
> > the root memcg. And we also do not need to check !mm in every loop of
> > while. So bail out early when !mm.
>
> mem_cgroup_charge and other callers unconditionally drop the reference
> so how come this does not underflow reference count?

For the root memcg, the CSS_NO_REF flag is set, so css_get
and css_put do not get or put reference.

Thanks.


>
> > Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> > Acked-by: Johannes Weiner <hannes@cmpxchg.org>
> > Reviewed-by: Shakeel Butt <shakeelb@google.com>
> > ---
> >  mm/memcontrol.c | 21 ++++++++++-----------
> >  1 file changed, 10 insertions(+), 11 deletions(-)
> >
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index f229de925aa5..9cbfff59b171 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -901,20 +901,19 @@ struct mem_cgroup *get_mem_cgroup_from_mm(struct mm_struct *mm)
> >       if (mem_cgroup_disabled())
> >               return NULL;
> >
> > +     /*
> > +      * Page cache insertions can happen without an
> > +      * actual mm context, e.g. during disk probing
> > +      * on boot, loopback IO, acct() writes etc.
> > +      */
> > +     if (unlikely(!mm))
> > +             return root_mem_cgroup;
> > +
> >       rcu_read_lock();
> >       do {
> > -             /*
> > -              * Page cache insertions can happen without an
> > -              * actual mm context, e.g. during disk probing
> > -              * on boot, loopback IO, acct() writes etc.
> > -              */
> > -             if (unlikely(!mm))
> > +             memcg = mem_cgroup_from_task(rcu_dereference(mm->owner));
> > +             if (unlikely(!memcg))
> >                       memcg = root_mem_cgroup;
> > -             else {
> > -                     memcg = mem_cgroup_from_task(rcu_dereference(mm->owner));
> > -                     if (unlikely(!memcg))
> > -                             memcg = root_mem_cgroup;
> > -             }
> >       } while (!css_tryget(&memcg->css));
> >       rcu_read_unlock();
> >       return memcg;
> > --
> > 2.11.0
>
> --
> Michal Hocko
> SUSE Labs