[tip:perf/core] perf hists: Do not skip elided fields when processing samples

From: tip-bot for Namhyung Kim
Date: Sun Nov 29 2015 - 02:57:02 EST


Commit-ID: e72655d97d24fff559b4ab59de791c3741a74c8c
Gitweb: http://git.kernel.org/tip/e72655d97d24fff559b4ab59de791c3741a74c8c
Author: Namhyung Kim <namhyung@xxxxxxxxxx>
AuthorDate: Sat, 28 Nov 2015 02:32:38 +0900
Committer: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
CommitDate: Fri, 27 Nov 2015 21:42:13 -0300

perf hists: Do not skip elided fields when processing samples

If user gives a filter, perf marks the corresponding column elided and
omits the output. But it should process and aggregates samples using
the field, otherwise samples will be aggregated as if the column was not
there resulted in incorrect output.

For example, I'd like to set a filter on native_write_msr_safe. The
original overhead of the function is negligible.

$ perf report | grep native_write_msr_safe
0.00% swapper [kernel.vmlinux] native_write_msr_safe
0.00% perf [kernel.vmlinux] native_write_msr_safe

However adding -S option gives different output.

$ perf report -S native_write_msr_safe --percentage absolute | \
> grep -e swapper -e perf
51.47% swapper [kernel.vmlinux]
4.14% perf [kernel.vmlinux]

Since it aggregated samples using comm and dso only. In fact, the above
values are same when it sorts with -s comm,dso.

$ perf report -s comm,dso | grep -e swapper -e perf
51.47% swapper [kernel.vmlinux]
4.14% perf [kernel.vmlinux]

This resulted in TUI failure with -ERANGE since it tries to increase
sample hit count for annotation with wrong symbols due to incorrect
aggregation.

This patch fixes it not to skip elided fields when comparing samples in
order to insert them to the hists.

Commiter note:

After the patch, with a different workloads:

# perf report --show-total-period -S native_write_msr_safe --stdio
#
# symbol: native_write_msr_safe
#
# Samples: 455 of event 'cycles:pp'
# Event count (approx.): 134787489
#
# Overhead Period Command Shared Object
# ........ ...... ............... ................
#
0.22% 293081 qemu-system-x86 [vmlinux]
0.19% 255914 swapper [vmlinux]
0.00% 2054 Timer [vmlinux]
0.00% 1021 firefox [vmlinux]
0.00% 2 perf [vmlinux]

# perf report --show-total-period | grep native_write_msr_safe
Failed to open /tmp/perf-14838.map, continuing without symbols
0.22% 293081 qemu-system-x86 [vmlinux] [k] native_write_msr_safe
0.19% 255914 swapper [vmlinux] [k] native_write_msr_safe
0.00% 2054 Timer [vmlinux] [k] native_write_msr_safe
0.00% 1021 firefox [vmlinux] [k] native_write_msr_safe
0.00% 2 perf [vmlinux] [k] native_write_msr_safe
#

Reported-by: Ingo Molnar <mingo@xxxxxxxxxx>
Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>
Tested-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
Cc: David Ahern <dsahern@xxxxxxxxx>
Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
Cc: Peter Zijlstra <a.p.zijlstra@xxxxxxxxx>
Link: http://lkml.kernel.org/r/1448645559-31167-2-git-send-email-namhyung@xxxxxxxxxx
Signed-off-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
---
tools/perf/util/hist.c | 6 ------
1 file changed, 6 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 4fd37d6..6e8e0ee 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -924,9 +924,6 @@ hist_entry__cmp(struct hist_entry *left, struct hist_entry *right)
int64_t cmp = 0;

perf_hpp__for_each_sort_list(fmt) {
- if (perf_hpp__should_skip(fmt))
- continue;
-
cmp = fmt->cmp(fmt, left, right);
if (cmp)
break;
@@ -942,9 +939,6 @@ hist_entry__collapse(struct hist_entry *left, struct hist_entry *right)
int64_t cmp = 0;

perf_hpp__for_each_sort_list(fmt) {
- if (perf_hpp__should_skip(fmt))
- continue;
-
cmp = fmt->collapse(fmt, left, right);
if (cmp)
break;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/