Re: FIX Re: [PATCH v7 3/3] perf-stat: enable counting events for BPF programs

[Song Liu <songliubraving@fb.com>]

> On Jan 20, 2021, at 8:30 AM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> 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.

Thanks Arnaldo!

Yes, my system have same online and possible CPUs. 

Since possible_cpu >= online_cpu, maybe we can use num_cpu_bpf in 
bpf_program_profiler__read() without he extra check? 

Song

[...]