Thread (84 messages) 84 messages, 6 authors, 2022-01-12

Re: [PATCH v4 44/48] perf bpf: Rename cpu to cpu_map_idx

From: Arnaldo Carvalho de Melo <acme@kernel.org>
Date: 2022-01-10 19:26:40
Also in: linux-perf-users, lkml
Subsystem: performance events subsystem, the rest · Maintainers: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Linus Torvalds

Em Mon, Jan 10, 2022 at 04:10:46PM -0300, Arnaldo Carvalho de Melo escreveu:
Em Tue, Jan 04, 2022 at 10:13:47PM -0800, Ian Rogers escreveu:
quoted
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...
Would be better to have the "Shorten 3 lines of code in bperf_read by
using perf_cpu_map__for_each_cpu." in a separate patch, try to do this
next time.

Below cures the two issues.

- Arnaldo


    Committer notes:

    Found when building with BUILD_BPF_SKEL=1:

    Remove unused 'num_cpu' variable in bperf__read().

    Make 'j' an 'int' as it is used in perf_cpu_map__for_each_cpu() to compare against an 'int'

diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.c
index ae5bd87ff02033c2..80d1a3a31052fe55 100644
--- a/tools/perf/util/bpf_counter.c
+++ b/tools/perf/util/bpf_counter.c
@@ -608,7 +608,8 @@ static int bperf__read(struct evsel *evsel)
 	__u32 num_cpu_bpf = cpu__max_cpu();
 	struct bpf_perf_event_value values[num_cpu_bpf];
 	int reading_map_fd, err = 0;
-	__u32 i, j, num_cpu;
+	__u32 i;
+	int j;
 
 	bperf_sync_counters(evsel);
 	reading_map_fd = bpf_map__fd(skel->maps.accum_readings);
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help