Re: [PATCH v4 44/48] perf bpf: Rename cpu to cpu_map_idx
From: Arnaldo Carvalho de Melo
Date: Mon Jan 10 2022 - 14:10:53 EST
Em Tue, Jan 04, 2022 at 10:13:47PM -0800, Ian Rogers escreveu:
> Synchronize the caller in evsel with the called function.
> Shorten 3 lines of code in bperf_read by using
> perf_cpu_map__for_each_cpu.
> This code is frequently using variables named cpu as cpu map indices,
> which doesn't matter as all CPUs are in the CPU map. It is strange in
> some cases the cpumap is used at all.
CC /tmp/build/perf/builtin-stat.o
INSTALL trace_plugins
CC /tmp/build/perf/util/evlist.o
CC /tmp/build/perf/util/evsel.o
CC /tmp/build/perf/util/header.o
CC /tmp/build/perf/util/bpf_counter.o
CC /tmp/build/perf/util/bpf_counter_cgroup.o
CC /tmp/build/perf/util/bpf_ftrace.o
In file included from util/cpumap.h:8,
from util/bpf_counter.c:23:
util/bpf_counter.c: In function ‘bperf__read’:
/var/home/acme/git/perf/tools/lib/perf/include/perf/cpumap.h:27:20: error: comparison of integer expressions of different signedness: ‘__u32’ {aka ‘unsigned int’} and ‘int’ [-Werror=sign-compare]
27 | (idx) < perf_cpu_map__nr(cpus); \
| ^
util/bpf_counter.c:626:25: note: in expansion of macro ‘perf_cpu_map__for_each_cpu’
626 | perf_cpu_map__for_each_cpu(cpu, j, all_cpu_map) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
util/bpf_counter.c:611:21: error: unused variable ‘num_cpu’ [-Werror=unused-variable]
611 | __u32 i, j, num_cpu;
| ^~~~~~~
cc1: all warnings being treated as errors
make[4]: *** [/var/home/acme/git/perf/tools/build/Makefile.build:96: /tmp/build/perf/util/bpf_counter.o] Error 1
make[4]: *** Waiting for unfinished jobs....
make[3]: *** [/var/home/acme/git/perf/tools/build/Makefile.build:139: util] Error 2
make[2]: *** [Makefile.perf:665: /tmp/build/perf/perf-in.o] Error 2
make[1]: *** [Makefile.perf:240: sub-make] Error 2
make: *** [Makefile:113: install-bin] Error 2
make: Leaving directory '/var/home/acme/git/perf/tools/perf'
Performance counter stats for 'make -k BUILD_BPF_SKEL=1 CORESIGHT=1 PYTHON=python3 O=/tmp/build/perf -C tools/perf install-bin':
15,589,752,360 cycles:u
28,227,862,710 instructions:u # 1.81 insn per cycle
1.996638375 seconds time elapsed
3.567457000 seconds user
0.934840000 seconds sys
⬢[acme@toolbox perf]$ git log --oneline -1
0083ba4c6a931fd0 (HEAD) perf bpf: Rename cpu to cpu_map_idx
⬢[acme@toolbox perf]$
Trying to fix...
> Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
> ---
> tools/perf/util/bpf_counter.c | 16 +++++++---------
> tools/perf/util/bpf_counter.h | 4 ++--
> 2 files changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.c
> index c17d4a43ce06..ae5bd87ff020 100644
> --- a/tools/perf/util/bpf_counter.c
> +++ b/tools/perf/util/bpf_counter.c
> @@ -265,7 +265,7 @@ static int bpf_program_profiler__read(struct evsel *evsel)
> return 0;
> }
>
> -static int bpf_program_profiler__install_pe(struct evsel *evsel, int cpu,
> +static int bpf_program_profiler__install_pe(struct evsel *evsel, int cpu_map_idx,
> int fd)
> {
> struct bpf_prog_profiler_bpf *skel;
> @@ -277,7 +277,7 @@ static int bpf_program_profiler__install_pe(struct evsel *evsel, int cpu,
> assert(skel != NULL);
>
> ret = bpf_map_update_elem(bpf_map__fd(skel->maps.events),
> - &cpu, &fd, BPF_ANY);
> + &cpu_map_idx, &fd, BPF_ANY);
> if (ret)
> return ret;
> }
> @@ -566,12 +566,12 @@ static int bperf__load(struct evsel *evsel, struct target *target)
> return err;
> }
>
> -static int bperf__install_pe(struct evsel *evsel, int cpu, int fd)
> +static int bperf__install_pe(struct evsel *evsel, int cpu_map_idx, int fd)
> {
> struct bperf_leader_bpf *skel = evsel->leader_skel;
>
> return bpf_map_update_elem(bpf_map__fd(skel->maps.events),
> - &cpu, &fd, BPF_ANY);
> + &cpu_map_idx, &fd, BPF_ANY);
> }
>
> /*
> @@ -623,9 +623,7 @@ static int bperf__read(struct evsel *evsel)
> case BPERF_FILTER_GLOBAL:
> assert(i == 0);
>
> - num_cpu = all_cpu_map->nr;
> - for (j = 0; j < num_cpu; j++) {
> - cpu = all_cpu_map->map[j];
> + perf_cpu_map__for_each_cpu(cpu, j, all_cpu_map) {
> perf_counts(evsel->counts, cpu, 0)->val = values[cpu].counter;
> perf_counts(evsel->counts, cpu, 0)->ena = values[cpu].enabled;
> perf_counts(evsel->counts, cpu, 0)->run = values[cpu].running;
> @@ -757,11 +755,11 @@ static inline bool bpf_counter_skip(struct evsel *evsel)
> evsel->follower_skel == NULL;
> }
>
> -int bpf_counter__install_pe(struct evsel *evsel, int cpu, int fd)
> +int bpf_counter__install_pe(struct evsel *evsel, int cpu_map_idx, int fd)
> {
> if (bpf_counter_skip(evsel))
> return 0;
> - return evsel->bpf_counter_ops->install_pe(evsel, cpu, fd);
> + return evsel->bpf_counter_ops->install_pe(evsel, cpu_map_idx, fd);
> }
>
> int bpf_counter__load(struct evsel *evsel, struct target *target)
> diff --git a/tools/perf/util/bpf_counter.h b/tools/perf/util/bpf_counter.h
> index 65ebaa6694fb..4dbf26408b69 100644
> --- a/tools/perf/util/bpf_counter.h
> +++ b/tools/perf/util/bpf_counter.h
> @@ -16,7 +16,7 @@ typedef int (*bpf_counter_evsel_op)(struct evsel *evsel);
> typedef int (*bpf_counter_evsel_target_op)(struct evsel *evsel,
> struct target *target);
> typedef int (*bpf_counter_evsel_install_pe_op)(struct evsel *evsel,
> - int cpu,
> + int cpu_map_idx,
> int fd);
>
> struct bpf_counter_ops {
> @@ -40,7 +40,7 @@ int bpf_counter__enable(struct evsel *evsel);
> int bpf_counter__disable(struct evsel *evsel);
> int bpf_counter__read(struct evsel *evsel);
> void bpf_counter__destroy(struct evsel *evsel);
> -int bpf_counter__install_pe(struct evsel *evsel, int cpu, int fd);
> +int bpf_counter__install_pe(struct evsel *evsel, int cpu_map_idx, int fd);
>
> #else /* HAVE_BPF_SKEL */
>
> --
> 2.34.1.448.ga2b2bfdf31-goog
--
- Arnaldo