Re: [PATCH v6 3/4] perf-stat: enable counting events for BPF programs
From: Arnaldo Carvalho de Melo
Date: Tue Dec 29 2020 - 14:19:58 EST
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@xxxxxxxxxx> wrote:
> >
> > Em Tue, Dec 29, 2020 at 06:42:18PM +0000, Song Liu escreveu:
> >>
> >>
> >>> On Dec 29, 2020, at 7:15 AM, Arnaldo Carvalho de Melo <acme@xxxxxxxxxx> wrote:
> >>>
> >>> Em Mon, Dec 28, 2020 at 11:43:25PM +0000, Song Liu escreveu:
> >>>>
> >>>>
> >>>>> On Dec 28, 2020, at 12:11 PM, Arnaldo Carvalho de Melo <acme@xxxxxxxxxx> wrote:
> >>>>>
> >>>>> Em Mon, Dec 28, 2020 at 09:40:53AM -0800, Song Liu escreveu:
> >>>>>> Introduce perf-stat -b option, which counts events for BPF programs, like:
> >>>>>>
> >>>>>> [root@localhost ~]# ~/perf stat -e ref-cycles,cycles -b 254 -I 1000
> >>>>>> 1.487903822 115,200 ref-cycles
> >>>>>> 1.487903822 86,012 cycles
> >>>>>> 2.489147029 80,560 ref-cycles
> >>>>>> 2.489147029 73,784 cycles
> >>>>>> 3.490341825 60,720 ref-cycles
> >>>>>> 3.490341825 37,797 cycles
> >>>>>> 4.491540887 37,120 ref-cycles
> >>>>>> 4.491540887 31,963 cycles
> >>>>>>
> >>>>>> The example above counts cycles and ref-cycles of BPF program of id 254.
> >>>>>> This is similar to bpftool-prog-profile command, but more flexible.
> >>>>>>
> >>>>>> perf-stat -b creates per-cpu perf_event and loads fentry/fexit BPF
> >>>>>> programs (monitor-progs) to the target BPF program (target-prog). The
> >>>>>> monitor-progs read perf_event before and after the target-prog, and
> >>>>>> aggregate the difference in a BPF map. Then the user space reads data
> >>>>>> from these maps.
> >>>>>>
> >>>>>> A new struct bpf_counter is introduced to provide common interface that
> >>>>>> uses BPF programs/maps to count perf events.
> >>>>>
> >>>>> Segfaulting here:
> >>>>>
> >>>>> [root@five ~]# bpftool prog | grep tracepoint
> >>>>> 110: tracepoint name syscall_unaugme tag 57cd311f2e27366b gpl
> >>>>> 111: tracepoint name sys_enter_conne tag 3555418ac9476139 gpl
> >>>>> 112: tracepoint name sys_enter_sendt tag bc7fcadbaf7b8145 gpl
> >>>>> 113: tracepoint name sys_enter_open tag 0e59c3ac2bea5280 gpl
> >>>>> 114: tracepoint name sys_enter_opena tag 0baf443610f59837 gpl
> >>>>> 115: tracepoint name sys_enter_renam tag 24664e4aca62d7fa gpl
> >>>>> 116: tracepoint name sys_enter_renam tag 20093e51a8634ebb gpl
> >>>>> 117: tracepoint name sys_enter tag 0bc3fc9d11754ba1 gpl
> >>>>> 118: tracepoint name sys_exit tag 29c7ae234d79bd5c gpl
> >>>>> [root@five ~]#
> >>>>> [root@five ~]# gdb perf
> >>>>> GNU gdb (GDB) Fedora 10.1-2.fc33
> >>>>> Reading symbols from perf...
> >>>>> (gdb) run stat -e instructions,cycles -b 113 -I 1000
> >>>>> Starting program: /root/bin/perf stat -e instructions,cycles -b 113 -I 1000
> >>>>> [Thread debugging using libthread_db enabled]
> >>>>> Using host libthread_db library "/lib64/libthread_db.so.1".
> >>>>> libbpf: elf: skipping unrecognized data section(9) .eh_frame
> >>>>> libbpf: elf: skipping relo section(15) .rel.eh_frame for section(9) .eh_frame
> >>>>> libbpf: elf: skipping unrecognized data section(9) .eh_frame
> >>>>> libbpf: elf: skipping relo section(15) .rel.eh_frame for section(9) .eh_frame
> >>>>>
> >>>>> Program received signal SIGSEGV, Segmentation fault.
> >>>>> 0x000000000058d55b in bpf_program_profiler__read (evsel=0xc612c0) at util/bpf_counter.c:217
> >>>>> 217 reading_map_fd = bpf_map__fd(skel->maps.accum_readings);
> >>>>> (gdb) bt
> >>>>> #0 0x000000000058d55b in bpf_program_profiler__read (evsel=0xc612c0) at util/bpf_counter.c:217
> >>>>> #1 0x0000000000000000 in ?? ()
> >>>>> (gdb)
> >>>>>
> >>>>> [acme@five perf]$ clang -v |& head -2
> >>>>> clang version 11.0.0 (Fedora 11.0.0-2.fc33)
> >>>>> Target: x86_64-unknown-linux-gnu
> >>>>> [acme@five perf]$
> >>>>>
> >>>>> Do you need any extra info?
> >>>>
> >>>> Hmm... I am not able to reproduce this. I am trying to setup an environment similar
> >>>> to fc33 (clang 11, etc.). Does this segfault every time, and on all programs?
> >>>
> >>> I'll try it with a BPF proggie attached to a kprobes, but here is
> >>> something else I noticed:
> >>>
> >>> [root@five perf]# export PYTHONPATH=/tmp/build/perf/python
> >>> [root@five perf]# tools/perf/python/twatch.py
> >>> Traceback (most recent call last):
> >>> File "/home/acme/git/perf/tools/perf/python/twatch.py", line 9, in <module>
> >>> import perf
> >>> ImportError: /tmp/build/perf/python/perf.cpython-39-x86_64-linux-gnu.so: undefined symbol: bpf_counter__destroy
> >>> [root@five perf]# perf test python
> >>> 19: 'import perf' in python : FAILED!
> >>> [root@five perf]# perf test -v python
> >>> 19: 'import perf' in python :
> >>> --- start ---
> >>> test child forked, pid 3198864
> >>> python usage test: "echo "import sys ; sys.path.append('/tmp/build/perf/python'); import perf" | '/usr/bin/python3' "
> >>> Traceback (most recent call last):
> >>> File "<stdin>", line 1, in <module>
> >>> ImportError: /tmp/build/perf/python/perf.cpython-39-x86_64-linux-gnu.so: undefined symbol: bpf_counter__destroy
> >>> test child finished with -1
> >>> ---- end ----
> >>> 'import perf' in python: FAILED!
> >>> [root@five perf]#
> >>>
> >>> This should be trivial, I hope, just add the new object file to
> >>> tools/perf/util/python-ext-sources, then do a 'perf test python', if it
> >>> fails, use 'perf test -v python' to see what is preventing the python
> >>> binding from loading.
> >>
> >> I fixed the undefined bpf_counter__destroy. But this one looks trickier:
> >>
> >> 19: 'import perf' in python :
> >> --- start ---
> >> test child forked, pid 2714986
> >> python usage test: "echo "import sys ; sys.path.append('python'); import perf" | '/bin/python2' "
> >> Traceback (most recent call last):
> >> File "<stdin>", line 1, in <module>
> >> ImportError: XXXXX /tools/perf/python/perf.so: undefined symbol: bpf_map_update_elem
> >>
> >> Given I already have:
> >
> > 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.
As we don't have a way to create such events via the perf python
binding, there will nothing to be done when destroying evsels created
via python.
- 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..9609cc166d71a6f5 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -79,6 +79,21 @@ 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 one more:
+ *
+ * Not to drag the BPF bandwagon...
+ */
+void bpf_counter__destroy(struct evsel *evsel);
+
+void bpf_counter__destroy(struct evsel *evsel __maybe_unused)
+{
+}
+
/*
* Support debug printing even though util/debug.c is not linked. That means
* implementing 'verbose' and 'eprintf'.