[PATCHSET 0/7] perf tools: A small random cleanup and fixups

[PATCHSET 0/7] perf tools: A small random cleanup and fixups

[Namhyung Kim]

Hello,

This is a just random collection of small cleanup and fixes that was
needed (or found) for my work.  I'd like to share it before posting
the real work as it's independent change and the work can be quite
large.. ;-)

Thanks,
Namhyung


Namhyung Kim (7):
  perf report: Get rid of report__inc_stat()
  perf report: Show progress bar for output resorting
  perf ui/tui: Print backtrace symbols when segfault occurred
  perf diff: Fix to sort by baseline field by default
  perf diff: Get rid of hists__compute_resort()
  perf tools: Append callchains only when requested
  perf tools: Set attr.task bit for a tracking event

 tools/perf/builtin-annotate.c     |  2 +-
 tools/perf/builtin-diff.c         | 84 +++++++++++++++++++--------------------
 tools/perf/builtin-report.c       | 40 ++++++++++++-------
 tools/perf/builtin-top.c          |  4 +-
 tools/perf/tests/hists_cumulate.c |  2 +-
 tools/perf/tests/hists_filter.c   |  2 +-
 tools/perf/tests/hists_output.c   | 10 ++---
 tools/perf/ui/tui/setup.c         | 26 +++++++++++-
 tools/perf/util/evsel.c           |  1 +
 tools/perf/util/hist.c            | 19 +++++++--
 tools/perf/util/hist.h            |  2 +-
 11 files changed, 116 insertions(+), 76 deletions(-)

-- 
2.1.3

[PATCH 1/7] perf report: Get rid of report__inc_stat()

[Namhyung Kim]

The report__inc_stat() is to collect number of hist entries in the
session in order to calculate the max size of progess bar.  It'd be
better if it does during addition of hist entries so that it can be
used by other places too.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-report.c | 16 +++-------------
 tools/perf/util/hist.c      |  2 ++
 2 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 39367609c707..e2a0c2bbb3bf 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -86,17 +86,6 @@ static int report__config(const char *var, const char *value, void *cb)
 	return perf_default_config(var, value, cb);
 }
 
-static void report__inc_stats(struct report *rep, struct hist_entry *he)
-{
-	/*
-	 * The @he is either of a newly created one or an existing one
-	 * merging current sample.  We only want to count a new one so
-	 * checking ->nr_events being 1.
-	 */
-	if (he->stat.nr_events == 1)
-		rep->nr_entries++;
-}
-
 static int hist_iter__report_callback(struct hist_entry_iter *iter,
 				      struct addr_location *al, bool single,
 				      void *arg)
@@ -108,8 +97,6 @@ static int hist_iter__report_callback(struct hist_entry_iter *iter,
 	struct mem_info *mi;
 	struct branch_info *bi;
 
-	report__inc_stats(rep, he);
-
 	if (!ui__has_annotation())
 		return 0;
 
@@ -486,6 +473,9 @@ static int __cmd_report(struct report *rep)
 
 	report__warn_kptr_restrict(rep);
 
+	evlist__for_each(session->evlist, pos)
+		rep->nr_entries += evsel__hists(pos)->nr_entries;
+
 	if (use_browser == 0) {
 		if (verbose > 3)
 			perf_session__fprintf(session, stdout);
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index f03854b5fb5f..30c49498495c 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -428,6 +428,8 @@ static struct hist_entry *add_hist_entry(struct hists *hists,
 	if (!he)
 		return NULL;
 
+	hists->nr_entries++;
+
 	rb_link_node(&he->rb_node_in, parent, p);
 	rb_insert_color(&he->rb_node_in, hists->entries_in);
 out:
-- 
2.1.3

[PATCH 2/7] perf report: Show progress bar for output resorting

[Namhyung Kim]

Sometimes it takes a time to resort hist entries for output in case of
a large data file.  Show a progress bar window and inform user.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-annotate.c     |  2 +-
 tools/perf/builtin-diff.c         |  2 +-
 tools/perf/builtin-report.c       | 24 ++++++++++++++++++++++--
 tools/perf/builtin-top.c          |  4 ++--
 tools/perf/tests/hists_cumulate.c |  2 +-
 tools/perf/tests/hists_filter.c   |  2 +-
 tools/perf/tests/hists_output.c   | 10 +++++-----
 tools/perf/util/hist.c            | 10 +++++++++-
 tools/perf/util/hist.h            |  2 +-
 9 files changed, 43 insertions(+), 15 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index e7417fe97a97..747f86103599 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -232,7 +232,7 @@ static int __cmd_annotate(struct perf_annotate *ann)
 		if (nr_samples > 0) {
 			total_nr_samples += nr_samples;
 			hists__collapse_resort(hists, NULL);
-			hists__output_resort(hists);
+			hists__output_resort(hists, NULL);
 
 			if (symbol_conf.event_group &&
 			    !perf_evsel__is_group_leader(pos))
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 90fd5d2f61f0..e787a4406d1f 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -605,7 +605,7 @@ static void hists__process(struct hists *hists)
 		hists__precompute(hists);
 		hists__compute_resort(hists);
 	} else {
-		hists__output_resort(hists);
+		hists__output_resort(hists, NULL);
 	}
 
 	hists__fprintf(hists, true, 0, 0, 0, stdout);
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index e2a0c2bbb3bf..2f91094e228b 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -444,6 +444,19 @@ static void report__collapse_hists(struct report *rep)
 	ui_progress__finish();
 }
 
+static void report__output_resort(struct report *rep)
+{
+	struct ui_progress prog;
+	struct perf_evsel *pos;
+
+	ui_progress__init(&prog, rep->nr_entries, "Sorting events for output...");
+
+	evlist__for_each(rep->session->evlist, pos)
+		hists__output_resort(evsel__hists(pos), &prog);
+
+	ui_progress__finish();
+}
+
 static int __cmd_report(struct report *rep)
 {
 	int ret;
@@ -495,13 +508,20 @@ static int __cmd_report(struct report *rep)
 	if (session_done())
 		return 0;
 
+	/*
+	 * recalculate number of entries after collapsing since it
+	 * might be changed during the collapse phase.
+	 */
+	rep->nr_entries = 0;
+	evlist__for_each(session->evlist, pos)
+		rep->nr_entries += evsel__hists(pos)->nr_entries;
+
 	if (rep->nr_entries == 0) {
 		ui__error("The %s file has no samples!\n", file->path);
 		return 0;
 	}
 
-	evlist__for_each(session->evlist, pos)
-		hists__output_resort(evsel__hists(pos));
+	report__output_resort(rep);
 
 	return report__browse_hists(rep);
 }
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index f1267bcf0af1..7d67e23414cc 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -285,7 +285,7 @@ static void perf_top__print_sym_table(struct perf_top *top)
 	}
 
 	hists__collapse_resort(hists, NULL);
-	hists__output_resort(hists);
+	hists__output_resort(hists, NULL);
 
 	hists__output_recalc_col_len(hists, top->print_entries - printed);
 	putchar('\n');
@@ -554,7 +554,7 @@ static void perf_top__sort_new_samples(void *arg)
 	}
 
 	hists__collapse_resort(hists, NULL);
-	hists__output_resort(hists);
+	hists__output_resort(hists, NULL);
 }
 
 static void *display_thread_tui(void *arg)
