Messages in this thread | | | Date | Mon, 10 Jan 2022 15:56:55 -0300 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [PATCH v4 02/48] perf stat: Add aggr creators that are passed a cpu. |
| |
Em Mon, Jan 10, 2022 at 03:53:45PM -0300, Arnaldo Carvalho de Melo escreveu: > Em Mon, Jan 10, 2022 at 03:52:11PM -0300, Arnaldo Carvalho de Melo escreveu: > > Em Mon, Jan 10, 2022 at 09:36:49AM -0800, Ian Rogers escreveu: > > > On Mon, Jan 10, 2022 at 9:10 AM John Garry <john.garry@huawei.com> wrote: > > > > > > > > On 05/01/2022 06:13, Ian Rogers wrote: > > > > > > > > > > +struct aggr_cpu_id cpu_map__get_socket(struct perf_cpu_map *map, int idx, > > > > > + void *data) > > > > > +{ > > > > > + if (idx < 0 || idx > map->nr) > > > > > + return cpu_map__empty_aggr_cpu_id(); > > > > > + > > > > > + return cpu_map__get_socket_aggr_by_cpu(map->map[idx], data); > > > > > +} > > > > > + > > > > > > > > > > > > This is later deleted in the series. Can the series be reworked so that > > > > we don't add stuff and then later delete it? One reason for that > > > > approach is that we don't spend time reviewing something which will be > > > > deleted, especially in such a big series... > > > > > > Hi John, > > > > > > I think you are asking to squash: > > > https://lore.kernel.org/lkml/20220105061351.120843-8-irogers@google.com/ > > > into this change. There are other similar related changes that may > > > also be squashed. The changes are trying to introduce a new API and > > > then add changes to switch over to using it. This is with a view to > > > making bisection easier, have each change only do 1 thing and so on. I > > > believe the format of the patches is house style, but it is fine to > > > squash changes together too. Having sent patches to Arnaldo and having > > > had them split I'm reluctant to do a v5 with them squashed without him > > > expressing a preference. > > > > Right, sometimes this is needed, I'm getting the patchkit now to test > > build it in my containers and will go patch by patch reviewing. > > Good start: > > ⬢[acme@toolbox perf]$ b4 am -ctsl --cc-trailers CAP-5=fWT_19OfZTTjvLUcChV4nDwqc5Zq4VE93Gak6OO4NORsA@mail.gmail.com > Looking up https://lore.kernel.org/r/CAP-5%3DfWT_19OfZTTjvLUcChV4nDwqc5Zq4VE93Gak6OO4NORsA%40mail.gmail.com > Grabbing thread from lore.kernel.org/all/CAP-5%3DfWT_19OfZTTjvLUcChV4nDwqc5Zq4VE93Gak6OO4NORsA%40mail.gmail.com/t.mbox.gz > Checking for newer revisions on https://lore.kernel.org/all/ > Analyzing 58 messages in the thread > Checking attestation on all messages, may take a moment... > --- > ✓ [PATCH v4 1/48] libperf: Add comments to perf_cpu_map. > + Reviewed-by: John Garry <john.garry@huawei.com> > + Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> > + Link: https://lore.kernel.org/r/20220105061351.120843-2-irogers@google.com > <BIG SNIP> > Cover: ./v4_20220104_irogers_refactor_perf_cpumap.cover > Link: https://lore.kernel.org/r/20220105061351.120843-1-irogers@google.com > Base: applies clean to current tree > git checkout -b v4_20220104_irogers_google_com HEAD > git am ./v4_20220104_irogers_refactor_perf_cpumap.mbx >
oops:
CC /tmp/build/perf/util/unwind-libunwind-local.o util/bpf_counter_cgroup.c: In function ‘bperf_load_program’: util/bpf_counter_cgroup.c:51:26: error: incompatible types when initializing type ‘int’ using type ‘struct perf_cpu’ 51 | int total_cpus = cpu__max_cpu(); | ^~~~~~~~~~~~ util/bpf_counter_cgroup.c:127:74: error: invalid operands to binary + (have ‘int’ and ‘struct perf_cpu’) 127 | __u32 idx = evsel->core.idx * total_cpus + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ | | | int 128 | evlist->core.all_cpus->map[cpu]; | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | struct perf_cpu util/bpf_counter_cgroup.c: In function ‘bperf_cgrp__sync_counters’: util/bpf_counter_cgroup.c:215:23: error: incompatible types when assigning to type ‘int’ from type ‘struct perf_cpu’ 215 | cpu = evlist->core.all_cpus->map[i]; | ^~~~~~ util/bpf_counter_cgroup.c: In function ‘bperf_cgrp__read’: util/bpf_counter_cgroup.c:248:26: error: incompatible types when initializing type ‘int’ using type ‘struct perf_cpu’ 248 | int total_cpus = cpu__max_cpu(); | ^~~~~~~~~~~~ util/bpf_counter_cgroup.c:275:31: error: incompatible types when assigning to type ‘int’ from type ‘struct perf_cpu’ 275 | cpu = evlist->core.all_cpus->map[i]; | ^~~~~~ make[4]: *** [/var/home/acme/git/perf/tools/build/Makefile.build:97: /tmp/build/perf/util/bpf_counter_cgroup.o] Error 1 make[4]: *** Waiting for unfinished jobs.... util/bpf_counter.c: In function ‘bperf__load’: util/bpf_counter.c:543:31: error: incompatible types when assigning to type ‘__u32’ {aka ‘unsigned int’} from type ‘struct perf_cpu’ 543 | key = evsel->core.cpus->map[i]; | ^~~~~ util/bpf_counter.c: In function ‘bperf_sync_counters’: util/bpf_counter.c:587:23: error: incompatible types when assigning to type ‘int’ from type ‘struct perf_cpu’ 587 | cpu = all_cpu_map->map[i]; | ^~~~~~~~~~~ util/bpf_counter.c: In function ‘bperf__read’: util/bpf_counter.c:608:29: error: incompatible types when initializing type ‘__u32’ {aka ‘unsigned int’} using type ‘struct perf_cpu’ 608 | __u32 num_cpu_bpf = cpu__max_cpu(); | ^~~~~~~~~~~~ In file included from /var/home/acme/git/perf/tools/perf/util/cpumap.h:8, from /var/home/acme/git/perf/tools/perf/util/env.h:7, from util/cgroup.h:8, from util/bpf_counter.c:22: /var/home/acme/git/perf/tools/lib/perf/include/perf/cpumap.h:25:33: error: incompatible types when assigning to type ‘__u32’ {aka ‘unsigned int’} from type ‘struct perf_cpu’ 25 | for ((idx) = 0, (cpu) = perf_cpu_map__cpu(cpus, idx); \ | ^~~~~~~~~~~~~~~~~ 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) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~ /var/home/acme/git/perf/tools/lib/perf/include/perf/cpumap.h:26:20: error: comparison of integer expressions of different signedness: ‘__u32’ {aka ‘unsigned int’} and ‘int’ [-Werror=sign-compare] 26 | (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) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~ /var/home/acme/git/perf/tools/lib/perf/include/perf/cpumap.h:27:31: error: incompatible types when assigning to type ‘__u32’ {aka ‘unsigned int’} from type ‘struct perf_cpu’ 27 | (idx)++, (cpu) = perf_cpu_map__cpu(cpus, idx)) | ^~~~~~~~~~~~~~~~~ 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:633:31: error: incompatible types when assigning to type ‘__u32’ {aka ‘unsigned int’} from type ‘struct perf_cpu’ 633 | cpu = evsel->core.cpus->map[i]; | ^~~~~ 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 util/bpf_ftrace.c: In function ‘perf_ftrace__latency_prepare_bpf’: util/bpf_ftrace.c:66:31: error: implicit declaration of function ‘cpu_map__cpu’; did you mean ‘cpu__max_cpu’? [-Werror=implicit-function-declaration] 66 | cpu = cpu_map__cpu(ftrace->evlist->core.cpus, i); | ^~~~~~~~~~~~ | cpu__max_cpu util/bpf_ftrace.c: In function ‘perf_ftrace__latency_read_bpf’: util/bpf_ftrace.c:125:21: error: incompatible types when initializing type ‘int’ using type ‘struct perf_cpu’ 125 | int ncpus = cpu__max_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_ftrace.o] Error 1 LD /tmp/build/perf/util/intel-pt-decoder/perf-in.o LD /tmp/build/perf/util/scripting-engines/perf-in.o 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[2]: *** Waiting for unfinished jobs.... 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':
279,675,447,200 cycles:u 348,898,435,627 instructions:u # 1.25 insn per cycle
4.978587957 seconds time elapsed
68.698979000 seconds user 12.541229000 seconds sys
⬢[acme@toolbox perf]$
This is building with:
$ make BUILD_BPF_SKEL=1 CORESIGHT=1 PYTHON=python3 O=/tmp/build/perf -C tools/perf install-bin
In general for such refactorings its important to run:
make -C tools/perf build-test
But there is no entry there with BUILD_BPF_SKEL=1, will fix that now and try to fix this in the patchkit as well.
- Arnaldo
| |