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:18:48 -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 07:11:12PM +0000, Song Liu escreveu:
> 
> 
> > On Dec 29, 2020, at 10:48 AM, Arnaldo Carvalho de Melo <acme@...nel.org> 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@...nel.org> 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@...nel.org> 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@...alhost ~]# ~/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@...e ~]# 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@...e ~]#
> >>>>> [root@...e ~]# 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@...e perf]$ clang -v |& head -2
> >>>>> clang version 11.0.0 (Fedora 11.0.0-2.fc33)
> >>>>> Target: x86_64-unknown-linux-gnu
> >>>>> [acme@...e 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@...e perf]# export PYTHONPATH=/tmp/build/perf/python
> >>> [root@...e 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@...e perf]# perf test python
> >>> 19: 'import perf' in python                                         : FAILED!
> >>> [root@...e 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@...e 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'.

Powered by blists - more mailing lists