From 02d8dabc50f94353075f2f62b1047c1306e8bf92 Mon Sep 17 00:00:00 2001 From: Stephane Eranian Date: Thu, 3 Sep 2015 15:23:40 +0200 Subject: perf stat: Fix per-pkg event reporting bug Per-pkg events need to be captured once per processor socket. The code in check_per_pkg() ensures only one value per processor package is used. However there is a problem with this function in case the first CPU of the package does not measure anything for the per-pkg event, but other CPUs do. Consider the following: $ create cgroup FOO; echo $$ >FOO/tasks; taskset -c 1 noploop & $ perf stat -a -I 1000 -e intel_cqm/llc_occupancy/ -G FOO sleep 100 1.00000 Bytes intel_cqm/llc_occupancy/ FOO The reason for this is that CPU0 in the cgroup has nothing running on it. Yet check_per_plg() will mark socket0 as processed and no other event value will be considered for the socket. This patch fixes the problem by having check_per_pkg() only consider events which actually ran. Signed-off-by: Stephane Eranian Cc: Adrian Hunter Cc: Andi Kleen Cc: David Ahern Cc: Jiri Olsa Cc: Kan Liang Cc: Namhyung Kim Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/1441286620-10117-1-git-send-email-eranian@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/stat.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) (limited to 'tools/perf') diff --git a/tools/perf/util/stat.c b/tools/perf/util/stat.c index 415c359..2d065d0 100644 --- a/tools/perf/util/stat.c +++ b/tools/perf/util/stat.c @@ -196,7 +196,8 @@ static void zero_per_pkg(struct perf_evsel *counter) memset(counter->per_pkg_mask, 0, MAX_NR_CPUS); } -static int check_per_pkg(struct perf_evsel *counter, int cpu, bool *skip) +static int check_per_pkg(struct perf_evsel *counter, + struct perf_counts_values *vals, int cpu, bool *skip) { unsigned long *mask = counter->per_pkg_mask; struct cpu_map *cpus = perf_evsel__cpus(counter); @@ -218,6 +219,17 @@ static int check_per_pkg(struct perf_evsel *counter, int cpu, bool *skip) counter->per_pkg_mask = mask; } + /* + * we do not consider an event that has not run as a good + * instance to mark a package as used (skip=1). Otherwise + * we may run into a situation where the first CPU in a package + * is not running anything, yet the second is, and this function + * would mark the package as used after the first CPU and would + * not read the values from the second CPU. + */ + if (!(vals->run && vals->ena)) + return 0; + s = cpu_map__get_socket(cpus, cpu); if (s < 0) return -1; @@ -235,7 +247,7 @@ process_counter_values(struct perf_stat_config *config, struct perf_evsel *evsel static struct perf_counts_values zero; bool skip = false; - if (check_per_pkg(evsel, cpu, &skip)) { + if (check_per_pkg(evsel, count, cpu, &skip)) { pr_err("failed to read per-pkg counter\n"); return -1; } -- cgit v1.1 From 179f36dde3cec0f9f05a757b68f6a58e4edbcc95 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Thu, 17 Sep 2015 11:30:20 -0300 Subject: Revert "perf symbols: Fix mismatched declarations for elf_getphdrnum" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit f785f2357673d520a0b7b468973cdd197f336494. We have a test to check if elf_getphdrnum() is present, so, if it fails, we'll get: [acme@rhel5 linux]$ cat /tmp/build/perf/feature/test-libelf-getphdrnum.make.output cc1: warnings being treated as errors test-libelf-getphdrnum.c: In function ‘main’: test-libelf-getphdrnum.c:7: warning: implicit declaration of function ‘elf_getphdrnum’ [acme@rhel5 linux]$ And this block will not be compiled: #ifndef HAVE_ELF_GETPHDRNUM_SUPPORT static int elf_getphdrnum(Elf *elf, size_t *dst) ... #endif So, if elf_getphdrnum() is being defined somewhere, there is a problem with the test that is not detecting that function, go fix it. Reported-by: Vinson Lee Cc: Adrian Hunter Cc: Borislav Petkov Cc: David Ahern Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Namhyung Kim Cc: "Naveen N. Rao" Cc: Peter Zijlstra Cc: Srikar Dronamraju Cc: Stephane Eranian Cc: Victor Kamensky Cc: Wang Nan Link: http://lkml.kernel.org/n/tip-qn459fal6acvcvm50i8zxx9k@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/symbol-elf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools/perf') diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c index 53bb5f5..f78ea3d 100644 --- a/tools/perf/util/symbol-elf.c +++ b/tools/perf/util/symbol-elf.c @@ -38,7 +38,7 @@ static inline char *bfd_demangle(void __maybe_unused *v, #endif #ifndef HAVE_ELF_GETPHDRNUM_SUPPORT -int elf_getphdrnum(Elf *elf, size_t *dst) +static int elf_getphdrnum(Elf *elf, size_t *dst) { GElf_Ehdr gehdr; GElf_Ehdr *ehdr; -- cgit v1.1 From f8ac8606fd3cd72183de8eec2b151ff05040c70f Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Thu, 17 Sep 2015 12:20:28 -0300 Subject: tools build: Add test for presence of numa_num_possible_cpus() in libnuma The existing numa test checks only if numa.h and numa_available() are present, but that can be satisfied with an old libnuma that is not enough for the 'perf bench numa' entry, so add a test to check for that: [acme@rhel5 linux]$ make NO_AUXTRACE=1 NO_LIBPERL=1 -C tools/perf O=/tmp/build/perf install-bin make: Entering directory `/home/acme/git/linux/tools/perf' BUILD: Doing 'make -j2' parallel build Auto-detecting system features: ... libelf: [ on ] ... libnuma: [ on ] ... numa_num_possible_cpus: [ OFF ] ... libperl: [ on ] config/Makefile:577: Old numa library found, disables 'perf bench numa mem' benchmark, please install numactl-devel/libnuma-devel/libnuma-dev >= 2.0.8 INSTALL binaries This fixes the build on old systems such as RHEL/CentOS 5.11. Cc: Adrian Hunter Cc: Borislav Petkov Cc: David Ahern Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Namhyung Kim Cc: "Naveen N. Rao" Cc: Peter Zijlstra Cc: Srikar Dronamraju Cc: Stephane Eranian Cc: Victor Kamensky Cc: Vinson Lee Cc: Wang Nan Link: http://lkml.kernel.org/n/tip-zqriqkezppi2de2iyjin1tnc@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/config/Makefile | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'tools/perf') diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile index 827557f..053e65b 100644 --- a/tools/perf/config/Makefile +++ b/tools/perf/config/Makefile @@ -573,9 +573,14 @@ ifndef NO_LIBNUMA msg := $(warning No numa.h found, disables 'perf bench numa mem' benchmark, please install numactl-devel/libnuma-devel/libnuma-dev); NO_LIBNUMA := 1 else - CFLAGS += -DHAVE_LIBNUMA_SUPPORT - EXTLIBS += -lnuma - $(call detected,CONFIG_NUMA) + ifeq ($(feature-numa_num_possible_cpus), 0) + msg := $(warning Old numa library found, disables 'perf bench numa mem' benchmark, please install numactl-devel/libnuma-devel/libnuma-dev >= 2.0.8); + NO_LIBNUMA := 1 + else + CFLAGS += -DHAVE_LIBNUMA_SUPPORT + EXTLIBS += -lnuma + $(call detected,CONFIG_NUMA) + endif endif endif -- cgit v1.1 From b0063dbfb031a7c728ed0d9533257e8329292cf1 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Thu, 17 Sep 2015 12:54:30 -0300 Subject: tools build: Add test for presence of __get_cpuid() gcc builtin The auxtrace code needed by Intel PT uses the __get_cpuid() gcc builtin, that is not present in old systems, breaking the build. Add a test to check for that builtin and disable AUXTRACE in those systems. [acme@rhel5 linux]$ make NO_LIBPERL=1 -C tools/perf O=/tmp/build/perf install-bin make: Entering directory `/home/acme/git/linux/tools/perf' BUILD: Doing 'make -j2' parallel build Auto-detecting system features: ... lzma: [ on ] ... get_cpuid: [ OFF ] config/Makefile:630: Your gcc lacks the __get_cpuid() builtin, disables support for auxtrace/Intel PT, please install a newer gcc MKDIR /tmp/build/perf/util/ This fixes the build on old systems such as RHEL/CentOS 5.11. Cc: Adrian Hunter Cc: Borislav Petkov Cc: David Ahern Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Namhyung Kim Cc: "Naveen N. Rao" Cc: Peter Zijlstra Cc: Srikar Dronamraju Cc: Stephane Eranian Cc: Victor Kamensky Cc: Vinson Lee Cc: Wang Nan Link: http://lkml.kernel.org/n/tip-d4puslul0jltoodzpx9r4sje@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/config/Makefile | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'tools/perf') diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile index 053e65b..38a0853 100644 --- a/tools/perf/config/Makefile +++ b/tools/perf/config/Makefile @@ -626,8 +626,13 @@ ifdef LIBBABELTRACE endif ifndef NO_AUXTRACE - $(call detected,CONFIG_AUXTRACE) - CFLAGS += -DHAVE_AUXTRACE_SUPPORT + ifeq ($(feature-get_cpuid), 0) + msg := $(warning Your gcc lacks the __get_cpuid() builtin, disables support for auxtrace/Intel PT, please install a newer gcc); + NO_AUXTRACE := 1 + else + $(call detected,CONFIG_AUXTRACE) + CFLAGS += -DHAVE_AUXTRACE_SUPPORT + endif endif # Among the variables below, these: -- cgit v1.1 From bf6445631c6f00882b25516a174d5073ce0c6f81 Mon Sep 17 00:00:00 2001 From: Peter Senna Tschudin Date: Thu, 17 Sep 2015 12:08:53 +0200 Subject: perf tools: Bool functions shouldn't return -1 Returning a negative value for a boolean function seem to have the undesired effect of returning true. Replace -1 by false in a bool-returning function. The diff of the .s file before and after the change (for x86_64): 3907c3907 < movl $1, %ebx --- > xorl %ebx, %ebx while if -1 is replaced by true, the diff is empty. This issue was found by the following Coccinelle semantic patch: @@ identifier f; constant C; typedef bool; @@ bool f (...){ <+... * return -C; ...+> } Signed-off-by: Peter Senna Tschudin Cc: Jiri Olsa Cc: Kan Liang Cc: Matt Fleming Cc: Milos Vyletel Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Steven Rostedt Link: http://lkml.kernel.org/r/1442484533-19742-1-git-send-email-peter.senna@gmail.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools/perf') diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c index 7acafb3..c2cd9bf2 100644 --- a/tools/perf/util/util.c +++ b/tools/perf/util/util.c @@ -709,7 +709,7 @@ bool find_process(const char *name) dir = opendir(procfs__mountpoint()); if (!dir) - return -1; + return false; /* Walk through the directory. */ while (ret && (d = readdir(dir)) != NULL) { -- cgit v1.1