From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Song Liu <songliubraving@fb.com>
Cc: open list <linux-kernel@vger.kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Namhyung Kim <namhyung@kernel.org>,
Mark Rutland <mark.rutland@arm.com>, Jiri Olsa <jolsa@redhat.com>,
Kernel Team <Kernel-team@fb.com>
Subject: FIX Re: [PATCH v7 3/3] perf-stat: enable counting events for BPF programs
Date: Wed, 20 Jan 2021 13:30:31 -0300 [thread overview]
Message-ID: <20210120163031.GU12699@kernel.org> (raw)
In-Reply-To: <20210120135013.GT12699@kernel.org>
Em Wed, Jan 20, 2021 at 10:50:13AM -0300, Arnaldo Carvalho de Melo escreveu:
> So sizeof(struct bpf_perf_event_value) == 24 and it is a per-cpu array, the
> machine has 24 cpus, why is the kernel thinking it has more and end up zeroing
> entries after the 24 cores? Some percpu map subtlety (or obvious thing ;-\) I'm
> missing?
>
> Checking lookups into per cpu maps in sample code now...
(gdb) run stat -b 244 -I 1000 -e cycles
Starting program: /root/bin/perf stat -b 244 -I 1000 -e cycles
[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
Breakpoint 1, bpf_program_profiler__read (evsel=0xce02c0) at util/bpf_counter.c:217
217 if (list_empty(&evsel->bpf_counter_list))
(gdb) p num_
num_cpu num_groups num_leaps num_print_iv num_stmts num_transitions num_warnings_issued
num_cpu_bpf num_ifs num_print_interval num_srcfiles num_to_str num_types
(gdb) p num_cpu
$1 = 24
(gdb) p num_cpu_bpf
$2 = 32
(gdb)
Humm, why?
But then libbpf and the sample/bpf/ code use it this way:
diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.c
index 8c977f038f497fc1..7dd3d57aba4f620c 100644
--- a/tools/perf/util/bpf_counter.c
+++ b/tools/perf/util/bpf_counter.c
@@ -207,7 +207,8 @@ static int bpf_program_profiler__enable(struct evsel *evsel)
static int bpf_program_profiler__read(struct evsel *evsel)
{
int num_cpu = evsel__nr_cpus(evsel);
- struct bpf_perf_event_value values[num_cpu];
+ int num_cpu_bpf = libbpf_num_possible_cpus();
+ struct bpf_perf_event_value values[num_cpu > num_cpu_bpf ? num_cpu : num_cpu_bpf];
struct bpf_counter *counter;
int reading_map_fd;
__u32 key = 0;
-------------------------------------------------------------
[root@five ~]# cat /sys/devices/system/cpu/possible
0-31
[root@five ~]#
I bet that in your test systems evsel__nr_cpus(evsel) matches
/sys/devices/system/cpu/possible and thus you don't see the problem.
evsel__nr_cpus(evsel) uses what is in:
[acme@five perf]$ cat /sys/devices/system/cpu/online
0-23
[acme@five perf]$
So that is the reason for the problem and the fix is to use
libbpf_num_possible_cpus(), I'll bolt that into the patch that
introduced that code.
- Arnaldo
next prev parent reply other threads:[~2021-01-20 16:35 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-29 21:42 [PATCH v7 0/3] Introduce perf-stat -b for BPF programs Song Liu
2020-12-29 21:42 ` [PATCH v7 1/3] bpftool: add Makefile target bootstrap Song Liu
2020-12-29 21:42 ` [PATCH v7 2/3] perf: support build BPF skeletons with perf Song Liu
2020-12-29 21:42 ` [PATCH v7 3/3] perf-stat: enable counting events for BPF programs Song Liu
2021-01-12 7:35 ` Namhyung Kim
2021-01-15 18:53 ` Arnaldo Carvalho de Melo
2021-01-18 19:38 ` Arnaldo Carvalho de Melo
2021-01-19 0:48 ` Song Liu
2021-01-19 14:31 ` Arnaldo Carvalho de Melo
2021-01-19 14:42 ` Arnaldo Carvalho de Melo
2021-01-19 16:31 ` Arnaldo Carvalho de Melo
2021-01-19 21:54 ` Song Liu
2021-01-19 22:30 ` Arnaldo Carvalho de Melo
2021-01-20 12:37 ` Arnaldo Carvalho de Melo
2021-01-20 13:01 ` Arnaldo Carvalho de Melo
2021-01-20 13:50 ` Arnaldo Carvalho de Melo
2021-01-20 16:30 ` Arnaldo Carvalho de Melo [this message]
2021-01-20 16:40 ` FIX " Song Liu
2021-01-20 17:04 ` Arnaldo Carvalho de Melo
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=20210120163031.GU12699@kernel.org \
--to=acme@kernel.org \
--cc=Kernel-team@fb.com \
--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 \
--cc=songliubraving@fb.com \
/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.