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: <20211018075623.26884-1-quanyang.wang@windriver.com>
 <YXIUMJWrXUcrvZf5@carbon.DHCP.thefacebook.com> <35e9e89f-d92f-06f9-b919-ef956d99d7df@windriver.com>
In-Reply-To: <35e9e89f-d92f-06f9-b919-ef956d99d7df@windriver.com>
From:   Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date:   Fri, 22 Oct 2021 14:29:24 -0700
Message-ID: <CAADnVQJANZQJ5pAeH5KkC2rtY1TDFA_AomDfaPaBY_hw8qhgSA@mail.gmail.com>
Subject: Re: [V2][PATCH] cgroup: fix memory leak caused by missing cgroup_bpf_offline
To:     Quanyang Wang <quanyang.wang@windriver.com>
Cc:     Roman Gushchin <guro@fb.com>, Tejun Heo <tj@kernel.org>,
        Zefan Li <lizefan.x@bytedance.com>,
        Johannes Weiner <hannes@cmpxchg.org>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Jens Axboe <axboe@kernel.dk>, Ming Lei <ming.lei@redhat.com>,
        mkoutny@suse.com,
        "open list:CONTROL GROUP (CGROUP)" <cgroups@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Network Development <netdev@vger.kernel.org>,
        bpf <bpf@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Oct 22, 2021 at 4:56 AM Quanyang Wang
<quanyang.wang@windriver.com> wrote:
> >> Fixes: 2b0d3d3e4fcf ("percpu_ref: reduce memory footprint of percpu_ref in fast path")
> >> Fixes: 4bfc0bb2c60e ("bpf: decouple the lifetime of cgroup_bpf from cgroup itself")
> >> Signed-off-by: Quanyang Wang <quanyang.wang@windriver.com>
> >> ---
> >> V1 ---> V2:
> >> 1. As per Daniel's suggestion, add description to commit msg about the
> >> balance of cgroup's refcount in cgroup_bpf_offline.
> >> 2. As per Michal's suggestion, add tag "Fixes: 4bfc0bb2c60e" and add
> >> description about it.
> >> 3. Fix indentation on the percpu_ref_is_dying line.
> >
> > Acked-by: Roman Gushchin <guro@fb.com>
> >
> > The fix looks correct, two fixes tag are fine too, if only it won't
> > confuse scripts picking up patches for stable backports.
> >
> > In fact, it's a very cold path, which is arguably never hit in the real
> > life. On cgroup v2 it's not an issue. I'm not sure we need a stable
> > backport at all, only if it creates a noise for some automation tests.
> >
> > Quanyang, out of curiosity, how did you find it?
> I ran ltp testsuite to find this.
>
> ./runltp -f controllers -s cgroup
>
> Thanks,
> Quanyang
> >
> > Anyway, thanks for catching and fixing it!

Applied to bpf tree. Thanks everyone!