[GIT PULL 0/5] perf/urgent hists hierarchy fixes

[GIT PULL 0/5] perf/urgent hists hierarchy fixes

[Arnaldo Carvalho de Melo]

Hi Ingo,

	Please consider pulling these 'perf report/top --hierarchy' fixes into
perf/urgent, they don't fix segfaults, just some oddities in scrolling, etc.

	If you think it is too late in the 4.9-rc window, then please pull it
instead into perf/core, I did the build/test/container tests for both branches.

Thanks,

- Arnaldo

Test results (for perf/urgent + this) at the end.

The following changes since commit f92b7604149a55cb601fc0b52911b1e11f0f2514:

  perf/x86/intel: Honour the CPUID for number of fixed counters in hypervisors (2016-10-28 11:06:25 +0200)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-hists-hierarchy-fixes-for-mingo-20161111

for you to fetch changes up to c72ab446cac1d6c9551fd26c4cfef1b2fc5041fd:

  perf hists: Fix column length on --hierarchy (2016-11-09 11:55:29 -0300)

----------------------------------------------------------------
perf/urgent fixes for perf {top,report} --hierarchy

- These are fixes for the --hierarchy view of perf top and report, fixing
  output oddities, mostly related to scrolling. (Namhyung Kim)

Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

----------------------------------------------------------------
Namhyung Kim (5):
      perf hist browser: Fix hierarchy column counts
      perf hists browser: Fix indentation of folded sign on --hierarchy
      perf hists browser: Show folded sign properly on --hierarchy
      perf hists browser: Fix column indentation on --hierarchy
      perf hists: Fix column length on --hierarchy

 tools/perf/ui/browsers/hists.c | 48 ++++++++++++++++++++++++++++++++----------
 tools/perf/util/hist.c         | 12 +++++------
 2 files changed, 43 insertions(+), 17 deletions(-)

  # perf test
   1: vmlinux symtab matches kallsyms                          : Ok
   2: detect openat syscall event                              : Ok
   3: detect openat syscall event on all cpus                  : Ok
   4: read samples using the mmap interface                    : Ok
   5: parse events tests                                       : Ok
   6: Validate PERF_RECORD_* events & perf_sample fields       : Ok
   7: Test perf pmu format parsing                             : Ok
   8: Test dso data read                                       : Ok
   9: Test dso data cache                                      : Ok
  10: Test dso data reopen                                     : Ok
  11: roundtrip evsel->name check                              : Ok
  12: Check parsing of sched tracepoints fields                : Ok
  13: Generate and check syscalls:sys_enter_openat event fields: Ok
  14: struct perf_event_attr setup                             : Ok
  15: Test matching and linking multiple hists                 : Ok
  16: Try 'import perf' in python, checking link problems      : Ok
  17: Test breakpoint overflow signal handler                  : Ok
  18: Test breakpoint overflow sampling                        : Ok
  19: Test number of exit event of a simple workload           : Ok
  20: Test software clock events have valid period values      : Ok
  21: Test object code reading                                 : Ok
  22: Test sample parsing                                      : Ok
  23: Test using a dummy software event to keep tracking       : Ok
  24: Test parsing with no sample_id_all bit set               : Ok
  25: Test filtering hist entries                              : Ok
  26: Test mmap thread lookup                                  : Ok
  27: Test thread mg sharing                                   : Ok
  28: Test output sorting of hist entries                      : Ok
  29: Test cumulation of child hist entries                    : Ok
  30: Test tracking with sched_switch                          : Ok
  31: Filter fds with revents mask in a fdarray                : Ok
  32: Add fd to a fdarray, making it autogrow                  : Ok
  33: Test kmod_path__parse function                           : Ok
  34: Test thread map                                          : Ok
  35: Test LLVM searching and compiling                        :
  35.1: Basic BPF llvm compiling test                          : Ok
  35.2: Test kbuild searching                                  : Ok
  35.3: Compile source for BPF prologue generation test        : Ok
  35.4: Compile source for BPF relocation test                 : Ok
  36: Test topology in session                                 : Ok
  37: Test BPF filter                                          :
  37.1: Test basic BPF filtering                               : Ok
  37.2: Test BPF prologue generation                           : Ok
  37.3: Test BPF relocation checker                            : Ok
  38: Test thread map synthesize                               : Ok
  39: Test cpu map synthesize                                  : Ok
  40: Test stat config synthesize                              : Ok
  41: Test stat synthesize                                     : Ok
  42: Test stat round synthesize                               : Ok
  43: Test attr update synthesize                              : Ok
  44: Test events times                                        : Ok
  45: Test backward reading from ring buffer                   : Ok
  46: Test cpu map print                                       : Ok
  47: Test SDT event probing                                   : Ok
  48: Test is_printable_array function                         : Ok
  49: Test bitmap print                                        : Ok
  50: x86 rdpmc test                                           : Ok
  51: Test converting perf time to TSC                         : Ok
  52: Test dwarf unwind                                        : Ok
  53: Test x86 instruction decoder - new instructions          : Ok
  54: Test intel cqm nmi context read                          : Skip
  # 

  # time dm
   1 alpine:3.4: Ok
   2 android-ndk:r12b-arm: Ok
   3 archlinux:latest: Ok
   4 centos:5: Ok
   5 centos:6: Ok
   6 centos:7: Ok
   7 debian:7: Ok
   8 debian:8: Ok
   9 debian:experimental: Ok
  10 fedora:20: Ok
  11 fedora:21: Ok
  12 fedora:22: Ok
  13 fedora:23: Ok
  14 fedora:24: Ok
  15 fedora:24-x-ARC-uClibc: Ok
  16 fedora:rawhide: Ok
  17 mageia:5: Ok
  18 opensuse:13.2: Ok
  19 opensuse:42.1: Ok
  20 opensuse:tumbleweed: Ok
  21 ubuntu:12.04.5: Ok
  22 ubuntu:14.04.4: Ok
  23 ubuntu:14.04.4-x-linaro-arm64: Ok
  24 ubuntu:15.10: Ok
  25 ubuntu:16.04: Ok
  26 ubuntu:16.04-x-arm: Ok
  27 ubuntu:16.04-x-arm64: Ok
  28 ubuntu:16.04-x-powerpc: Ok
  29 ubuntu:16.04-x-powerpc64: Ok
  30 ubuntu:16.04-x-powerpc64el: Ok
  31 ubuntu:16.04-x-s390: Ok
  32 ubuntu:16.10: Ok

  real	35m13.612s
  user	0m2.028s
  sys	0m2.454s
  #

  $ make -C tools/perf build-test
  make: Entering directory '/home/acme/git/linux/tools/perf'
  - tarpkg: ./tests/perf-targz-src-pkg .
                  make_no_ui_O: && make NO_NEWT=1 NO_SLANG=1 NO_GTK2=1
            make_no_libaudit_O: && make NO_LIBAUDIT=1
                make_no_newt_O: && make NO_NEWT=1
              make_clean_all_O: && make clean all
         make_install_prefix_O: && make install prefix=/tmp/krava
                   make_help_O: && make help
           make_no_libpython_O: && make NO_LIBPYTHON=1
             make_no_scripts_O: && make NO_LIBPYTHON=1 NO_LIBPERL=1
             make_no_libperl_O: && make NO_LIBPERL=1
           make_no_libunwind_O: && make NO_LIBUNWIND=1
            make_no_demangle_O: && make NO_DEMANGLE=1
                make_minimal_O: && make NO_LIBPERL=1 NO_LIBPYTHON=1 NO_NEWT=1 NO_GTK2=1 NO_DEMANGLE=1 NO_LIBELF=1 NO_LIBUNWIND=1 NO_BACKTRACE=1 NO_LIBNUMA=1 NO_LIBAUDIT=1 NO_LIBBIONIC=1 NO_LIBDW_DWARF_UNWIND=1 NO_AUXTRACE=1 NO_LIBBPF=1 NO_LIBCRYPTO=1 NO_SDT=1
                 make_perf_o_O: && make perf.o
  make_no_libdw_dwarf_unwind_O: && make NO_LIBDW_DWARF_UNWIND=1
   make_install_prefix_slash_O: && make install prefix=/tmp/krava/
              make_no_libelf_O: && make NO_LIBELF=1
            make_install_bin_O: && make install-bin
                    make_doc_O: && make doc
            make_no_auxtrace_O: && make NO_AUXTRACE=1
              make_no_libbpf_O: && make NO_LIBBPF=1
       make_util_pmu_bison_o_O: && make util/pmu-bison.o
                 make_static_O: && make LDFLAGS=-static
                make_install_O: && make install
             make_no_libnuma_O: && make NO_LIBNUMA=1
        make_with_babeltrace_O: && make LIBBABELTRACE=1
                   make_tags_O: && make tags
             make_util_map_o_O: && make util/map.o
           make_no_backtrace_O: && make NO_BACKTRACE=1
                make_no_gtk2_O: && make NO_GTK2=1
                  make_debug_O: && make DEBUG=1
               make_no_slang_O: && make NO_SLANG=1
           make_no_libbionic_O: && make NO_LIBBIONIC=1
                   make_pure_O: && make
  OK
  make: Leaving directory '/home/acme/git/linux/tools/perf'

