From: Song Liu <songliubraving@fb.com>
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: lkml <linux-kernel@vger.kernel.org>,
"peterz@infradead.org" <peterz@infradead.org>,
"mingo@redhat.com" <mingo@redhat.com>,
"alexander.shishkin@linux.intel.com"
<alexander.shishkin@linux.intel.com>,
"namhyung@kernel.org" <namhyung@kernel.org>,
"mark.rutland@arm.com" <mark.rutland@arm.com>,
"jolsa@redhat.com" <jolsa@redhat.com>,
Kernel Team <Kernel-team@fb.com>
Subject: Re: [PATCH v6 3/4] perf-stat: enable counting events for BPF programs
Date: Tue, 29 Dec 2020 21:40:55 +0000 [thread overview]
Message-ID: <83C9733B-9F8A-4C28-AA53-487FFCDDB4B4@fb.com> (raw)
In-Reply-To: <20201229193243.GN521329@kernel.org>
> On Dec 29, 2020, at 11:32 AM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> Em Tue, Dec 29, 2020 at 04:23:47PM -0300, Arnaldo Carvalho de Melo escreveu:
>> 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@kernel.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.
>
> For 3, now with a kprobe:
>
> [root@five ~]# bpftool prog | grep hrtimer -A10
> 99: kprobe name hrtimer_nanosle tag 0e77bacaf4555f83 gpl
> loaded_at 2020-12-29T16:25:34-0300 uid 0
> xlated 80B jited 49B memlock 4096B
> btf_id 253
> [root@five ~]# perf stat -I 1000 -b 99
> 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
> 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
> 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
> 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
> 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
> Segmentation fault (core dumped)
> [root@five ~]#
>
> (gdb) run stat -I 1000 -b 99
> Starting program: /root/bin/perf stat -I 1000 -b 99
> Missing separate debuginfos, use: dnf debuginfo-install glibc-2.32-2.fc33.x86_64
> [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
> 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
> 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
> 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
> 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.
> 0x000000000056559b in bpf_program_profiler__read (evsel=0xc39770) at util/bpf_counter.c:217
> 217 reading_map_fd = bpf_map__fd(skel->maps.accum_readings);
> Missing separate debuginfos, use: dnf debuginfo-install bzip2-libs-1.0.8-4.fc33.x86_64 cyrus-sasl-lib-2.1.27-6.fc33.x86_64 elfutils-debuginfod-client-0.182-1.fc33.x86_64 elfutils-libelf-0.182-1.fc33.x86_64 elfutils-libs-0.182-1.fc33.x86_64 keyutils-libs-1.6-5.fc33.x86_64 krb5-libs-1.18.2-29.fc33.x86_64 libbabeltrace-1.5.8-3.fc33.x86_64 libbrotli-1.0.9-3.fc33.x86_64 libcap-2.26-8.fc33.x86_64 libcom_err-1.45.6-4.fc33.x86_64 libcurl-7.71.1-8.fc33.x86_64 libgcc-10.2.1-9.fc33.x86_64 libidn2-2.3.0-4.fc33.x86_64 libnghttp2-1.41.0-3.fc33.x86_64 libpsl-0.21.1-2.fc33.x86_64 libselinux-3.1-2.fc33.x86_64 libssh-0.9.5-1.fc33.x86_64 libunistring-0.9.10-9.fc33.x86_64 libunwind-1.4.0-4.fc33.x86_64 libuuid-2.36-3.fc33.x86_64 libxcrypt-4.4.17-1.fc33.x86_64 libzstd-1.4.5-5.fc33.x86_64 numactl-libs-2.0.14-1.fc33.x86_64 openldap-2.4.50-5.fc33.x86_64 openssl-libs-1.1.1i-1.fc33.x86_64 pcre-8.44-2.fc33.x86_64 pcre2-10.36-1.fc33.x86_64 perl-libs-5.32.0-464.fc33.x86_64 popt-1.18-2.fc33.x86_64 python3-libs-3.9.1-1.fc33.x86_64 slang-2.3.2-8.fc33.x86_64 xz-libs-5.2.5-3.fc33.x86_64
> (gdb) bt
> #0 0x000000000056559b in bpf_program_profiler__read (evsel=0xc39770) at util/bpf_counter.c:217
> #1 0x0000000000000000 in ?? ()
> (gdb) p skel->maps.accum_readings
> Cannot access memory at address 0x20
> (gdb) p skel
> $1 = (struct bpf_prog_profiler_bpf *) 0x0
> (gdb) list -10
> 202 int reading_map_fd;
> 203 __u32 key = 0;
> 204 int err, cpu;
> 205
> 206 if (list_empty(&evsel->bpf_counter_list))
> 207 return -EAGAIN;
> 208
> 209 for (cpu = 0; cpu < num_cpu; cpu++) {
> 210 perf_counts(evsel->counts, cpu, 0)->val = 0;
> 211 perf_counts(evsel->counts, cpu, 0)->ena = 0;
> (gdb)
> 212 perf_counts(evsel->counts, cpu, 0)->run = 0;
> 213 }
> 214 list_for_each_entry(counter, &evsel->bpf_counter_list, list) {
> 215 struct bpf_prog_profiler_bpf *skel = counter->skel;
> 216
> 217 reading_map_fd = bpf_map__fd(skel->maps.accum_readings);
> 218
> 219 err = bpf_map_lookup_elem(reading_map_fd, &key, values);
> 220 if (err) {
> 221 fprintf(stderr, "failed to read value\n");
> (gdb) p counter->skel
> $2 = (void *) 0x0
> (gdb) p perf_evsel__name(counter)
> No symbol "perf_evsel__name" in current context.
> (gdb) p evsel__name(counter)
> $3 = 0xc77420 "unknown attr type: 13078424"
> (gdb) p evsel->type
> There is no member named type.
> (gdb) p evsel->core.
> attr cpus fd id ids node nr_members own_cpus sample_id system_wide threads
> (gdb) p evsel->core.attr.type
> $4 = 1
> (gdb) p evsel->core.attr.config
> $5 = 0
> (gdb) p evsel->evlist
> $6 = (struct evlist *) 0xc3cfd0
> (gdb) p evsel->evlist->core.nr_entries
> $7 = 10
> (gdb)
>
>
> 10 entries, the default for 'perf stat'
>
>
> With just one event:
>
> (gdb) run stat -e cycles -I 1000 -b 99
> Starting program: /root/bin/perf stat -e cycles -I 1000 -b 99
> Missing separate debuginfos, use: dnf debuginfo-install glibc-2.32-2.fc33.x86_64
> [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
>
> Program received signal SIGSEGV, Segmentation fault.
> 0x000000000056559b in bpf_program_profiler__read (evsel=0xc392c0) at util/bpf_counter.c:217
> 217 reading_map_fd = bpf_map__fd(skel->maps.accum_readings);
> Missing separate debuginfos, use: dnf debuginfo-install bzip2-libs-1.0.8-4.fc33.x86_64 cyrus-sasl-lib-2.1.27-6.fc33.x86_64 elfutils-debuginfod-client-0.182-1.fc33.x86_64 elfutils-libelf-0.182-1.fc33.x86_64 elfutils-libs-0.182-1.fc33.x86_64 keyutils-libs-1.6-5.fc33.x86_64 krb5-libs-1.18.2-29.fc33.x86_64 libbabeltrace-1.5.8-3.fc33.x86_64 libbrotli-1.0.9-3.fc33.x86_64 libcap-2.26-8.fc33.x86_64 libcom_err-1.45.6-4.fc33.x86_64 libcurl-7.71.1-8.fc33.x86_64 libgcc-10.2.1-9.fc33.x86_64 libidn2-2.3.0-4.fc33.x86_64 libnghttp2-1.41.0-3.fc33.x86_64 libpsl-0.21.1-2.fc33.x86_64 libselinux-3.1-2.fc33.x86_64 libssh-0.9.5-1.fc33.x86_64 libunistring-0.9.10-9.fc33.x86_64 libunwind-1.4.0-4.fc33.x86_64 libuuid-2.36-3.fc33.x86_64 libxcrypt-4.4.17-1.fc33.x86_64 libzstd-1.4.5-5.fc33.x86_64 numactl-libs-2.0.14-1.fc33.x86_64 openldap-2.4.50-5.fc33.x86_64 openssl-libs-1.1.1i-1.fc33.x86_64 pcre-8.44-2.fc33.x86_64 pcre2-10.36-1.fc33.x86_64 perl-libs-5.32.0-464.fc33.x86_64 popt-1.18-2.fc33.x86_64 python3-libs-3.9.1-1.fc33.x86_64 slang-2.3.2-8.fc33.x86_64 xz-libs-5.2.5-3.fc33.x86_64
> (gdb) bt
> #0 0x000000000056559b in bpf_program_profiler__read (evsel=0xc392c0) at util/bpf_counter.c:217
> #1 0x0000000000000000 in ?? ()
> (gdb) p evsel->name
> $1 = 0xc37960 "cycles"
> (gdb) p evsel->bpf_counter_
> bpf_counter_list bpf_counter_ops
> (gdb) p evsel->bpf_counter_ops
> $2 = (struct bpf_counter_ops *) 0xa08ec0 <bpf_program_profiler_ops>
> (gdb) p evsel->bpf_counter_
> bpf_counter_list bpf_counter_ops
> (gdb) p evsel->bpf_counter_list
> $3 = {next = 0xc36e18, prev = 0xc36e18}
> (gdb) p evsel->s
> sample_size side_band stats supported synth_sample_type
> (gdb) list -5
> 207 return -EAGAIN;
> 208
> 209 for (cpu = 0; cpu < num_cpu; cpu++) {
> 210 perf_counts(evsel->counts, cpu, 0)->val = 0;
> 211 perf_counts(evsel->counts, cpu, 0)->ena = 0;
> 212 perf_counts(evsel->counts, cpu, 0)->run = 0;
> 213 }
> 214 list_for_each_entry(counter, &evsel->bpf_counter_list, list) {
> 215 struct bpf_prog_profiler_bpf *skel = counter->skel;
> 216
> (gdb)
> 217 reading_map_fd = bpf_map__fd(skel->maps.accum_readings);
> 218
> 219 err = bpf_map_lookup_elem(reading_map_fd, &key, values);
> 220 if (err) {
> 221 fprintf(stderr, "failed to read value\n");
> 222 return err;
> 223 }
> 224
> 225 for (cpu = 0; cpu < num_cpu; cpu++) {
> 226 perf_counts(evsel->counts, cpu, 0)->val += values[cpu].counter;
> (gdb) p counter->skel
> $4 = (void *) 0x0
> (gdb)
>
> skel is NULL?!
So it is skel == NULL. In v7 (coming soon), I fixed some issues in the
allocate/free of skel, and added some assert(). Let's see how that goes..
Thanks,
Song
>
> I ran out of time, have to go errands now. will bbl.
>
> - Arnaldo
>
>>>> 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'.
>
> --
>
> - Arnaldo
next prev parent reply other threads:[~2020-12-29 21:42 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-28 17:40 [PATCH v6 0/4] Introduce perf-stat -b for BPF programs Song Liu
2020-12-28 17:40 ` [PATCH v6 1/4] bpftool: add Makefile target bootstrap Song Liu
2020-12-28 17:40 ` [PATCH v6 2/4] perf: support build BPF skeletons with perf Song Liu
2020-12-29 7:01 ` Namhyung Kim
2020-12-29 11:48 ` Arnaldo Carvalho de Melo
2020-12-29 17:14 ` Song Liu
2020-12-29 18:16 ` Arnaldo Carvalho de Melo
2020-12-28 17:40 ` [PATCH v6 3/4] perf-stat: enable counting events for BPF programs Song Liu
2020-12-28 20:11 ` Arnaldo Carvalho de Melo
2020-12-28 23:43 ` Song Liu
2020-12-29 5:53 ` Song Liu
2020-12-29 15:15 ` Arnaldo Carvalho de Melo
2020-12-29 18:42 ` Song Liu
2020-12-29 18:48 ` Arnaldo Carvalho de Melo
2020-12-29 19:11 ` Song Liu
2020-12-29 19:18 ` Arnaldo Carvalho de Melo
2020-12-29 19:23 ` Arnaldo Carvalho de Melo
2020-12-29 19:32 ` Arnaldo Carvalho de Melo
2020-12-29 21:40 ` Song Liu [this message]
2020-12-29 7:22 ` Namhyung Kim
2020-12-29 17:46 ` Song Liu
2020-12-29 17:59 ` Song Liu
2020-12-28 17:40 ` [PATCH v6 4/4] perf-stat: add documentation for -b option Song Liu
2020-12-29 7:24 ` Namhyung Kim
2020-12-29 16:59 ` Song Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=83C9733B-9F8A-4C28-AA53-487FFCDDB4B4@fb.com \
--to=songliubraving@fb.com \
--cc=Kernel-team@fb.com \
--cc=acme@kernel.org \
--cc=alexander.shishkin@linux.intel.com \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.