Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 10 Apr 2023 22:02:25 -0300
From:   Arnaldo Carvalho de Melo <acme@kernel.org>
To:     Namhyung Kim <namhyung@kernel.org>
Cc:     Ian Rogers <irogers@google.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Jiri Olsa <jolsa@kernel.org>,
        Adrian Hunter <adrian.hunter@intel.com>,
        Kan Liang <kan.liang@linux.intel.com>,
        Florian Fischer <florian.fischer@muhq.space>,
        linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1] perf evsel: Avoid segv if delete is called on NULL
Message-ID: <ZDSxoUqxij2yJjXX@kernel.org>
References: <20230410205659.3131608-1-irogers@google.com>
 <CAM9d7cjuqcwcy+CwAPj8wK4hO2Pzr6xor76+jW-Do443Gr2ENQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAM9d7cjuqcwcy+CwAPj8wK4hO2Pzr6xor76+jW-Do443Gr2ENQ@mail.gmail.com>
Precedence: bulk

Em Mon, Apr 10, 2023 at 04:20:57PM -0700, Namhyung Kim escreveu:
> Hi Ian,
> 
> On Mon, Apr 10, 2023 at 1:57 PM Ian Rogers <irogers@google.com> wrote:
> >
> > Seen in "perf stat --bpf-counters --for-each-cgroup test" running in a
> > container:
> >
> > libbpf: Failed to bump RLIMIT_MEMLOCK (err = -1), you might need to do it explicitly!
> > libbpf: Error in bpf_object__probe_loading():Operation not permitted(1). Couldn't load trivial BPF program. Make sure your kernel supports BPF (CONFIG_BPF_SYSCALL=y) and/or that RLIMIT_MEMLOCK is set to big enough value.
> > libbpf: failed to load object 'bperf_cgroup_bpf'
> > libbpf: failed to load BPF skeleton 'bperf_cgroup_bpf': -1
> > Failed to load cgroup skeleton
> >
> >     #0 0x55f28a650981 in list_empty tools/include/linux/list.h:189
> >     #1 0x55f28a6593b4 in evsel__exit util/evsel.c:1518
> >     #2 0x55f28a6596af in evsel__delete util/evsel.c:1544
> >     #3 0x55f28a89d166 in bperf_cgrp__destroy util/bpf_counter_cgroup.c:283
> >     #4 0x55f28a899e9a in bpf_counter__destroy util/bpf_counter.c:816
> >     #5 0x55f28a659455 in evsel__exit util/evsel.c:1520
> >     #6 0x55f28a6596af in evsel__delete util/evsel.c:1544
> >     #7 0x55f28a640d4d in evlist__purge util/evlist.c:148
> >     #8 0x55f28a640ea6 in evlist__delete util/evlist.c:169
> >     #9 0x55f28a4efbf2 in cmd_stat tools/perf/builtin-stat.c:2598
> >     #10 0x55f28a6050c2 in run_builtin tools/perf/perf.c:330
> >     #11 0x55f28a605633 in handle_internal_command tools/perf/perf.c:384
> >     #12 0x55f28a6059fb in run_argv tools/perf/perf.c:428
> >     #13 0x55f28a6061d3 in main tools/perf/perf.c:562
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> 
> Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks, applied.

- Arnaldo

 
> In addition to this, I think bperf code should clear the evsel->bpf_skel
> at the end of the bpf_counter__destroy() to avoid confusion with the
> bpf_filter as they share the fields in a union.
> 
> Thanks,
> Namhyung
> 
> 
> > ---
> >  tools/perf/util/evsel.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> > index dc3faf005c3b..fe3ce765a4f3 100644
> > --- a/tools/perf/util/evsel.c
> > +++ b/tools/perf/util/evsel.c
> > @@ -1541,6 +1541,9 @@ void evsel__exit(struct evsel *evsel)
> >
> >  void evsel__delete(struct evsel *evsel)
> >  {
> > +       if (!evsel)
> > +               return;
> > +
> >         evsel__exit(evsel);
> >         free(evsel);
> >  }
> > --
> > 2.40.0.577.gac1e443424-goog
> >

-- 

- Arnaldo