[PATCH 1/5] perf hist browser: Fix hierarchy column counts

[Arnaldo Carvalho de Melo]

From: Namhyung Kim <namhyung@kernel.org>

The perf report/top on TUI supports horizontal scrolling using LEFT and
RIGHT keys.

But it calculate the number of columns incorrectly when hierarchy mode
is enabled so that keep pressing RIGHT key can make the output
disappeared.

In the hierarchy mode, all sort keys are collapsed into a single column,
so it needs to be applied when calculating column numbers.

Reported-and-Tested-by: Markus Trippelsdorf <markus@trippelsdorf.de>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20161024162110.17918-1-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 4ffff7be9299..5adedc1a09d3 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -2076,8 +2076,21 @@ void hist_browser__init(struct hist_browser *browser,
 	browser->b.use_navkeypressed	= true;
 	browser->show_headers		= symbol_conf.show_hist_headers;
 
-	hists__for_each_format(hists, fmt)
+	if (symbol_conf.report_hierarchy) {
+		struct perf_hpp_list_node *fmt_node;
+
+		/* count overhead columns (in the first node) */
+		fmt_node = list_first_entry(&hists->hpp_formats,
+					    struct perf_hpp_list_node, list);
+		perf_hpp_list__for_each_format(&fmt_node->hpp, fmt)
+			++browser->b.columns;
+
+		/* add a single column for whole hierarchy sort keys*/
 		++browser->b.columns;
+	} else {
+		hists__for_each_format(hists, fmt)
+			++browser->b.columns;
+	}
 
 	hists__reset_column_width(hists);
 }