diff --git a/tools/perf/tests/hists_cumulate.c b/tools/perf/tests/hists_cumulate.c
index b6a9e2a02e46..5bb3ad42be57 100644
--- a/tools/perf/tests/hists_cumulate.c
+++ b/tools/perf/tests/hists_cumulate.c
@@ -187,7 +187,7 @@ static int do_test(struct hists *hists, struct result *expected, size_t nr_expec
 	 * function since TEST_ASSERT_VAL() returns in case of failure.
 	 */
 	hists__collapse_resort(hists, NULL);
-	hists__output_resort(hists);
+	hists__output_resort(hists, NULL);
 
 	if (verbose > 2) {
 		pr_info("use callchain: %d, cumulate callchain: %d\n",
diff --git a/tools/perf/tests/hists_filter.c b/tools/perf/tests/hists_filter.c
index 74f257a81265..59e53db7914c 100644
--- a/tools/perf/tests/hists_filter.c
+++ b/tools/perf/tests/hists_filter.c
@@ -138,7 +138,7 @@ int test__hists_filter(void)
 		struct hists *hists = evsel__hists(evsel);
 
 		hists__collapse_resort(hists, NULL);
-		hists__output_resort(hists);
+		hists__output_resort(hists, NULL);
 
 		if (verbose > 2) {
 			pr_info("Normal histogram\n");
diff --git a/tools/perf/tests/hists_output.c b/tools/perf/tests/hists_output.c
index 41328a0ed47a..b52c9faea224 100644
--- a/tools/perf/tests/hists_output.c
+++ b/tools/perf/tests/hists_output.c
@@ -152,7 +152,7 @@ static int test1(struct perf_evsel *evsel, struct machine *machine)
 		goto out;
 
 	hists__collapse_resort(hists, NULL);
-	hists__output_resort(hists);
+	hists__output_resort(hists, NULL);
 
 	if (verbose > 2) {
 		pr_info("[fields = %s, sort = %s]\n", field_order, sort_order);
@@ -252,7 +252,7 @@ static int test2(struct perf_evsel *evsel, struct machine *machine)
 		goto out;
 
 	hists__collapse_resort(hists, NULL);
-	hists__output_resort(hists);
+	hists__output_resort(hists, NULL);
 
 	if (verbose > 2) {
 		pr_info("[fields = %s, sort = %s]\n", field_order, sort_order);
@@ -306,7 +306,7 @@ static int test3(struct perf_evsel *evsel, struct machine *machine)
 		goto out;
 
 	hists__collapse_resort(hists, NULL);
-	hists__output_resort(hists);
+	hists__output_resort(hists, NULL);
 
 	if (verbose > 2) {
 		pr_info("[fields = %s, sort = %s]\n", field_order, sort_order);
@@ -384,7 +384,7 @@ static int test4(struct perf_evsel *evsel, struct machine *machine)
 		goto out;
 
 	hists__collapse_resort(hists, NULL);
-	hists__output_resort(hists);
+	hists__output_resort(hists, NULL);
 
 	if (verbose > 2) {
 		pr_info("[fields = %s, sort = %s]\n", field_order, sort_order);
@@ -487,7 +487,7 @@ static int test5(struct perf_evsel *evsel, struct machine *machine)
 		goto out;
 
 	hists__collapse_resort(hists, NULL);
-	hists__output_resort(hists);
+	hists__output_resort(hists, NULL);
 
 	if (verbose > 2) {
 		pr_info("[fields = %s, sort = %s]\n", field_order, sort_order);
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 30c49498495c..bd4a2cd73236 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -6,6 +6,7 @@
 #include "evlist.h"
 #include "evsel.h"
 #include "annotate.h"
+#include "ui/progress.h"
 #include <math.h>
 
 static bool hists__filter_entry_by_dso(struct hists *hists,
@@ -985,6 +986,7 @@ static bool hists__collapse_insert_entry(struct hists *hists __maybe_unused,
 		else
 			p = &(*p)->rb_right;
 	}
+	hists->nr_entries++;
 
 	rb_link_node(&he->rb_node_in, parent, p);
 	rb_insert_color(&he->rb_node_in, root);
@@ -1022,7 +1024,10 @@ void hists__collapse_resort(struct hists *hists, struct ui_progress *prog)
 	if (!sort__need_collapse)
 		return;
 
+	hists->nr_entries = 0;
+
 	root = hists__get_rotate_entries_in(hists);
+
 	next = rb_first(root);
 
 	while (next) {
@@ -1117,7 +1122,7 @@ static void __hists__insert_output_entry(struct rb_root *entries,
 	rb_insert_color(&he->rb_node, entries);
 }
 
-void hists__output_resort(struct hists *hists)
+void hists__output_resort(struct hists *hists, struct ui_progress *prog)
 {
 	struct rb_root *root;
 	struct rb_node *next;
@@ -1146,6 +1151,9 @@ void hists__output_resort(struct hists *hists)
 
 		if (!n->filtered)
 			hists__calc_col_len(hists, n);
+
+		if (prog)
+			ui_progress__update(prog, 1);
 	}
 }
 
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 2f9095934361..9305580cd53e 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -121,7 +121,7 @@ int hist_entry__sort_snprintf(struct hist_entry *he, char *bf, size_t size,
 			      struct hists *hists);
 void hist_entry__delete(struct hist_entry *he);
 
-void hists__output_resort(struct hists *hists);
+void hists__output_resort(struct hists *hists, struct ui_progress *prog);
 void hists__collapse_resort(struct hists *hists, struct ui_progress *prog);
 
 void hists__decay_entries(struct hists *hists, bool zap_user, bool zap_kernel);
-- 
2.1.3

[PATCH 3/7] perf ui/tui: Print backtrace symbols when segfault occurred

[Namhyung Kim]

The output will look like below.  (I added an error into ui__init()
for the test).

  $ perf report
  perf: Segmentation fault
  -------- backtrace --------
  perf[0x503781]
  /usr/lib/libc.so.6(+0x33b20)[0x7f1a14f04b20]
  perf(ui__init+0xd5)[0x503645]
  perf(setup_browser+0x97)[0x4ce4e7]
  perf(cmd_report+0xcea)[0x4392ba]
  perf[0x428493]
  perf(main+0x60a)[0x427c0a]
  /usr/lib/libc.so.6(__libc_start_main+0xf0)[0x7f1a14ef1040]
  perf[0x427d29]
  [0x0]

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/tui/setup.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
index 2f612562978c..3c38f25b1695 100644
--- a/tools/perf/ui/tui/setup.c
+++ b/tools/perf/ui/tui/setup.c
@@ -1,5 +1,8 @@
 #include <signal.h>
 #include <stdbool.h>
+#ifdef HAVE_BACKTRACE_SUPPORT
+#include <execinfo.h>
+#endif
 
 #include "../../util/cache.h"
 #include "../../util/debug.h"
@@ -88,6 +91,25 @@ int ui__getch(int delay_secs)
 	return SLkp_getkey();
 }
 
+#ifdef HAVE_BACKTRACE_SUPPORT
+static void ui__signal_backtrace(int sig)
+{
+	void *stackdump[32];
+	size_t size;
+
+	ui__exit(false);
+	psignal(sig, "perf");
+
+	printf("-------- backtrace --------\n");
+	size = backtrace(stackdump, ARRAY_SIZE(stackdump));
+	backtrace_symbols_fd(stackdump, size, STDOUT_FILENO);
+
+	exit(0);
+}
+#else
+# define ui__signal_backtrace  ui__signal
+#endif
+
 static void ui__signal(int sig)
 {
 	ui__exit(false);
@@ -122,8 +144,8 @@ int ui__init(void)
 	ui_browser__init();
 	tui_progress__init();
 
-	signal(SIGSEGV, ui__signal);
-	signal(SIGFPE, ui__signal);
+	signal(SIGSEGV, ui__signal_backtrace);
+	signal(SIGFPE, ui__signal_backtrace);
 	signal(SIGINT, ui__signal);
 	signal(SIGQUIT, ui__signal);
 	signal(SIGTERM, ui__signal);
-- 
2.1.3

[PATCH 4/7] perf diff: Fix to sort by baseline field by default

[Namhyung Kim]

The currently perf diff didn't add the baseline and delta (or other
compute) fields to the sort list so output will be sorted by other
fields like alphabetical order of DSO or symbol as below example.

Fix it by adding hpp formats for the fields and provides default
compare functions.

Before:

  $ perf diff
  # Event 'cycles'
  #
  # Baseline    Delta  Shared Object       Symbol
  # ........  .......  ..................  ...............................
  #
                       [bridge]            [k] ip_sabotage_in
                       [btrfs]             [k] __etree_search.constprop.47
       0.01%           [btrfs]             [k] btrfs_file_mmap
       0.01%   -0.01%  [btrfs]             [k] btrfs_getattr
                       [e1000e]            [k] e1000_watchdog
       0.00%           [kernel.vmlinux]    [k] PageHuge
       0.00%           [kernel.vmlinux]    [k] __acct_update_integrals
       0.00%           [kernel.vmlinux]    [k] __activate_page
                       [kernel.vmlinux]    [k] __alloc_fd
       0.02%   +0.02%  [kernel.vmlinux]    [k] __alloc_pages_nodemask
       ...

After:

  # Baseline    Delta  Shared Object       Symbol
  # ........  .......  ..................  ................................
  #
      24.73%   -4.62%  perf                [.] append_chain_children
       7.96%   -1.29%  perf                [.] dso__find_symbol
       6.97%   -2.07%  libc-2.20.so        [.] vfprintf
       4.61%   +0.88%  libc-2.20.so        [.] __fprintf_chk
       4.41%   +2.43%  perf                [.] sort__comm_cmp
       4.10%   -0.16%  perf                [.] comm__str
       4.03%   -0.93%  perf                [.] machine__findnew_thread_time
       3.82%   +3.09%  perf                [.] __hists__add_entry
       2.95%   -0.18%  perf                [.] sort__dso_cmp
       ...

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-diff.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index e787a4406d1f..318ab9c3f0ba 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -545,6 +545,42 @@ hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
 	return __hist_entry__cmp_compute(p_left, p_right, c);
 }
 
+static int64_t
+hist_entry__cmp_nop(struct hist_entry *left __maybe_unused,
+		    struct hist_entry *right __maybe_unused)
+{
+	return 0;
+}
+
+static int64_t
+hist_entry__cmp_baseline(struct hist_entry *left, struct hist_entry *right)
+{
+	if (sort_compute)
+		return 0;
+
+	if (left->stat.period == right->stat.period)
+		return 0;
+	return left->stat.period > right->stat.period ? 1 : -1;
+}
+
+static int64_t
+hist_entry__cmp_delta(struct hist_entry *left, struct hist_entry *right)
+{
+	return hist_entry__cmp_compute(right, left, COMPUTE_DELTA);
+}
+
+static int64_t
+hist_entry__cmp_ratio(struct hist_entry *left, struct hist_entry *right)
+{
+	return hist_entry__cmp_compute(right, left, COMPUTE_RATIO);
+}
+
+static int64_t
+hist_entry__cmp_wdiff(struct hist_entry *left, struct hist_entry *right)
+{
+	return hist_entry__cmp_compute(right, left, COMPUTE_WEIGHTED_DIFF);
+}
+
 static void insert_hist_entry_by_compute(struct rb_root *root,
 					 struct hist_entry *he,
 					 int c)
@@ -1038,27 +1074,35 @@ static void data__hpp_register(struct data__file *d, int idx)
 	fmt->header = hpp__header;
 	fmt->width  = hpp__width;
 	fmt->entry  = hpp__entry_global;
+	fmt->cmp    = hist_entry__cmp_nop;
+	fmt->collapse = hist_entry__cmp_nop;
 
 	/* TODO more colors */
 	switch (idx) {
 	case PERF_HPP_DIFF__BASELINE:
 		fmt->color = hpp__color_baseline;
+		fmt->sort  = hist_entry__cmp_baseline;
 		break;
 	case PERF_HPP_DIFF__DELTA:
 		fmt->color = hpp__color_delta;
+		fmt->sort  = hist_entry__cmp_delta;
 		break;
 	case PERF_HPP_DIFF__RATIO:
 		fmt->color = hpp__color_ratio;
+		fmt->sort  = hist_entry__cmp_ratio;
 		break;
 	case PERF_HPP_DIFF__WEIGHTED_DIFF:
 		fmt->color = hpp__color_wdiff;
+		fmt->sort  = hist_entry__cmp_wdiff;
 		break;
 	default:
+		fmt->sort  = hist_entry__cmp_nop;
 		break;
 	}
 
 	init_header(d, dfmt);
 	perf_hpp__column_register(fmt);
+	perf_hpp__register_sort_field(fmt);
 }
 
 static void ui_init(void)
-- 
2.1.3

[PATCH 5/7] perf diff: Get rid of hists__compute_resort()

[Namhyung Kim]

The hists__compute_resort() is to sort output fields based on the
given field/criteria.  This was done without the sort list but as we
added the field to the sort list, we can do it with normal
hists__output_resort() using the ->sort callback.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-diff.c | 59 +++--------------------------------------------
 1 file changed, 3 insertions(+), 56 deletions(-)

diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 318ab9c3f0ba..72c718e6549c 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -581,68 +581,15 @@ hist_entry__cmp_wdiff(struct hist_entry *left, struct hist_entry *right)
 	return hist_entry__cmp_compute(right, left, COMPUTE_WEIGHTED_DIFF);
 }
 
-static void insert_hist_entry_by_compute(struct rb_root *root,
-					 struct hist_entry *he,
-					 int c)
-{
-	struct rb_node **p = &root->rb_node;
-	struct rb_node *parent = NULL;
-	struct hist_entry *iter;
-
-	while (*p != NULL) {
-		parent = *p;
-		iter = rb_entry(parent, struct hist_entry, rb_node);
-		if (hist_entry__cmp_compute(he, iter, c) < 0)
-			p = &(*p)->rb_left;
-		else
-			p = &(*p)->rb_right;
-	}
-
-	rb_link_node(&he->rb_node, parent, p);
-	rb_insert_color(&he->rb_node, root);
-}
-
-static void hists__compute_resort(struct hists *hists)
-{
-	struct rb_root *root;
-	struct rb_node *next;
-
-	if (sort__need_collapse)
-		root = &hists->entries_collapsed;
-	else
-		root = hists->entries_in;
-
-	hists->entries = RB_ROOT;
-	next = rb_first(root);
-
-	hists__reset_stats(hists);
-	hists__reset_col_len(hists);
-
-	while (next != NULL) {
-		struct hist_entry *he;
-
-		he = rb_entry(next, struct hist_entry, rb_node_in);
-		next = rb_next(&he->rb_node_in);
-
-		insert_hist_entry_by_compute(&hists->entries, he, compute);
-		hists__inc_stats(hists, he);
-
-		if (!he->filtered)
-			hists__calc_col_len(hists, he);
-	}
-}
-
 static void hists__process(struct hists *hists)
 {
 	if (show_baseline_only)
 		hists__baseline_only(hists);
 
-	if (sort_compute) {
+	if (sort_compute)
 		hists__precompute(hists);
-		hists__compute_resort(hists);
-	} else {
-		hists__output_resort(hists, NULL);
-	}
+
+	hists__output_resort(hists, NULL);
 
 	hists__fprintf(hists, true, 0, 0, 0, stdout);
 }
-- 
2.1.3

[PATCH 6/7] perf tools: Append callchains only when requested

[Namhyung Kim]

The perf report --children can be called with callchain disabled so no
need to append callchains.  Actually the root of callchain tree is not
initialized properly in this case.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/hist.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index bd4a2cd73236..30ff2cb92884 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -300,7 +300,7 @@ static struct hist_entry *hist_entry__new(struct hist_entry *template,
 	size_t callchain_size = 0;
 	struct hist_entry *he;
 
-	if (symbol_conf.use_callchain || symbol_conf.cumulate_callchain)
+	if (symbol_conf.use_callchain)
 		callchain_size = sizeof(struct callchain_root);
 
 	he = zalloc(sizeof(*he) + callchain_size);
@@ -735,7 +735,7 @@ iter_add_single_cumulative_entry(struct hist_entry_iter *iter,
 	iter->he = he;
 	he_cache[iter->curr++] = he;
 
-	callchain_append(he->callchain, &callchain_cursor, sample->period);
+	hist_entry__append_callchain(he, sample);
 
 	/*
 	 * We need to re-initialize the cursor since callchain_append()
@@ -808,7 +808,8 @@ iter_add_next_cumulative_entry(struct hist_entry_iter *iter,
 	iter->he = he;
 	he_cache[iter->curr++] = he;
 
-	callchain_append(he->callchain, &cursor, sample->period);
+	if (symbol_conf.use_callchain)
+		callchain_append(he->callchain, &cursor, sample->period);
 	return 0;
 }
 
-- 
2.1.3

[PATCH 7/7] perf tools: Set attr.task bit for a tracking event

[Namhyung Kim]

The perf_event_attr.task bit is to track task (fork and exit) events
but it missed to be set by perf_evsel__config().  While it was not a
problem in practice since setting other bits (comm/mmap) ended up
being in same result, it'd be good to set it explicitly anyway.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/evsel.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 1e90c8557ede..e17d2b1624bc 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -709,6 +709,7 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts)
 	if (opts->sample_weight)
 		perf_evsel__set_sample_bit(evsel, WEIGHT);
 
+	attr->task  = track;
 	attr->mmap  = track;
 	attr->mmap2 = track && !perf_missing_features.mmap2;
 	attr->comm  = track;
-- 
2.1.3

Re: [PATCH 4/7] perf diff: Fix to sort by baseline field by default

[Arnaldo Carvalho de Melo]

Em Mon, Dec 22, 2014 at 01:44:12PM +0900, Namhyung Kim escreveu:
> The currently perf diff didn't add the baseline and delta (or other
> compute) fields to the sort list so output will be sorted by other
> fields like alphabetical order of DSO or symbol as below example.
> 
> Fix it by adding hpp formats for the fields and provides default
> compare functions.
> 
> Before:
> 
>   $ perf diff
>   # Event 'cycles'
>   #
>   # Baseline    Delta  Shared Object       Symbol
>   # ........  .......  ..................  ...............................
>   #
>                        [bridge]            [k] ip_sabotage_in
>                        [btrfs]             [k] __etree_search.constprop.47
>        0.01%           [btrfs]             [k] btrfs_file_mmap
>        0.01%   -0.01%  [btrfs]             [k] btrfs_getattr
>                        [e1000e]            [k] e1000_watchdog
>        0.00%           [kernel.vmlinux]    [k] PageHuge
>        0.00%           [kernel.vmlinux]    [k] __acct_update_integrals
>        0.00%           [kernel.vmlinux]    [k] __activate_page
>                        [kernel.vmlinux]    [k] __alloc_fd
>        0.02%   +0.02%  [kernel.vmlinux]    [k] __alloc_pages_nodemask
>        ...
> 
> After:
> 
>   # Baseline    Delta  Shared Object       Symbol
>   # ........  .......  ..................  ................................
>   #
>       24.73%   -4.62%  perf                [.] append_chain_children
>        7.96%   -1.29%  perf                [.] dso__find_symbol
>        6.97%   -2.07%  libc-2.20.so        [.] vfprintf
>        4.61%   +0.88%  libc-2.20.so        [.] __fprintf_chk
>        4.41%   +2.43%  perf                [.] sort__comm_cmp
>        4.10%   -0.16%  perf                [.] comm__str
>        4.03%   -0.93%  perf                [.] machine__findnew_thread_time
>        3.82%   +3.09%  perf                [.] __hists__add_entry
>        2.95%   -0.18%  perf                [.] sort__dso_cmp
>        ...

Looks better, here I tried running 'perf record usleep 1'  in quick
succession to then run 'perf diff'  before and after this patch, got:

Before:

[ssdandy ~]$ cat /tmp/before
# Event 'cycles'
#
# Baseline    Delta  Shared Object     Symbol                        
# ........  .......  ................  ..............................
#
            +36.29%  [kernel.vmlinux]  [k] __split_vma.isra.31       
    34.55%           [kernel.vmlinux]  [k] copy_user_generic_unrolled
             +2.45%  [kernel.vmlinux]  [k] local_clock               
     0.11%   +0.02%  [kernel.vmlinux]  [k] native_write_msr_safe     
     2.12%           [kernel.vmlinux]  [k] perf_event_comm_output    
            +61.13%  [kernel.vmlinux]  [k] unmap_page_range          
    63.22%           libc-2.17.so      [.] 0x000000000007c3e0        
[acme@ssdandy ~]$

After:

[acme@ssdandy linux]$ perf diff
# Event 'cycles'
#
# Baseline    Delta  Shared Object     Symbol                        
# ........  .......  ................  ..............................
#
    63.22%           libc-2.17.so      [.] 0x000000000007c3e0        
    34.55%           [kernel.vmlinux]  [k] copy_user_generic_unrolled
     2.12%           [kernel.vmlinux]  [k] perf_event_comm_output    
     0.11%   +0.02%  [kernel.vmlinux]  [k] native_write_msr_safe     
            +36.29%  [kernel.vmlinux]  [k] __split_vma.isra.31       
             +2.45%  [kernel.vmlinux]  [k] local_clock               
            +61.13%  [kernel.vmlinux]  [k] unmap_page_range          
[acme@ssdandy linux]$

Which was ok up to the point where symbols that only appeared on the
second run were not sorted by delta, can you fix that?

Please let me know if it is better to apply this one then a followup to
sort the deltas or if a combined patch to achieve both is best.

I.e.:

[acme@ssdandy linux]$ perf diff
# Event 'cycles'
#
# Baseline    Delta  Shared Object     Symbol                        
# ........  .......  ................  ..............................
#
    63.22%           libc-2.17.so      [.] 0x000000000007c3e0        
    34.55%           [kernel.vmlinux]  [k] copy_user_generic_unrolled
     2.12%           [kernel.vmlinux]  [k] perf_event_comm_output    
     0.11%   +0.02%  [kernel.vmlinux]  [k] native_write_msr_safe     
            +61.13%  [kernel.vmlinux]  [k] unmap_page_range          
            +36.29%  [kernel.vmlinux]  [k] __split_vma.isra.31       
             +2.45%  [kernel.vmlinux]  [k] local_clock               

[acme@ssdandy linux]$

Humm, but then wouldn't we be more interested in sorting _everything_ by
delta?

- Arnaldo

Re: [PATCH 7/7] perf tools: Set attr.task bit for a tracking event

[Arnaldo Carvalho de Melo]

Em Mon, Dec 22, 2014 at 01:44:15PM +0900, Namhyung Kim escreveu:
> The perf_event_attr.task bit is to track task (fork and exit) events
> but it missed to be set by perf_evsel__config().  While it was not a
> problem in practice since setting other bits (comm/mmap) ended up
> being in same result, it'd be good to set it explicitly anyway.

I didn't understand, so this isn't strictly needed? I.e. what is the
point of the attr->task bit then?

/me goes to check...

- Arnaldo
 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/evsel.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 1e90c8557ede..e17d2b1624bc 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -709,6 +709,7 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts)
>  	if (opts->sample_weight)
>  		perf_evsel__set_sample_bit(evsel, WEIGHT);
>  
> +	attr->task  = track;
>  	attr->mmap  = track;
>  	attr->mmap2 = track && !perf_missing_features.mmap2;
>  	attr->comm  = track;
> -- 
> 2.1.3

Re: [PATCH 7/7] perf tools: Set attr.task bit for a tracking event

[Namhyung Kim]

On Mon, Dec 22, 2014 at 11:49:19AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Dec 22, 2014 at 01:44:15PM +0900, Namhyung Kim escreveu:
> > The perf_event_attr.task bit is to track task (fork and exit) events
> > but it missed to be set by perf_evsel__config().  While it was not a
> > problem in practice since setting other bits (comm/mmap) ended up
> > being in same result, it'd be good to set it explicitly anyway.
> 
> I didn't understand, so this isn't strictly needed? I.e. what is the
> point of the attr->task bit then?

Yes, it's not strictly needed for this case.  The attr->task is to
track task related events (fork/exit) only but other meta events like
comm and mmap[2] also needs the task events.  So setting attr->comm
and/or attr->mmap causes the kernel emits the task events anyway.  So
the attr->task is only meaningful when other bits are off but I'd like
to set it for completeness.

Thanks,
Namhyung

Re: [PATCH 4/7] perf diff: Fix to sort by baseline field by default

[Namhyung Kim]

On Mon, Dec 22, 2014 at 11:45:13AM -0300, Arnaldo Carvalho de Melo wrote:
> Looks better, here I tried running 'perf record usleep 1'  in quick
> succession to then run 'perf diff'  before and after this patch, got:
> 
> Before:
> 
> [ssdandy ~]$ cat /tmp/before
> # Event 'cycles'
> #
> # Baseline    Delta  Shared Object     Symbol                        
> # ........  .......  ................  ..............................
> #
>             +36.29%  [kernel.vmlinux]  [k] __split_vma.isra.31       
>     34.55%           [kernel.vmlinux]  [k] copy_user_generic_unrolled
>              +2.45%  [kernel.vmlinux]  [k] local_clock               
>      0.11%   +0.02%  [kernel.vmlinux]  [k] native_write_msr_safe     
>      2.12%           [kernel.vmlinux]  [k] perf_event_comm_output    
>             +61.13%  [kernel.vmlinux]  [k] unmap_page_range          
>     63.22%           libc-2.17.so      [.] 0x000000000007c3e0        
> [acme@ssdandy ~]$
> 
> After:
> 
> [acme@ssdandy linux]$ perf diff
> # Event 'cycles'
> #
> # Baseline    Delta  Shared Object     Symbol                        
> # ........  .......  ................  ..............................
> #
>     63.22%           libc-2.17.so      [.] 0x000000000007c3e0        
>     34.55%           [kernel.vmlinux]  [k] copy_user_generic_unrolled
>      2.12%           [kernel.vmlinux]  [k] perf_event_comm_output    
>      0.11%   +0.02%  [kernel.vmlinux]  [k] native_write_msr_safe     
>             +36.29%  [kernel.vmlinux]  [k] __split_vma.isra.31       
>              +2.45%  [kernel.vmlinux]  [k] local_clock               
>             +61.13%  [kernel.vmlinux]  [k] unmap_page_range          
> [acme@ssdandy linux]$
> 
> Which was ok up to the point where symbols that only appeared on the
> second run were not sorted by delta, can you fix that?

I'll do that later.


> 
> Please let me know if it is better to apply this one then a followup to
> sort the deltas or if a combined patch to achieve both is best.

I prefer applying this for now and then improve sorting later..

> 
> I.e.:
> 
> [acme@ssdandy linux]$ perf diff
> # Event 'cycles'
> #
> # Baseline    Delta  Shared Object     Symbol                        
> # ........  .......  ................  ..............................
> #
>     63.22%           libc-2.17.so      [.] 0x000000000007c3e0        
>     34.55%           [kernel.vmlinux]  [k] copy_user_generic_unrolled
>      2.12%           [kernel.vmlinux]  [k] perf_event_comm_output    
>      0.11%   +0.02%  [kernel.vmlinux]  [k] native_write_msr_safe     
>             +61.13%  [kernel.vmlinux]  [k] unmap_page_range          
>             +36.29%  [kernel.vmlinux]  [k] __split_vma.isra.31       
>              +2.45%  [kernel.vmlinux]  [k] local_clock               
> 
> [acme@ssdandy linux]$
> 
> Humm, but then wouldn't we be more interested in sorting _everything_ by
> delta?

I don't understand whay you said..  what do you mean by 'everything be
delta'?  Is that something other than perf diff -o 1 ?

Thanks,
Namhyung

Re: [PATCH 7/7] perf tools: Set attr.task bit for a tracking event

[Arnaldo Carvalho de Melo]

Em Tue, Dec 23, 2014 at 01:06:33PM +0900, Namhyung Kim escreveu:
> On Mon, Dec 22, 2014 at 11:49:19AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Dec 22, 2014 at 01:44:15PM +0900, Namhyung Kim escreveu:
> > > The perf_event_attr.task bit is to track task (fork and exit) events
> > > but it missed to be set by perf_evsel__config().  While it was not a
> > > problem in practice since setting other bits (comm/mmap) ended up
> > > being in same result, it'd be good to set it explicitly anyway.
> > 
> > I didn't understand, so this isn't strictly needed? I.e. what is the
> > point of the attr->task bit then?
> 
> Yes, it's not strictly needed for this case.  The attr->task is to
> track task related events (fork/exit) only but other meta events like
> comm and mmap[2] also needs the task events.  So setting attr->comm
> and/or attr->mmap causes the kernel emits the task events anyway.  So
> the attr->task is only meaningful when other bits are off but I'd like
> to set it for completeness.

Ok, I will update the changelog comment with this information.

- Arnaldo

Re: [PATCH 4/7] perf diff: Fix to sort by baseline field by default

[Arnaldo Carvalho de Melo]

Em Tue, Dec 23, 2014 at 01:12:13PM +0900, Namhyung Kim escreveu:
> On Mon, Dec 22, 2014 at 11:45:13AM -0300, Arnaldo Carvalho de Melo wrote:
> > Looks better, here I tried running 'perf record usleep 1'  in quick
> > succession to then run 'perf diff'  before and after this patch, got:
> > 
> > Before:
> > 
> > [ssdandy ~]$ cat /tmp/before
> > # Event 'cycles'
> > #
> > # Baseline    Delta  Shared Object     Symbol                        
> > # ........  .......  ................  ..............................
> > #
> >             +36.29%  [kernel.vmlinux]  [k] __split_vma.isra.31       
> >     34.55%           [kernel.vmlinux]  [k] copy_user_generic_unrolled
> >              +2.45%  [kernel.vmlinux]  [k] local_clock               
> >      0.11%   +0.02%  [kernel.vmlinux]  [k] native_write_msr_safe     
> >      2.12%           [kernel.vmlinux]  [k] perf_event_comm_output    
> >             +61.13%  [kernel.vmlinux]  [k] unmap_page_range          
> >     63.22%           libc-2.17.so      [.] 0x000000000007c3e0        
> > [acme@ssdandy ~]$
> > 
> > After:
> > 
> > [acme@ssdandy linux]$ perf diff
> > # Event 'cycles'
> > #
> > # Baseline    Delta  Shared Object     Symbol                        
> > # ........  .......  ................  ..............................
> > #
> >     63.22%           libc-2.17.so      [.] 0x000000000007c3e0        
> >     34.55%           [kernel.vmlinux]  [k] copy_user_generic_unrolled
> >      2.12%           [kernel.vmlinux]  [k] perf_event_comm_output    
> >      0.11%   +0.02%  [kernel.vmlinux]  [k] native_write_msr_safe     
> >             +36.29%  [kernel.vmlinux]  [k] __split_vma.isra.31       
> >              +2.45%  [kernel.vmlinux]  [k] local_clock               
> >             +61.13%  [kernel.vmlinux]  [k] unmap_page_range          
> > [acme@ssdandy linux]$
> > 
> > Which was ok up to the point where symbols that only appeared on the
> > second run were not sorted by delta, can you fix that?
 
> I'll do that later.

Ok, thanks for considering doing that!
 
> > Please let me know if it is better to apply this one then a followup to
> > sort the deltas or if a combined patch to achieve both is best.
> 
> I prefer applying this for now and then improve sorting later..

Ok, will apply it, thanks for checking.
 
> > I.e.:
> > 
> > [acme@ssdandy linux]$ perf diff
> > # Event 'cycles'
> > #
> > # Baseline    Delta  Shared Object     Symbol                        
> > # ........  .......  ................  ..............................
> > #
> >     63.22%           libc-2.17.so      [.] 0x000000000007c3e0        
> >     34.55%           [kernel.vmlinux]  [k] copy_user_generic_unrolled
> >      2.12%           [kernel.vmlinux]  [k] perf_event_comm_output    
> >      0.11%   +0.02%  [kernel.vmlinux]  [k] native_write_msr_safe     
> >             +61.13%  [kernel.vmlinux]  [k] unmap_page_range          
> >             +36.29%  [kernel.vmlinux]  [k] __split_vma.isra.31       
> >              +2.45%  [kernel.vmlinux]  [k] local_clock               

> > [acme@ssdandy linux]$

> > Humm, but then wouldn't we be more interested in sorting _everything_ by
> > delta?

> I don't understand whay you said..  what do you mean by 'everything be
> delta'?  Is that something other than perf diff -o 1 ?

A-ha, exactly, thanks for educating me! :-)

- Arnaldo

[tip:perf/urgent] perf report: Show progress bar for output resorting

[tip-bot for Namhyung Kim]

Commit-ID:  740b97f9509ac5a015278940747178af4eb0900d
Gitweb:     http://git.kernel.org/tip/740b97f9509ac5a015278940747178af4eb0900d
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Mon, 22 Dec 2014 13:44:10 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 23 Dec 2014 12:01:37 -0300

perf report: Show progress bar for output resorting

Sometimes it takes a long time to resort hist entries for output in case
of a large data file.  Show a progress bar window and inform user.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1419223455-4362-3-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-annotate.c     |  2 +-
 tools/perf/builtin-diff.c         |  2 +-
 tools/perf/builtin-report.c       | 24 ++++++++++++++++++++++--
 tools/perf/builtin-top.c          |  4 ++--
 tools/perf/tests/hists_cumulate.c |  2 +-
 tools/perf/tests/hists_filter.c   |  2 +-
 tools/perf/tests/hists_output.c   | 10 +++++-----
 tools/perf/util/hist.c            | 10 +++++++++-
 tools/perf/util/hist.h            |  2 +-
 9 files changed, 43 insertions(+), 15 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index e7417fe..747f861 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -232,7 +232,7 @@ static int __cmd_annotate(struct perf_annotate *ann)
 		if (nr_samples > 0) {
 			total_nr_samples += nr_samples;
 			hists__collapse_resort(hists, NULL);
-			hists__output_resort(hists);
+			hists__output_resort(hists, NULL);
 
 			if (symbol_conf.event_group &&
 			    !perf_evsel__is_group_leader(pos))
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 1ce425d..303c1e1 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -605,7 +605,7 @@ static void hists__process(struct hists *hists)
 		hists__precompute(hists);
 		hists__compute_resort(hists);
 	} else {
-		hists__output_resort(hists);
+		hists__output_resort(hists, NULL);
 	}
 
 	hists__fprintf(hists, true, 0, 0, 0, stdout);
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 3936760..072ae8a 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -457,6 +457,19 @@ static void report__collapse_hists(struct report *rep)
 	ui_progress__finish();
 }
 
+static void report__output_resort(struct report *rep)
+{
+	struct ui_progress prog;
+	struct perf_evsel *pos;
+
+	ui_progress__init(&prog, rep->nr_entries, "Sorting events for output...");
+
+	evlist__for_each(rep->session->evlist, pos)
+		hists__output_resort(evsel__hists(pos), &prog);
+
+	ui_progress__finish();
+}
+
 static int __cmd_report(struct report *rep)
 {
 	int ret;
@@ -505,13 +518,20 @@ static int __cmd_report(struct report *rep)
 	if (session_done())
 		return 0;
 
+	/*
+	 * recalculate number of entries after collapsing since it
+	 * might be changed during the collapse phase.
+	 */
+	rep->nr_entries = 0;
+	evlist__for_each(session->evlist, pos)
+		rep->nr_entries += evsel__hists(pos)->nr_entries;
+
 	if (rep->nr_entries == 0) {
 		ui__error("The %s file has no samples!\n", file->path);
 		return 0;
 	}
 
-	evlist__for_each(session->evlist, pos)
-		hists__output_resort(evsel__hists(pos));
+	report__output_resort(rep);
 
 	return report__browse_hists(rep);
 }
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 0aa7747..961cea1 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -285,7 +285,7 @@ static void perf_top__print_sym_table(struct perf_top *top)
 	}
 
 	hists__collapse_resort(hists, NULL);
-	hists__output_resort(hists);
+	hists__output_resort(hists, NULL);
 
 	hists__output_recalc_col_len(hists, top->print_entries - printed);
 	putchar('\n');
@@ -554,7 +554,7 @@ static void perf_top__sort_new_samples(void *arg)
 	}
 
 	hists__collapse_resort(hists, NULL);
-	hists__output_resort(hists);
+	hists__output_resort(hists, NULL);
 }
 
 static void *display_thread_tui(void *arg)
diff --git a/tools/perf/tests/hists_cumulate.c b/tools/perf/tests/hists_cumulate.c
index 614d5c4..4b8226e 100644
--- a/tools/perf/tests/hists_cumulate.c
+++ b/tools/perf/tests/hists_cumulate.c
@@ -187,7 +187,7 @@ static int do_test(struct hists *hists, struct result *expected, size_t nr_expec
 	 * function since TEST_ASSERT_VAL() returns in case of failure.
 	 */
 	hists__collapse_resort(hists, NULL);
-	hists__output_resort(hists);
+	hists__output_resort(hists, NULL);
 
 	if (verbose > 2) {
 		pr_info("use callchain: %d, cumulate callchain: %d\n",
diff --git a/tools/perf/tests/hists_filter.c b/tools/perf/tests/hists_filter.c
index 74f257a..59e53db 100644
--- a/tools/perf/tests/hists_filter.c
+++ b/tools/perf/tests/hists_filter.c
@@ -138,7 +138,7 @@ int test__hists_filter(void)
 		struct hists *hists = evsel__hists(evsel);
 
 		hists__collapse_resort(hists, NULL);
-		hists__output_resort(hists);
+		hists__output_resort(hists, NULL);
 
 		if (verbose > 2) {
 			pr_info("Normal histogram\n");
diff --git a/tools/perf/tests/hists_output.c b/tools/perf/tests/hists_output.c
index a748f2b..f554761 100644
--- a/tools/perf/tests/hists_output.c
+++ b/tools/perf/tests/hists_output.c
@@ -152,7 +152,7 @@ static int test1(struct perf_evsel *evsel, struct machine *machine)
 		goto out;
 
 	hists__collapse_resort(hists, NULL);
-	hists__output_resort(hists);
+	hists__output_resort(hists, NULL);
 
 	if (verbose > 2) {
 		pr_info("[fields = %s, sort = %s]\n", field_order, sort_order);
@@ -252,7 +252,7 @@ static int test2(struct perf_evsel *evsel, struct machine *machine)
 		goto out;
 
 	hists__collapse_resort(hists, NULL);
-	hists__output_resort(hists);
+	hists__output_resort(hists, NULL);
 
 	if (verbose > 2) {
 		pr_info("[fields = %s, sort = %s]\n", field_order, sort_order);
@@ -306,7 +306,7 @@ static int test3(struct perf_evsel *evsel, struct machine *machine)
 		goto out;
 
 	hists__collapse_resort(hists, NULL);
-	hists__output_resort(hists);
+	hists__output_resort(hists, NULL);
 
 	if (verbose > 2) {
 		pr_info("[fields = %s, sort = %s]\n", field_order, sort_order);
@@ -384,7 +384,7 @@ static int test4(struct perf_evsel *evsel, struct machine *machine)
 		goto out;
 
 	hists__collapse_resort(hists, NULL);
-	hists__output_resort(hists);
+	hists__output_resort(hists, NULL);
 
 	if (verbose > 2) {
 		pr_info("[fields = %s, sort = %s]\n", field_order, sort_order);
@@ -487,7 +487,7 @@ static int test5(struct perf_evsel *evsel, struct machine *machine)
 		goto out;
 
 	hists__collapse_resort(hists, NULL);
-	hists__output_resort(hists);
+	hists__output_resort(hists, NULL);
 
 	if (verbose > 2) {
 		pr_info("[fields = %s, sort = %s]\n", field_order, sort_order);
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 6e88b9e..1cc6ea4 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -6,6 +6,7 @@
 #include "evlist.h"
 #include "evsel.h"
 #include "annotate.h"
+#include "ui/progress.h"
 #include <math.h>
 
 static bool hists__filter_entry_by_dso(struct hists *hists,
@@ -987,6 +988,7 @@ static bool hists__collapse_insert_entry(struct hists *hists __maybe_unused,
 		else
 			p = &(*p)->rb_right;
 	}
+	hists->nr_entries++;
 
 	rb_link_node(&he->rb_node_in, parent, p);
 	rb_insert_color(&he->rb_node_in, root);
@@ -1024,7 +1026,10 @@ void hists__collapse_resort(struct hists *hists, struct ui_progress *prog)
 	if (!sort__need_collapse)
 		return;
 
+	hists->nr_entries = 0;
+
 	root = hists__get_rotate_entries_in(hists);
+
 	next = rb_first(root);
 
 	while (next) {
@@ -1119,7 +1124,7 @@ static void __hists__insert_output_entry(struct rb_root *entries,
 	rb_insert_color(&he->rb_node, entries);
 }
 
-void hists__output_resort(struct hists *hists)
+void hists__output_resort(struct hists *hists, struct ui_progress *prog)
 {
 	struct rb_root *root;
 	struct rb_node *next;
@@ -1148,6 +1153,9 @@ void hists__output_resort(struct hists *hists)
 
 		if (!n->filtered)
 			hists__calc_col_len(hists, n);
+
+		if (prog)
+			ui_progress__update(prog, 1);
 	}
 }
 
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index d0ef9a1..46bd503 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -121,7 +121,7 @@ int hist_entry__sort_snprintf(struct hist_entry *he, char *bf, size_t size,
 			      struct hists *hists);
 void hist_entry__free(struct hist_entry *);
 
-void hists__output_resort(struct hists *hists);
+void hists__output_resort(struct hists *hists, struct ui_progress *prog);
 void hists__collapse_resort(struct hists *hists, struct ui_progress *prog);
 
 void hists__decay_entries(struct hists *hists, bool zap_user, bool zap_kernel);

[tip:perf/urgent] perf ui/tui: Print backtrace symbols when segfault occurs

[tip-bot for Namhyung Kim]

Commit-ID:  b11bc8e28f4829f693ef6c0178fe1811386ac828
Gitweb:     http://git.kernel.org/tip/b11bc8e28f4829f693ef6c0178fe1811386ac828
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Mon, 22 Dec 2014 13:44:11 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 23 Dec 2014 12:05:59 -0300

perf ui/tui: Print backtrace symbols when segfault occurs

The output will look like below.  (I added an error into ui__init() for
the test).

  $ perf report
  perf: Segmentation fault
  -------- backtrace --------
  perf[0x503781]
  /usr/lib/libc.so.6(+0x33b20)[0x7f1a14f04b20]
  perf(ui__init+0xd5)[0x503645]
  perf(setup_browser+0x97)[0x4ce4e7]
  perf(cmd_report+0xcea)[0x4392ba]
  perf[0x428493]
  perf(main+0x60a)[0x427c0a]
  /usr/lib/libc.so.6(__libc_start_main+0xf0)[0x7f1a14ef1040]
  perf[0x427d29]
  [0x0]

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1419223455-4362-4-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/tui/setup.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
index 2f61256..3c38f25 100644
--- a/tools/perf/ui/tui/setup.c
+++ b/tools/perf/ui/tui/setup.c
@@ -1,5 +1,8 @@
 #include <signal.h>
 #include <stdbool.h>
+#ifdef HAVE_BACKTRACE_SUPPORT
+#include <execinfo.h>
+#endif
 
 #include "../../util/cache.h"
 #include "../../util/debug.h"
@@ -88,6 +91,25 @@ int ui__getch(int delay_secs)
 	return SLkp_getkey();
 }
 
+#ifdef HAVE_BACKTRACE_SUPPORT
+static void ui__signal_backtrace(int sig)
+{
+	void *stackdump[32];
+	size_t size;
+
+	ui__exit(false);
+	psignal(sig, "perf");
+
+	printf("-------- backtrace --------\n");
+	size = backtrace(stackdump, ARRAY_SIZE(stackdump));
+	backtrace_symbols_fd(stackdump, size, STDOUT_FILENO);
+
+	exit(0);
+}
+#else
+# define ui__signal_backtrace  ui__signal
+#endif
+
 static void ui__signal(int sig)
 {
 	ui__exit(false);
@@ -122,8 +144,8 @@ int ui__init(void)
 	ui_browser__init();
 	tui_progress__init();
 
-	signal(SIGSEGV, ui__signal);
-	signal(SIGFPE, ui__signal);
+	signal(SIGSEGV, ui__signal_backtrace);
+	signal(SIGFPE, ui__signal_backtrace);
 	signal(SIGINT, ui__signal);
 	signal(SIGQUIT, ui__signal);
 	signal(SIGTERM, ui__signal);

[tip:perf/urgent] perf callchain: Append callchains only when requested

[tip-bot for Namhyung Kim]

Commit-ID:  82aa019e0098a1e0801df94345c0297448323126
Gitweb:     http://git.kernel.org/tip/82aa019e0098a1e0801df94345c0297448323126
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Mon, 22 Dec 2014 13:44:14 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 23 Dec 2014 12:06:39 -0300

perf callchain: Append callchains only when requested

The perf report --children can be called with callchain disabled so no
need to append callchains.  Actually the root of callchain tree is not
initialized properly in this case.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1419223455-4362-7-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hist.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 1cc6ea4..0ced178 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -304,7 +304,7 @@ static struct hist_entry *hist_entry__new(struct hist_entry *template,
 	size_t callchain_size = 0;
 	struct hist_entry *he;
 
-	if (symbol_conf.use_callchain || symbol_conf.cumulate_callchain)
+	if (symbol_conf.use_callchain)
 		callchain_size = sizeof(struct callchain_root);
 
 	he = zalloc(sizeof(*he) + callchain_size);
@@ -737,7 +737,7 @@ iter_add_single_cumulative_entry(struct hist_entry_iter *iter,
 	iter->he = he;
 	he_cache[iter->curr++] = he;
 
-	callchain_append(he->callchain, &callchain_cursor, sample->period);
+	hist_entry__append_callchain(he, sample);
 
 	/*
 	 * We need to re-initialize the cursor since callchain_append()
@@ -810,7 +810,8 @@ iter_add_next_cumulative_entry(struct hist_entry_iter *iter,
 	iter->he = he;
 	he_cache[iter->curr++] = he;
 
-	callchain_append(he->callchain, &cursor, sample->period);
+	if (symbol_conf.use_callchain)
+		callchain_append(he->callchain, &cursor, sample->period);
 	return 0;
 }
 

Re: [PATCH 6/7] perf tools: Append callchains only when requested

[Arnaldo Carvalho de Melo]

Em Mon, Dec 22, 2014 at 01:44:14PM +0900, Namhyung Kim escreveu:
> The perf report --children can be called with callchain disabled so no
> need to append callchains.  Actually the root of callchain tree is not
> initialized properly in this case.

Hi Namhyung,

	I should have caught this using 'perf test', but it slipped
thru, please try running 'perf test cumulation', I noticed that it fails
and that if I revert this changeset it works again, can you please
check?

- Arnaldo
 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/hist.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index bd4a2cd73236..30ff2cb92884 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -300,7 +300,7 @@ static struct hist_entry *hist_entry__new(struct hist_entry *template,
>  	size_t callchain_size = 0;
>  	struct hist_entry *he;
>  
> -	if (symbol_conf.use_callchain || symbol_conf.cumulate_callchain)
> +	if (symbol_conf.use_callchain)
>  		callchain_size = sizeof(struct callchain_root);
>  
>  	he = zalloc(sizeof(*he) + callchain_size);
> @@ -735,7 +735,7 @@ iter_add_single_cumulative_entry(struct hist_entry_iter *iter,
>  	iter->he = he;
>  	he_cache[iter->curr++] = he;
>  
> -	callchain_append(he->callchain, &callchain_cursor, sample->period);
> +	hist_entry__append_callchain(he, sample);
>  
>  	/*
>  	 * We need to re-initialize the cursor since callchain_append()
> @@ -808,7 +808,8 @@ iter_add_next_cumulative_entry(struct hist_entry_iter *iter,
>  	iter->he = he;
>  	he_cache[iter->curr++] = he;
>  
> -	callchain_append(he->callchain, &cursor, sample->period);
> +	if (symbol_conf.use_callchain)
> +		callchain_append(he->callchain, &cursor, sample->period);
>  	return 0;
>  }
>  
> -- 
> 2.1.3

Re: [PATCH 6/7] perf tools: Append callchains only when requested

[Namhyung Kim]

Hi Arnaldo,

On Sat, Jan 3, 2015 at 11:25 AM, Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
> Em Mon, Dec 22, 2014 at 01:44:14PM +0900, Namhyung Kim escreveu:
>> The perf report --children can be called with callchain disabled so no
>> need to append callchains.  Actually the root of callchain tree is not
>> initialized properly in this case.
>
> Hi Namhyung,
>
>         I should have caught this using 'perf test', but it slipped
> thru, please try running 'perf test cumulation', I noticed that it fails
> and that if I revert this changeset it works again, can you please
> check?

This change should not affect the test result, but I guess the test
somehow relied on the undefined behavior.  Actually I posted a patch
to fix this problem.  So please take a look at the link below:

https://lkml.org/lkml/2014/12/22/561

Thanks,
Namhyung

Re: [PATCH 6/7] perf tools: Append callchains only when requested

[Arnaldo Carvalho de Melo]

Em Sun, Jan 04, 2015 at 12:01:26AM +0900, Namhyung Kim escreveu:
> On Sat, Jan 3, 2015 at 11:25 AM, Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> > Em Mon, Dec 22, 2014 at 01:44:14PM +0900, Namhyung Kim escreveu:
> >> The perf report --children can be called with callchain disabled so no
> >> need to append callchains.  Actually the root of callchain tree is not
> >> initialized properly in this case.

> >         I should have caught this using 'perf test', but it slipped
> > thru, please try running 'perf test cumulation', I noticed that it fails
> > and that if I revert this changeset it works again, can you please
> > check?
 
> This change should not affect the test result, but I guess the test
> somehow relied on the undefined behavior.  Actually I posted a patch
> to fix this problem.  So please take a look at the link below:
 
> https://lkml.org/lkml/2014/12/22/561

Ok, missed this one, will get it on perf/urgent and look if there are
patches pending merging in that pull req.

Thanks,

- Arnaldo

Re: [PATCH 6/7] perf tools: Append callchains only when requested

[Namhyung Kim]

Hi Arnaldo,

On Mon, Jan 05, 2015 at 09:49:24AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Sun, Jan 04, 2015 at 12:01:26AM +0900, Namhyung Kim escreveu:
> > On Sat, Jan 3, 2015 at 11:25 AM, Arnaldo Carvalho de Melo
> > <acme@kernel.org> wrote:
> > > Em Mon, Dec 22, 2014 at 01:44:14PM +0900, Namhyung Kim escreveu:
> > >> The perf report --children can be called with callchain disabled so no
> > >> need to append callchains.  Actually the root of callchain tree is not
> > >> initialized properly in this case.
> 
> > >         I should have caught this using 'perf test', but it slipped
> > > thru, please try running 'perf test cumulation', I noticed that it fails
> > > and that if I revert this changeset it works again, can you please
> > > check?
>  
> > This change should not affect the test result, but I guess the test
> > somehow relied on the undefined behavior.  Actually I posted a patch
> > to fix this problem.  So please take a look at the link below:
>  
> > https://lkml.org/lkml/2014/12/22/561
> 
> Ok, missed this one, will get it on perf/urgent and look if there are
> patches pending merging in that pull req.

Could please consider applying it below:

https://lkml.org/lkml/2014/12/24/13

Thanks,
Namhyung

[tip:perf/core] perf report: Get rid of report__inc_stat()

[tip-bot for Namhyung Kim]

Commit-ID:  590cd344e2099c7b040b29d3a711b4c26358def5
Gitweb:     http://git.kernel.org/tip/590cd344e2099c7b040b29d3a711b4c26358def5
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Mon, 22 Dec 2014 13:44:09 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 21 Jan 2015 13:24:32 -0300

perf report: Get rid of report__inc_stat()

The report__inc_stat() function collects the number of hist entries in
the session in order to calculate the max size of the progess bar.

It'd be better if it does it during the addition of hist entries so that
it can be used by other places too.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1419223455-4362-2-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-report.c | 16 +++-------------
 tools/perf/util/hist.c      |  2 ++
 2 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 072ae8a..2f91094 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -86,17 +86,6 @@ static int report__config(const char *var, const char *value, void *cb)
 	return perf_default_config(var, value, cb);
 }
 
-static void report__inc_stats(struct report *rep, struct hist_entry *he)
-{
-	/*
-	 * The @he is either of a newly created one or an existing one
-	 * merging current sample.  We only want to count a new one so
-	 * checking ->nr_events being 1.
-	 */
-	if (he->stat.nr_events == 1)
-		rep->nr_entries++;
-}
-
 static int hist_iter__report_callback(struct hist_entry_iter *iter,
 				      struct addr_location *al, bool single,
 				      void *arg)
@@ -108,8 +97,6 @@ static int hist_iter__report_callback(struct hist_entry_iter *iter,
 	struct mem_info *mi;
 	struct branch_info *bi;
 
-	report__inc_stats(rep, he);
-
 	if (!ui__has_annotation())
 		return 0;
 
@@ -499,6 +486,9 @@ static int __cmd_report(struct report *rep)
 
 	report__warn_kptr_restrict(rep);
 
+	evlist__for_each(session->evlist, pos)
+		rep->nr_entries += evsel__hists(pos)->nr_entries;
+
 	if (use_browser == 0) {
 		if (verbose > 3)
 			perf_session__fprintf(session, stdout);
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 038483a..e17163f 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -429,6 +429,8 @@ static struct hist_entry *add_hist_entry(struct hists *hists,
 	if (!he)
 		return NULL;
 
+	hists->nr_entries++;
+
 	rb_link_node(&he->rb_node_in, parent, p);
 	rb_insert_color(&he->rb_node_in, hists->entries_in);
 out: