linux-kernel - Re: [PATCH v6 3/4] perf-stat: enable counting events for BPF programs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 29 Dec 2020 16:23:47 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     Song Liu <songliubraving@...com>
Cc:     lkml <linux-kernel@...r.kernel.org>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "alexander.shishkin@...ux.intel.com" 
        <alexander.shishkin@...ux.intel.com>,
        "namhyung@...nel.org" <namhyung@...nel.org>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "jolsa@...hat.com" <jolsa@...hat.com>,
        Kernel Team <Kernel-team@...com>
Subject: Re: [PATCH v6 3/4] perf-stat: enable counting events for BPF programs

Em Tue, Dec 29, 2020 at 04:18:48PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Dec 29, 2020 at 07:11:12PM +0000, Song Liu escreveu:
> > > On Dec 29, 2020, at 10:48 AM, Arnaldo Carvalho de Melo <acme@...nel.org> wrote:
> > > I'll check this one to get a patch that at least moves the needle here,
> > > i.e. probably we can leave supporting bpf counters in the python binding
> > > for a later step.

> > Thanks Arnaldo!

> > Currently, I have:
> > 1. Fixed issues highlighted by Namhyung;
> > 2. Merged 3/4 and 4/4;
> > 3. NOT found segfault;
> > 4. NOT fixed python import perf. 

> > I don't have good ideas with 3 and 4... Shall I send current code as v7?

> For 4, please fold the patch below into the relevant patch, we don't
> need bpf_counter.h included in util/evsel.h, you even added a forward
> declaration for that 'struct bpf_counter_ops'.
 
> And in general we should refrain from adding extra includes to header
> files, .h-ell is not good.
> 
> Then we provide a stub for that bpf_counter__destroy() so that
> util/evsel.o when linked into the perf python biding find it there,
> links ok.

Ok, one more stub is needed, I wasn't building all the time with 

  $ make BUILD_BPF_SKEL=1

Ditch the previous patch please, use the one below instead:

- Arnaldo

diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 40e3946cd7518113..8226b1fefa8cf2a3 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -10,7 +10,6 @@
 #include <internal/evsel.h>
 #include <perf/evsel.h>
 #include "symbol_conf.h"
-#include "bpf_counter.h"
 #include <internal/cpumap.h>
 
 struct bpf_object;
diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index cc5ade85a33fc999..278abecb5bdfc0d2 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -79,6 +79,27 @@ int metricgroup__copy_metric_events(struct evlist *evlist, struct cgroup *cgrp,
 	return 0;
 }
 
+/*
+ * XXX: All these evsel destructors need some better mechanism, like a linked
+ * list of destructors registered when the relevant code indeed is used instead
+ * of having more and more calls in perf_evsel__delete(). -- acme
+ *
+ * For now, add some more:
+ *
+ * Not to drag the BPF bandwagon...
+ */
+void bpf_counter__destroy(struct evsel *evsel);
+int bpf_counter__install_pe(struct evsel *evsel, int cpu, int fd);
+
+void bpf_counter__destroy(struct evsel *evsel __maybe_unused)
+{
+}
+
+int bpf_counter__install_pe(struct evsel *evsel __maybe_unused, int cpu __maybe_unused, int fd __maybe_unused)
+{
+	return 0;
+}
+
 /*
  * Support debug printing even though util/debug.c is not linked.  That means
  * implementing 'verbose' and 'eprintf'.