-- 
2.7.4

[PATCH 2/5] perf hists browser: Fix indentation of folded sign on --hierarchy

[Arnaldo Carvalho de Melo]

From: Namhyung Kim <namhyung@kernel.org>

It should indent 2 spaces for folded sign and a whitespace.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Markus Trippelsdorf <markus@trippelsdorf.de>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20161108130833.9263-2-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 5adedc1a09d3..225ef2a15a13 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1337,8 +1337,8 @@ static int hist_browser__show_hierarchy_entry(struct hist_browser *browser,
 		}
 
 		if (first) {
-			ui_browser__printf(&browser->b, "%c", folded_sign);
-			width--;
+			ui_browser__printf(&browser->b, "%c ", folded_sign);
+			width -= 2;
 			first = false;
 		} else {
 			ui_browser__printf(&browser->b, "  ");
@@ -1555,7 +1555,7 @@ static int hists_browser__scnprintf_hierarchy_headers(struct hist_browser *brows
 	int indent = hists->nr_hpp_node - 2;
 	bool first_node, first_col;
 
-	ret = scnprintf(buf, size, " ");
+	ret = scnprintf(buf, size, "  ");
 	if (advance_hpp_check(&dummy_hpp, ret))
 		return ret;
 
-- 
2.7.4

[PATCH 3/5] perf hists browser: Show folded sign properly on --hierarchy

[Arnaldo Carvalho de Melo]

From: Namhyung Kim <namhyung@kernel.org>

When horizontal scrolling is used in hierarchy mode, the folded signed
disappears at the right most column.

Committer note:

To test it, run 'perf top --hierarchy, see the '+' symbol at the first
column, then press the right arrow key, the '+' symbol will disappear,
this patch fixes that.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Tested-by: Markus Trippelsdorf <markus@trippelsdorf.de>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20161108130833.9263-3-namhyung@kernel.org
[ Move 'width -= 2' invariant to right after the if/else ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 225ef2a15a13..e767fbd17ad2 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1381,7 +1381,13 @@ static int hist_browser__show_hierarchy_entry(struct hist_browser *browser,
 		}
 
 		perf_hpp_list__for_each_format(entry->hpp_list, fmt) {
-			ui_browser__write_nstring(&browser->b, "", 2);
+			if (first) {
+				ui_browser__printf(&browser->b, "%c ", folded_sign);
+				first = false;
+			} else {
+				ui_browser__write_nstring(&browser->b, "", 2);
+			}
+
 			width -= 2;
 
 			/*
-- 
2.7.4

[PATCH 4/5] perf hists browser: Fix column indentation on --hierarchy

[Arnaldo Carvalho de Melo]

From: Namhyung Kim <namhyung@kernel.org>

When horizontall scrolling is used in hierarchy mode, the the right most
column has unnecessary indentation.  Actually it's needed only if some
of left (overhead) columns were shown.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Tested-by: Markus Trippelsdorf <markus@trippelsdorf.de>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20161108130833.9263-4-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index e767fbd17ad2..a53fef0c673b 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1361,8 +1361,10 @@ static int hist_browser__show_hierarchy_entry(struct hist_browser *browser,
 		width -= hpp.buf - s;
 	}
 
-	ui_browser__write_nstring(&browser->b, "", hierarchy_indent);
-	width -= hierarchy_indent;
+	if (!first) {
+		ui_browser__write_nstring(&browser->b, "", hierarchy_indent);
+		width -= hierarchy_indent;
+	}
 
 	if (column >= browser->b.horiz_scroll) {
 		char s[2048];
@@ -1565,6 +1567,7 @@ static int hists_browser__scnprintf_hierarchy_headers(struct hist_browser *brows
 	if (advance_hpp_check(&dummy_hpp, ret))
 		return ret;
 
+	first_node = true;
 	/* the first hpp_list_node is for overhead columns */
 	fmt_node = list_first_entry(&hists->hpp_formats,
 				    struct perf_hpp_list_node, list);
@@ -1579,12 +1582,16 @@ static int hists_browser__scnprintf_hierarchy_headers(struct hist_browser *brows
 		ret = scnprintf(dummy_hpp.buf, dummy_hpp.size, "  ");
 		if (advance_hpp_check(&dummy_hpp, ret))
 			break;
+
+		first_node = false;
 	}
 
-	ret = scnprintf(dummy_hpp.buf, dummy_hpp.size, "%*s",
-			indent * HIERARCHY_INDENT, "");
-	if (advance_hpp_check(&dummy_hpp, ret))
-		return ret;
+	if (!first_node) {
+		ret = scnprintf(dummy_hpp.buf, dummy_hpp.size, "%*s",
+				indent * HIERARCHY_INDENT, "");
+		if (advance_hpp_check(&dummy_hpp, ret))
+			return ret;
+	}
 
 	first_node = true;
 	list_for_each_entry_continue(fmt_node, &hists->hpp_formats, list) {
-- 
2.7.4

[PATCH 5/5] perf hists: Fix column length on --hierarchy

[Arnaldo Carvalho de Melo]

From: Namhyung Kim <namhyung@kernel.org>

Markus reported that there's a weird behavior on perf top --hierarchy
regarding the column length.

Looking at the code, I found a dubious code which affects the symptoms.
When --hierarchy option is used, the last column length might be
inaccurate since it skips to update the length on leaf entries.

I cannot remember why it did and looks like a leftover from previous
version during the development.

Anyway, updating the column length often is not harmful.  So let's move
the code out.

Reported-and-Tested-by: Markus Trippelsdorf <markus@trippelsdorf.de>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Fixes: 1a3906a7e6b9 ("perf hists: Resort hist entries with hierarchy")
Link: http://lkml.kernel.org/r/20161108130833.9263-5-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hist.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index b02992efb513..a69f027368ef 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -1600,18 +1600,18 @@ static void hists__hierarchy_output_resort(struct hists *hists,
 		if (prog)
 			ui_progress__update(prog, 1);
 
+		hists->nr_entries++;
+		if (!he->filtered) {
+			hists->nr_non_filtered_entries++;
+			hists__calc_col_len(hists, he);
+		}
+
 		if (!he->leaf) {
 			hists__hierarchy_output_resort(hists, prog,
 						       &he->hroot_in,
 						       &he->hroot_out,
 						       min_callchain_hits,
 						       use_callchain);
-			hists->nr_entries++;
-			if (!he->filtered) {
-				hists->nr_non_filtered_entries++;
-				hists__calc_col_len(hists, he);
-			}
-
 			continue;
 		}
 
-- 
2.7.4

Re: [GIT PULL 0/5] perf/urgent hists hierarchy fixes

[Ingo Molnar]

* Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Hi Ingo,
> 
> 	Please consider pulling these 'perf report/top --hierarchy' fixes into
> perf/urgent, they don't fix segfaults, just some oddities in scrolling, etc.
> 
> 	If you think it is too late in the 4.9-rc window, then please pull it
> instead into perf/core, I did the build/test/container tests for both branches.
> 
> Thanks,
> 
> - Arnaldo
> 
> Test results (for perf/urgent + this) at the end.
> 
> The following changes since commit f92b7604149a55cb601fc0b52911b1e11f0f2514:
> 
>   perf/x86/intel: Honour the CPUID for number of fixed counters in hypervisors (2016-10-28 11:06:25 +0200)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-hists-hierarchy-fixes-for-mingo-20161111
> 
> for you to fetch changes up to c72ab446cac1d6c9551fd26c4cfef1b2fc5041fd:
> 
>   perf hists: Fix column length on --hierarchy (2016-11-09 11:55:29 -0300)
> 
> ----------------------------------------------------------------
> perf/urgent fixes for perf {top,report} --hierarchy
> 
> - These are fixes for the --hierarchy view of perf top and report, fixing
>   output oddities, mostly related to scrolling. (Namhyung Kim)
> 
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> ----------------------------------------------------------------
> Namhyung Kim (5):
>       perf hist browser: Fix hierarchy column counts
>       perf hists browser: Fix indentation of folded sign on --hierarchy
>       perf hists browser: Show folded sign properly on --hierarchy
>       perf hists browser: Fix column indentation on --hierarchy
>       perf hists: Fix column length on --hierarchy
> 
>  tools/perf/ui/browsers/hists.c | 48 ++++++++++++++++++++++++++++++++----------
>  tools/perf/util/hist.c         | 12 +++++------
>  2 files changed, 43 insertions(+), 17 deletions(-)

Pulled, thanks a lot Arnaldo!

	Ingo