Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1335433 > unrolled thread

[PATCHSET 00/25] perf tools: Add support for hierachy view (v6)

Back to article view | Back to linux.kernel

Contents

  [PATCHSET 00/25] perf tools: Add support for hierachy view (v6) Namhyung Kim <namhyung@kernel.org> - 2016-02-16 15:20 +0100
    [PATCH v6 23/25] perf report: Add --hierarchy option Namhyung Kim <namhyung@kernel.org> - 2016-02-16 15:20 +0100
    [PATCH v6 24/25] perf hists: Support decaying in hierarchy mode Namhyung Kim <namhyung@kernel.org> - 2016-02-16 15:20 +0100
    [PATCH v6 14/25] perf hists: Resort after filtering hierarchy Namhyung Kim <namhyung@kernel.org> - 2016-02-16 15:20 +0100
    [PATCH v6 05/25] perf callchain: Check return value of split_add_child() Namhyung Kim <namhyung@kernel.org> - 2016-02-16 15:20 +0100
      [tip:perf/core] perf callchain:   Check return value of split_add_child() tip-bot for Namhyung Kim <tipbot@zytor.com> - 2016-02-20 12:50 +0100
    [PATCH v6 13/25] perf hists: Support filtering in hierarchy mode Namhyung Kim <namhyung@kernel.org> - 2016-02-16 15:20 +0100
    [PATCH v6 04/25] perf callchain: Add enum match_result for match_chain() Namhyung Kim <namhyung@kernel.org> - 2016-02-16 15:20 +0100
      [tip:perf/core] perf callchain:   Add enum match_result for match_chain() tip-bot for Namhyung Kim <tipbot@zytor.com> - 2016-02-20 12:50 +0100
    [PATCH v6 21/25] perf hists browser: Align column header in hierarchy mode Namhyung Kim <namhyung@kernel.org> - 2016-02-16 15:20 +0100
    [PATCH v6 03/25] perf callchain: Check return value of fill_node() Namhyung Kim <namhyung@kernel.org> - 2016-02-16 15:20 +0100
      [tip:perf/core] perf callchain: Check return value of fill_node() tip-bot for Namhyung Kim <tipbot@zytor.com> - 2016-02-20 12:50 +0100
    [PATCH v6 02/25] perf callchain: Check return value of add_child() Namhyung Kim <namhyung@kernel.org> - 2016-02-16 15:20 +0100
      [tip:perf/core] perf callchain: Check return value of add_child() tip-bot for Namhyung Kim <tipbot@zytor.com> - 2016-02-20 12:50 +0100
    [PATCH v6 25/25] perf top: Add --hierarchy option Namhyung Kim <namhyung@kernel.org> - 2016-02-16 15:20 +0100
      Re: [PATCH v6 25/25] perf top: Add --hierarchy option Jiri Olsa <jolsa@redhat.com> - 2016-02-21 00:20 +0100
        Re: [PATCH v6 25/25] perf top: Add --hierarchy option Namhyung Kim <namhyung@kernel.org> - 2016-02-21 19:20 +0100
    [PATCH v6 16/25] perf ui/stdio: Implement hierarchy output mode Namhyung Kim <namhyung@kernel.org> - 2016-02-16 15:20 +0100
      Re: [PATCH v6 16/25] perf ui/stdio: Implement hierarchy output mode Jiri Olsa <jolsa@redhat.com> - 2016-02-21 00:20 +0100
        Re: [PATCH v6 16/25] perf ui/stdio: Implement hierarchy output mode Namhyung Kim <namhyung@kernel.org> - 2016-02-21 19:10 +0100
    [PATCH v6 09/25] perf hists: Basic support of hierarchical report view Namhyung Kim <namhyung@kernel.org> - 2016-02-16 15:20 +0100
      Re: [PATCH v6 09/25] perf hists: Basic support of hierarchical  report view Jiri Olsa <jolsa@redhat.com> - 2016-02-21 00:20 +0100
        Re: [PATCH v6 09/25] perf hists: Basic support of hierarchical  report view Namhyung Kim <namhyung@kernel.org> - 2016-02-21 21:00 +0100
    [PATCH v6 20/25] perf hists browser: Implement hierarchy output Namhyung Kim <namhyung@kernel.org> - 2016-02-16 15:20 +0100
      Re: [PATCH v6 20/25] perf hists browser: Implement hierarchy output Jiri Olsa <jolsa@redhat.com> - 2016-02-21 00:20 +0100
    [PATCH v6 19/25] perf hists browser: Support collapsing/expanding whole entries in hierarchy Namhyung Kim <namhyung@kernel.org> - 2016-02-16 15:20 +0100
    [PATCH v6 08/25] perf report: Check error during report__collapse_hists() Namhyung Kim <namhyung@kernel.org> - 2016-02-16 15:20 +0100
      [tip:perf/core] perf report:   Check error during report__collapse_hists() tip-bot for Namhyung Kim <tipbot@zytor.com> - 2016-02-20 12:50 +0100

Page 1 of 2  [1] 2  Next page →

#1335433 — [PATCHSET 00/25] perf tools: Add support for hierachy view (v6)

Hello,

This patchset implements a new feature that collects hist entries in a
hierachical manner.  That means lower-level entries belong to an
upper-level entry.  The entry hierachy is built on the sort keys
given, so users can set it whatever they want.  It only shows
top-level entries first, and user can expand/collapse it dynamically.

The patch 01 is a bug fix and can be applied separately.  The patch 02
to 08 are preparation of the hierarchy patchset and it handles the
error path properly.  The patch 09 to 15 implements basic logic of the
hierarchy mode and the rest adds support for each UI.

 * Changes from v5)
  - separate resort after filter  (Jiri)
  - count sort keys when register  (Jiri)
  - add enum hierarchy_move_dir  (Jiri)

 * Changes from v4)
  - rebased onto the current acme/perf/core
  - fix memory leak on callchian_merge error path  (Arnaldo)
  - fix a bug on perf-top regarding percent calculation
  - split hierarchy filtering code
 
 * Changes from v3)
  - rebased onto the percent limit patchset v2

 * Changes from v2)
  - check memory allocation failure in hists__hierarchy_insert_entry  (Jiri)
  - remove unused rb_hierarchy_first()  (Arnaldo)
  - support callchain percent limit  (Andi)
  - break TUI context menu cleanup  (Arnaldo)
  

This time I implemented it for every output browser including TUI.
A screenshot on TUI looks like below:

For normal output:

  $ perf report --tui
  Samples: 3K of event 'cycles:pp', Event count (approx.): 1695979674
    Overhead  Command        Shared Object         Symbol
  ------------------------------------------------------------------------
  -    7.57%  swapper        [kernel.vmlinux]      [k] intel_idle
       intel_idle
       cpuidle_enter_state
       cpuidle_enter
       call_cpuidle
     + cpu_startup_entry
  +    1.16   firefox        firefox               [.] 0x00000000000019433
  +    0.97%  firefox        libpthread-2.22.so    [.] pthread_mutex_lock
  ...


With hierarchy view,

  $ perf report --tui --hierarchy
  Samples: 3K of event 'cycles:pp', Event count (approx.): 1695979674
   Overhead        Command / Shared Object / Symbol
  -------------------------------------------------------------------
  +  76.30%        firefox
  -   9.95%        swapper
     -   9.51%        [kernel.vmlinux]
        -   7.57         [k] intel_idle
	     intel_idle
	     cpuidle_enter_state
	     cpuidle_enter
	     call_cpuidle
	   + cpu_startup_entry
	+   0.15%        [k] __schedule
	+   0.12%        [k] menu_select
	...
     +   0.34%        [sdhci]
     +   0.06%        [e1000e]
     ...
 +    5.65%        Xorg
 +    5.42%        Socket Thread
 ...

As you can see, overhead of an upper level entry is the sum of
overhead of lower level entries.  The entries are aligned by its order
of matching sort keys.

This is available from 'perf/hierarchy-v6' branch in my tree:

  git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git


Any comments are welcome, thanks!
Namhyung


Cc: Don Zickus <dzickus@redhat.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: Moinuddin Quadri <moin18@gmail.com>


Namhyung Kim (25):
  perf hists browser: Fix percentage update on key press
  perf callchain: Check return value of add_child()
  perf callchain: Check return value of fill_node()
  perf callchain: Add enum match_result for match_chain()
  perf callchain: Check return value of split_add_child()
  perf callchain: Check return value of append_chain_children()
  perf hists: Return error from hists__collapse_resort()
  perf report: Check error during report__collapse_hists()
  perf hists: Basic support of hierarchical report view
  perf hists: Resort hist entries with hierarchy
  perf hists: Add helper functions for hierarchy mode
  perf hists: Introduce hist_entry__filter()
  perf hists: Support filtering in hierarchy mode
  perf hists: Resort after filtering hierarchy
  perf hists: Count number of sort keys
  perf ui/stdio: Implement hierarchy output mode
  perf ui/stdio: Align column header for hierarchy output
  perf hists browser: Count number of hierarchy entries
  perf hists browser: Support collapsing/expanding whole entries in
    hierarchy
  perf hists browser: Implement hierarchy output
  perf hists browser: Align column header in hierarchy mode
  perf ui/gtk: Implement hierarchy output mode
  perf report: Add --hierarchy option
  perf hists: Support decaying in hierarchy mode
  perf top: Add --hierarchy option

 tools/perf/Documentation/perf-report.txt |   3 +
 tools/perf/Documentation/perf-top.txt    |   3 +
 tools/perf/Documentation/tips.txt        |   1 +
 tools/perf/builtin-report.c              |  31 +-
 tools/perf/builtin-top.c                 |  15 +
 tools/perf/ui/browsers/hists.c           | 481 ++++++++++++++++++++++++++---
 tools/perf/ui/gtk/hists.c                | 161 +++++++++-
 tools/perf/ui/hist.c                     |   3 +
 tools/perf/ui/stdio/hist.c               | 175 ++++++++++-
 tools/perf/util/callchain.c              | 102 +++++--
 tools/perf/util/ctype.c                  |   9 +
 tools/perf/util/hist.c                   | 498 ++++++++++++++++++++++++++++---
 tools/perf/util/hist.h                   |  27 +-
 tools/perf/util/sort.c                   | 113 +++++++
 tools/perf/util/sort.h                   |  14 +-
 tools/perf/util/symbol.h                 |   3 +-
 tools/perf/util/util.h                   |   2 +
 17 files changed, 1524 insertions(+), 117 deletions(-)

-- 
2.7.1

[toc] | [next] | [standalone]

#1335434 — [PATCH v6 23/25] perf report: Add --hierarchy option

The --hierarchy option is to show output in hierarchy mode.  It extends
folding/unfolding in the TUI and GTK browsers to support sort items as
well as callchains.  Users can toggle the items to see the performance
result at wanted level.

  $ perf report --hierarchy --tui
   Overhead        Command / Shared Object / Symbol
  +  32.96%        gnome-shell
  -  15.11%        swapper
     -  14.97%        [kernel.vmlinux]
            6.82%        [k] intel_idle
	    0.66%        [k] menu_select
	    0.43%        [k] __hrtimer_start_range_ns
  ...

Acked-by: Pekka Enberg <penberg@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Documentation/perf-report.txt |  3 +++
 tools/perf/Documentation/tips.txt        |  1 +
 tools/perf/builtin-report.c              | 17 +++++++++++++++++
 3 files changed, 21 insertions(+)

diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
index 89cab84e92fd..12113992ac9d 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -401,6 +401,9 @@ include::itrace.txt[]
 --raw-trace::
 	When displaying traceevent output, do not use print fmt or plugins.
 
+--hierarchy::
+	Enable hierarchical output.
+
 include::callchain-overhead-calculation.txt[]
 
 SEE ALSO
diff --git a/tools/perf/Documentation/tips.txt b/tools/perf/Documentation/tips.txt
index e0ce9573b79b..5950b5a24efd 100644
--- a/tools/perf/Documentation/tips.txt
+++ b/tools/perf/Documentation/tips.txt
@@ -27,3 +27,4 @@ Skip collecing build-id when recording: perf record -B
 To change sampling frequency to 100 Hz: perf record -F 100
 See assembly instructions with percentage: perf annotate <symbol>
 If you prefer Intel style assembly, try: perf annotate -M intel
+For hierarchical output, try: perf report --hierarchy
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 760e886ca9d9..f4d8244449ca 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -811,6 +811,8 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 		    "only show processor socket that match with this filter"),
 	OPT_BOOLEAN(0, "raw-trace", &symbol_conf.raw_trace,
 		    "Show raw trace event output (do not use print fmt or plugins)"),
+	OPT_BOOLEAN(0, "hierarchy", &symbol_conf.report_hierarchy,
+		    "Show entries in a hierarchy"),
 	OPT_END()
 	};
 	struct perf_data_file file = {
@@ -920,6 +922,21 @@ repeat:
 		symbol_conf.cumulate_callchain = false;
 	}
 
+	if (symbol_conf.report_hierarchy) {
+		/* disable incompatible options */
+		symbol_conf.event_group = false;
+		symbol_conf.cumulate_callchain = false;
+
+		if (field_order) {
+			pr_err("Error: --hierarchy and --fields options cannot be used together\n");
+			parse_options_usage(report_usage, options, "F", 1);
+			parse_options_usage(NULL, options, "hierarchy", 0);
+			goto error;
+		}
+
+		sort__need_collapse = true;
+	}
+
 	/* Force tty output for header output and per-thread stat. */
 	if (report.header || report.header_only || report.show_threads)
 		use_browser = 0;
-- 
2.7.1

[toc] | [prev] | [next] | [standalone]

#1335435 — [PATCH v6 24/25] perf hists: Support decaying in hierarchy mode

In the hierarchy mode, hist entries should decay their children too.
Also update hists__delete_entry() to be able to free child entries.

Acked-by: Pekka Enberg <penberg@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/hist.c | 42 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index f7b86a372b87..ab277b354c53 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -245,6 +245,8 @@ static void he_stat__decay(struct he_stat *he_stat)
 	/* XXX need decay for weight too? */
 }
 
+static void hists__delete_entry(struct hists *hists, struct hist_entry *he);
+
 static bool hists__decay_entry(struct hists *hists, struct hist_entry *he)
 {
 	u64 prev_period = he->stat.period;
@@ -260,21 +262,45 @@ static bool hists__decay_entry(struct hists *hists, struct hist_entry *he)
 
 	diff = prev_period - he->stat.period;
 
-	hists->stats.total_period -= diff;
-	if (!he->filtered)
-		hists->stats.total_non_filtered_period -= diff;
+	if (!he->depth) {
+		hists->stats.total_period -= diff;
+		if (!he->filtered)
+			hists->stats.total_non_filtered_period -= diff;
+	}
+
+	if (!he->leaf) {
+		struct hist_entry *child;
+		struct rb_node *node = rb_first(&he->hroot_out);
+		while (node) {
+			child = rb_entry(node, struct hist_entry, rb_node);
+			node = rb_next(node);
+
+			if (hists__decay_entry(hists, child))
+				hists__delete_entry(hists, child);
+		}
+	}
 
 	return he->stat.period == 0;
 }
 
 static void hists__delete_entry(struct hists *hists, struct hist_entry *he)
 {
-	rb_erase(&he->rb_node, &hists->entries);
+	struct rb_root *root_in;
+	struct rb_root *root_out;
 
-	if (sort__need_collapse)
-		rb_erase(&he->rb_node_in, &hists->entries_collapsed);
-	else
-		rb_erase(&he->rb_node_in, hists->entries_in);
+	if (he->parent_he) {
+		root_in  = &he->parent_he->hroot_in;
+		root_out = &he->parent_he->hroot_out;
+	} else {
+		if (sort__need_collapse)
+			root_in = &hists->entries_collapsed;
+		else
+			root_in = hists->entries_in;
+		root_out = &hists->entries;
+	}
+
+	rb_erase(&he->rb_node_in, root_in);
+	rb_erase(&he->rb_node, root_out);
 
 	--hists->nr_entries;
 	if (!he->filtered)
-- 
2.7.1

[toc] | [prev] | [next] | [standalone]

#1335436 — [PATCH v6 14/25] perf hists: Resort after filtering hierarchy

In hierarchy mode, a filter can affect periods of entries in upper
hierarchy.  So it needs to resort the hists after filter.

For example, let's look at following example:

 Overhead      Command / Shared Object / Symbol
 ------------  --------------------------------
 30.00%        perf
    20.00%        perf
       10.00%        main
        5.00%        pr_debug
	5.00%        memcpy
    10.00%        [kernel.vmlinux]
        8.00%        memset
	2.00%        cpu_idle

If we apply simbol filter for 'mem' it should look like this

 13.00%        perf
     8.00%        [kernel.vmlinux]
        8.00%        memset
     5.00%        perf
	5.00%        memcpy

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

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 585743853ca7..f7b86a372b87 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -1645,9 +1645,47 @@ static void hists__filter_by_type(struct hists *hists, int type, filter_fn_t fil
 	}
 }
 
+static void resort_filtered_entry(struct rb_root *root, struct hist_entry *he)
+{
+	struct rb_node **p = &root->rb_node;
+	struct rb_node *parent = NULL;
+	struct hist_entry *iter;
+	struct rb_root new_root = RB_ROOT;
+	struct rb_node *nd;
+
+	while (*p != NULL) {
+		parent = *p;
+		iter = rb_entry(parent, struct hist_entry, rb_node);
+
+		if (hist_entry__sort(he, iter) > 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);
+
+	if (he->leaf || he->filtered)
+		return;
+
+	nd = rb_first(&he->hroot_out);
+	while (nd) {
+		struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
+
+		nd = rb_next(nd);
+		rb_erase(&h->rb_node, &he->hroot_out);
+
+		resort_filtered_entry(&new_root, h);
+	}
+
+	he->hroot_out = new_root;
+}
+
 static void hists__filter_hierarchy(struct hists *hists, int type, const void *arg)
 {
 	struct rb_node *nd;
+	struct rb_root new_root = RB_ROOT;
 
 	hists->stats.nr_non_filtered_samples = 0;
 
@@ -1691,6 +1729,22 @@ static void hists__filter_hierarchy(struct hists *hists, int type, const void *a
 			nd = __rb_hierarchy_next(&h->rb_node, HMD_FORCE_SIBLING);
 		}
 	}
+
+	/*
+	 * resort output after applying a new filter since filter in a lower
+	 * hierarchy can change periods in a upper hierarchy.
+	 */
+	nd = rb_first(&hists->entries);
+	while (nd) {
+		struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
+
+		nd = rb_next(nd);
+		rb_erase(&h->rb_node, &hists->entries);
+
+		resort_filtered_entry(&new_root, h);
+	}
+
+	hists->entries = new_root;
 }
 
 void hists__filter_by_thread(struct hists *hists)
-- 
2.7.1

[toc] | [prev] | [next] | [standalone]

#1335437 — [PATCH v6 05/25] perf callchain: Check return value of split_add_child()

Now create_child() and add_child() return errors so check and pass it
to the caller.

Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/callchain.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index dab2c1f1e86b..5259379892e1 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -508,7 +508,7 @@ static enum match_result match_chain(struct callchain_cursor_node *node,
  * give a part of its callchain to the created child.
  * Then create another child to host the given callchain of new branch
  */
-static void
+static int
 split_add_child(struct callchain_node *parent,
 		struct callchain_cursor *cursor,
 		struct callchain_list *to_split,
@@ -520,6 +520,8 @@ split_add_child(struct callchain_node *parent,
 
 	/* split */
 	new = create_child(parent, true);
+	if (new == NULL)
+		return -1;
 
 	/* split the callchain and move a part to the new child */
 	old_tail = parent->val.prev;
@@ -554,7 +556,7 @@ split_add_child(struct callchain_node *parent,
 		node = callchain_cursor_current(cursor);
 		new = add_child(parent, cursor, period);
 		if (new == NULL)
-			return;
+			return -1;
 
 		/*
 		 * This is second child since we moved parent's children
@@ -576,6 +578,7 @@ split_add_child(struct callchain_node *parent,
 		parent->hit = period;
 		parent->count = 1;
 	}
+	return 0;
 }
 
 static enum match_result
@@ -670,7 +673,10 @@ append_chain(struct callchain_node *root,
 
 	/* we match only a part of the node. Split it and add the new chain */
 	if (matches < root->val_nr) {
-		split_add_child(root, cursor, cnode, start, matches, period);
+		if (split_add_child(root, cursor, cnode, start, matches,
+				    period) < 0)
+			return MATCH_ERROR;
+
 		return MATCH_EQ;
 	}
 
-- 
2.7.1

[toc] | [prev] | [next] | [standalone]

#1338725 — [tip:perf/core] perf callchain: Check return value of split_add_child()

Commit-ID:  f2bb4c5af4fe16d8b1e4ae371e1ceaa817380a88
Gitweb:     http://git.kernel.org/tip/f2bb4c5af4fe16d8b1e4ae371e1ceaa817380a88
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Tue, 16 Feb 2016 23:08:23 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 19 Feb 2016 19:14:36 -0300

perf callchain: Check return value of split_add_child()

Now create_child() and add_child() return errors so check and pass it
to the caller.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1455631723-17345-6-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/callchain.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index dab2c1f..5259379 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -508,7 +508,7 @@ static enum match_result match_chain(struct callchain_cursor_node *node,
  * give a part of its callchain to the created child.
  * Then create another child to host the given callchain of new branch
  */
-static void
+static int
 split_add_child(struct callchain_node *parent,
 		struct callchain_cursor *cursor,
 		struct callchain_list *to_split,
@@ -520,6 +520,8 @@ split_add_child(struct callchain_node *parent,
 
 	/* split */
 	new = create_child(parent, true);
+	if (new == NULL)
+		return -1;
 
 	/* split the callchain and move a part to the new child */
 	old_tail = parent->val.prev;
@@ -554,7 +556,7 @@ split_add_child(struct callchain_node *parent,
 		node = callchain_cursor_current(cursor);
 		new = add_child(parent, cursor, period);
 		if (new == NULL)
-			return;
+			return -1;
 
 		/*
 		 * This is second child since we moved parent's children
@@ -576,6 +578,7 @@ split_add_child(struct callchain_node *parent,
 		parent->hit = period;
 		parent->count = 1;
 	}
+	return 0;
 }
 
 static enum match_result
@@ -670,7 +673,10 @@ append_chain(struct callchain_node *root,
 
 	/* we match only a part of the node. Split it and add the new chain */
 	if (matches < root->val_nr) {
-		split_add_child(root, cursor, cnode, start, matches, period);
+		if (split_add_child(root, cursor, cnode, start, matches,
+				    period) < 0)
+			return MATCH_ERROR;
+
 		return MATCH_EQ;
 	}
 

[toc] | [prev] | [next] | [standalone]

#1335438 — [PATCH v6 13/25] perf hists: Support filtering in hierarchy mode

The hists__filter_hierarchy() function implements filtering in hierarchy
mode.  Now we have hist_entry__filter() so use it for entries in the
hierarchy.  It returns 3 kind of values.

A negative value means that it's not filtered by this type.  It marks
current entry as filtered tentatively so if a lower level entry removes
the filter it also removes the all parent so that we can find the entry
in the output.

Zero means it's filtered out by this type. A positive value means it's
not filtered so it removes the filter and shows in the output.  In these
cases, it moves to next entry since lower level entry won't match by
this type of filter anymore.  Thus all children will be filtered or not
together.

Acked-by: Pekka Enberg <penberg@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/hist.c | 101 +++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 93 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 075450154f9a..585743853ca7 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -1539,6 +1539,27 @@ static void hists__remove_entry_filter(struct hists *hists, struct hist_entry *h
 				       enum hist_filter filter)
 {
 	h->filtered &= ~(1 << filter);
+
+	if (symbol_conf.report_hierarchy) {
+		struct hist_entry *parent = h->parent_he;
+
+		while (parent) {
+			he_stat__add_stat(&parent->stat, &h->stat);
+
+			parent->filtered &= ~(1 << filter);
+
+			if (parent->filtered)
+				goto next;
+
+			/* force fold unfiltered entry for simplicity */
+			parent->unfolded = false;
+			parent->row_offset = 0;
+			parent->nr_rows = 0;
+next:
+			parent = parent->parent_he;
+		}
+	}
+
 	if (h->filtered)
 		return;
 
@@ -1624,28 +1645,92 @@ static void hists__filter_by_type(struct hists *hists, int type, filter_fn_t fil
 	}
 }
 
+static void hists__filter_hierarchy(struct hists *hists, int type, const void *arg)
+{
+	struct rb_node *nd;
+
+	hists->stats.nr_non_filtered_samples = 0;
+
+	hists__reset_filter_stats(hists);
+	hists__reset_col_len(hists);
+
+	nd = rb_first(&hists->entries);
+	while (nd) {
+		struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
+		int ret;
+
+		ret = hist_entry__filter(h, type, arg);
+
+		/*
+		 * case 1. non-matching type
+		 * zero out the period, set filter marker and move to child
+		 */
+		if (ret < 0) {
+			memset(&h->stat, 0, sizeof(h->stat));
+			h->filtered |= (1 << type);
+
+			nd = __rb_hierarchy_next(&h->rb_node, HMD_FORCE_CHILD);
+		}
+		/*
+		 * case 2. matched type (filter out)
+		 * set filter marker and move to next
+		 */
+		else if (ret == 1) {
+			h->filtered |= (1 << type);
+
+			nd = __rb_hierarchy_next(&h->rb_node, HMD_FORCE_SIBLING);
+		}
+		/*
+		 * case 3. ok (not filtered)
+		 * add period to hists and parents, erase the filter marker
+		 * and move to next sibling
+		 */
+		else {
+			hists__remove_entry_filter(hists, h, type);
+
+			nd = __rb_hierarchy_next(&h->rb_node, HMD_FORCE_SIBLING);
+		}
+	}
+}
+
 void hists__filter_by_thread(struct hists *hists)
 {
-	hists__filter_by_type(hists, HIST_FILTER__THREAD,
-			      hists__filter_entry_by_thread);
+	if (symbol_conf.report_hierarchy)
+		hists__filter_hierarchy(hists, HIST_FILTER__THREAD,
+					hists->thread_filter);
+	else
+		hists__filter_by_type(hists, HIST_FILTER__THREAD,
+				      hists__filter_entry_by_thread);
 }
 
 void hists__filter_by_dso(struct hists *hists)
 {
-	hists__filter_by_type(hists, HIST_FILTER__DSO,
-			      hists__filter_entry_by_dso);
+	if (symbol_conf.report_hierarchy)
+		hists__filter_hierarchy(hists, HIST_FILTER__DSO,
+					hists->dso_filter);
+	else
+		hists__filter_by_type(hists, HIST_FILTER__DSO,
+				      hists__filter_entry_by_dso);
 }
 
 void hists__filter_by_symbol(struct hists *hists)
 {
-	hists__filter_by_type(hists, HIST_FILTER__SYMBOL,
-			      hists__filter_entry_by_symbol);
+	if (symbol_conf.report_hierarchy)
+		hists__filter_hierarchy(hists, HIST_FILTER__SYMBOL,
+					hists->symbol_filter_str);
+	else
+		hists__filter_by_type(hists, HIST_FILTER__SYMBOL,
+				      hists__filter_entry_by_symbol);
 }
 
 void hists__filter_by_socket(struct hists *hists)
 {
-	hists__filter_by_type(hists, HIST_FILTER__SOCKET,
-			      hists__filter_entry_by_socket);
+	if (symbol_conf.report_hierarchy)
+		hists__filter_hierarchy(hists, HIST_FILTER__SOCKET,
+					&hists->socket_filter);
+	else
+		hists__filter_by_type(hists, HIST_FILTER__SOCKET,
+				      hists__filter_entry_by_socket);
 }
 
 void events_stats__inc(struct events_stats *stats, u32 type)
-- 
2.7.1

[toc] | [prev] | [next] | [standalone]

#1335440 — [PATCH v6 04/25] perf callchain: Add enum match_result for match_chain()

The append_chain() might return either result of match_chain() or
other (error) code.  But match_chain() can return any value in s64 type
so it's hard to check the error case.  Add new enum match_result and
make match_chain() return non-negative values only so that we can check
the error cases.

Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/callchain.c | 52 +++++++++++++++++++++++++++++----------------
 1 file changed, 34 insertions(+), 18 deletions(-)

diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index a82ea6f6fc0f..dab2c1f1e86b 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -475,16 +475,32 @@ add_child(struct callchain_node *parent,
 	return new;
 }
 
-static s64 match_chain(struct callchain_cursor_node *node,
-		      struct callchain_list *cnode)
+enum match_result {
+	MATCH_ERROR  = -1,
+	MATCH_EQ,
+	MATCH_LT,
+	MATCH_GT,
+};
+
+static enum match_result match_chain(struct callchain_cursor_node *node,
+				     struct callchain_list *cnode)
 {
 	struct symbol *sym = node->sym;
+	u64 left, right;
 
 	if (cnode->ms.sym && sym &&
-	    callchain_param.key == CCKEY_FUNCTION)
-		return cnode->ms.sym->start - sym->start;
-	else
-		return cnode->ip - node->ip;
+	    callchain_param.key == CCKEY_FUNCTION) {
+		left = cnode->ms.sym->start;
+		right = sym->start;
+	} else {
+		left = cnode->ip;
+		right = node->ip;
+	}
+
+	if (left == right)
+		return MATCH_EQ;
+
+	return left > right ? MATCH_GT : MATCH_LT;
 }
 
 /*
@@ -549,7 +565,7 @@ split_add_child(struct callchain_node *parent,
 		cnode = list_first_entry(&first->val, struct callchain_list,
 					 list);
 
-		if (match_chain(node, cnode) < 0)
+		if (match_chain(node, cnode) == MATCH_LT)
 			pp = &p->rb_left;
 		else
 			pp = &p->rb_right;
@@ -562,7 +578,7 @@ split_add_child(struct callchain_node *parent,
 	}
 }
 
-static int
+static enum match_result
 append_chain(struct callchain_node *root,
 	     struct callchain_cursor *cursor,
 	     u64 period);
@@ -583,17 +599,17 @@ append_chain_children(struct callchain_node *root,
 
 	/* lookup in childrens */
 	while (*p) {
-		s64 ret;
+		enum match_result ret;
 
 		parent = *p;
 		rnode = rb_entry(parent, struct callchain_node, rb_node_in);
 
 		/* If at least first entry matches, rely to children */
 		ret = append_chain(rnode, cursor, period);
-		if (ret == 0)
+		if (ret == MATCH_EQ)
 			goto inc_children_hit;
 
-		if (ret < 0)
+		if (ret == MATCH_LT)
 			p = &parent->rb_left;
 		else
 			p = &parent->rb_right;
@@ -611,7 +627,7 @@ inc_children_hit:
 	root->children_count++;
 }
 
-static int
+static enum match_result
 append_chain(struct callchain_node *root,
 	     struct callchain_cursor *cursor,
 	     u64 period)
@@ -620,7 +636,7 @@ append_chain(struct callchain_node *root,
 	u64 start = cursor->pos;
 	bool found = false;
 	u64 matches;
-	int cmp = 0;
+	enum match_result cmp = MATCH_ERROR;
 
 	/*
 	 * Lookup in the current node
@@ -636,7 +652,7 @@ append_chain(struct callchain_node *root,
 			break;
 
 		cmp = match_chain(node, cnode);
-		if (cmp)
+		if (cmp != MATCH_EQ)
 			break;
 
 		found = true;
@@ -646,7 +662,7 @@ append_chain(struct callchain_node *root,
 
 	/* matches not, relay no the parent */
 	if (!found) {
-		WARN_ONCE(!cmp, "Chain comparison error\n");
+		WARN_ONCE(cmp == MATCH_ERROR, "Chain comparison error\n");
 		return cmp;
 	}
 
@@ -655,20 +671,20 @@ append_chain(struct callchain_node *root,
 	/* we match only a part of the node. Split it and add the new chain */
 	if (matches < root->val_nr) {
 		split_add_child(root, cursor, cnode, start, matches, period);
-		return 0;
+		return MATCH_EQ;
 	}
 
 	/* we match 100% of the path, increment the hit */
 	if (matches == root->val_nr && cursor->pos == cursor->nr) {
 		root->hit += period;
 		root->count++;
-		return 0;
+		return MATCH_EQ;
 	}
 
 	/* We match the node and still have a part remaining */
 	append_chain_children(root, cursor, period);
 
-	return 0;
+	return MATCH_EQ;
 }
 
 int callchain_append(struct callchain_root *root,
-- 
2.7.1

[toc] | [prev] | [next] | [standalone]

#1338718 — [tip:perf/core] perf callchain: Add enum match_result for match_chain()

Commit-ID:  2d713b809d89a3d10c6a85162bf7cce0468e45d9
Gitweb:     http://git.kernel.org/tip/2d713b809d89a3d10c6a85162bf7cce0468e45d9
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Tue, 16 Feb 2016 23:08:22 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 19 Feb 2016 19:14:20 -0300

perf callchain: Add enum match_result for match_chain()

The append_chain() might return either result of match_chain() or other
(error) code.  But match_chain() can return any value in s64 type so
it's hard to check the error case.  Add new enum match_result and make
match_chain() return non-negative values only so that we can check the
error cases.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1455631723-17345-5-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/callchain.c | 52 +++++++++++++++++++++++++++++----------------
 1 file changed, 34 insertions(+), 18 deletions(-)

diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index a82ea6f..dab2c1f 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -475,16 +475,32 @@ add_child(struct callchain_node *parent,
 	return new;
 }
 
-static s64 match_chain(struct callchain_cursor_node *node,
-		      struct callchain_list *cnode)
+enum match_result {
+	MATCH_ERROR  = -1,
+	MATCH_EQ,
+	MATCH_LT,
+	MATCH_GT,
+};
+
+static enum match_result match_chain(struct callchain_cursor_node *node,
+				     struct callchain_list *cnode)
 {
 	struct symbol *sym = node->sym;
+	u64 left, right;
 
 	if (cnode->ms.sym && sym &&
-	    callchain_param.key == CCKEY_FUNCTION)
-		return cnode->ms.sym->start - sym->start;
-	else
-		return cnode->ip - node->ip;
+	    callchain_param.key == CCKEY_FUNCTION) {
+		left = cnode->ms.sym->start;
+		right = sym->start;
+	} else {
+		left = cnode->ip;
+		right = node->ip;
+	}
+
+	if (left == right)
+		return MATCH_EQ;
+
+	return left > right ? MATCH_GT : MATCH_LT;
 }
 
 /*
@@ -549,7 +565,7 @@ split_add_child(struct callchain_node *parent,
 		cnode = list_first_entry(&first->val, struct callchain_list,
 					 list);
 
-		if (match_chain(node, cnode) < 0)
+		if (match_chain(node, cnode) == MATCH_LT)
 			pp = &p->rb_left;
 		else
 			pp = &p->rb_right;
@@ -562,7 +578,7 @@ split_add_child(struct callchain_node *parent,
 	}
 }
 
-static int
+static enum match_result
 append_chain(struct callchain_node *root,
 	     struct callchain_cursor *cursor,
 	     u64 period);
@@ -583,17 +599,17 @@ append_chain_children(struct callchain_node *root,
 
 	/* lookup in childrens */
 	while (*p) {
-		s64 ret;
+		enum match_result ret;
 
 		parent = *p;
 		rnode = rb_entry(parent, struct callchain_node, rb_node_in);
 
 		/* If at least first entry matches, rely to children */
 		ret = append_chain(rnode, cursor, period);
-		if (ret == 0)
+		if (ret == MATCH_EQ)
 			goto inc_children_hit;
 
-		if (ret < 0)
+		if (ret == MATCH_LT)
 			p = &parent->rb_left;
 		else
 			p = &parent->rb_right;
@@ -611,7 +627,7 @@ inc_children_hit:
 	root->children_count++;
 }
 
-static int
+static enum match_result
 append_chain(struct callchain_node *root,
 	     struct callchain_cursor *cursor,
 	     u64 period)
@@ -620,7 +636,7 @@ append_chain(struct callchain_node *root,
 	u64 start = cursor->pos;
 	bool found = false;
 	u64 matches;
-	int cmp = 0;
+	enum match_result cmp = MATCH_ERROR;
 
 	/*
 	 * Lookup in the current node
@@ -636,7 +652,7 @@ append_chain(struct callchain_node *root,
 			break;
 
 		cmp = match_chain(node, cnode);
-		if (cmp)
+		if (cmp != MATCH_EQ)
 			break;
 
 		found = true;
@@ -646,7 +662,7 @@ append_chain(struct callchain_node *root,
 
 	/* matches not, relay no the parent */
 	if (!found) {
-		WARN_ONCE(!cmp, "Chain comparison error\n");
+		WARN_ONCE(cmp == MATCH_ERROR, "Chain comparison error\n");
 		return cmp;
 	}
 
@@ -655,20 +671,20 @@ append_chain(struct callchain_node *root,
 	/* we match only a part of the node. Split it and add the new chain */
 	if (matches < root->val_nr) {
 		split_add_child(root, cursor, cnode, start, matches, period);
-		return 0;
+		return MATCH_EQ;
 	}
 
 	/* we match 100% of the path, increment the hit */
 	if (matches == root->val_nr && cursor->pos == cursor->nr) {
 		root->hit += period;
 		root->count++;
-		return 0;
+		return MATCH_EQ;
 	}
 
 	/* We match the node and still have a part remaining */
 	append_chain_children(root, cursor, period);
 
-	return 0;
+	return MATCH_EQ;
 }
 
 int callchain_append(struct callchain_root *root,

[toc] | [prev] | [next] | [standalone]

#1335441 — [PATCH v6 21/25] perf hists browser: Align column header in hierarchy mode

Like in stdio, fit column header to hierarchy output.  Merge column
headers with "/" as a separator.

   Overhead        Command / Shared Object / Symbol
  ...
  +   0.09%        dwm
  +   0.06%        emacs
  -   0.05%        perf
     -   0.05%        [kernel.vmlinux]
        +   0.03%        [k] memcpy_orig
        +   0.01%        [k] unmap_single_vma
        +   0.01%        [k] smp_call_function_single
        +   0.00%        [k] native_irq_return_iret
        +   0.00%        [k] arch_trigger_all_cpu_backtrace_handler
        +   0.00%        [k] native_write_msr_safe

Acked-by: Pekka Enberg <penberg@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 69 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 68 insertions(+), 1 deletion(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index d209cf07bd12..e9dc5c2a40a5 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1430,11 +1430,78 @@ static int hists_browser__scnprintf_headers(struct hist_browser *browser, char *
 	return ret;
 }
 
+static int hists_browser__scnprintf_hierarchy_headers(struct hist_browser *browser, char *buf, size_t size)
+{
+	struct hists *hists = browser->hists;
+	struct perf_hpp dummy_hpp = {
+		.buf    = buf,
+		.size   = size,
+	};
+	struct perf_hpp_fmt *fmt;
+	size_t ret = 0;
+	int column = 0;
+	int nr_sort_keys = hists->hpp_list->nr_sort_keys;
+	bool first = true;
+
+	ret = scnprintf(buf, size, " ");
+	if (advance_hpp_check(&dummy_hpp, ret))
+		return ret;
+
+	hists__for_each_format(hists, fmt) {
+		if (column++ < browser->b.horiz_scroll)
+			continue;
+
+		if (perf_hpp__is_sort_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
+			break;
+
+		ret = fmt->header(fmt, &dummy_hpp, hists_to_evsel(hists));
+		if (advance_hpp_check(&dummy_hpp, ret))
+			break;
+
+		ret = scnprintf(dummy_hpp.buf, dummy_hpp.size, "  ");
+		if (advance_hpp_check(&dummy_hpp, ret))
+			break;
+	}
+
+	ret = scnprintf(dummy_hpp.buf, dummy_hpp.size, "%*s",
+			(nr_sort_keys - 1) * HIERARCHY_INDENT, "");
+	if (advance_hpp_check(&dummy_hpp, ret))
+		return ret;
+
+	hists__for_each_format(hists, fmt) {
+		if (!perf_hpp__is_sort_entry(fmt) && !perf_hpp__is_dynamic_entry(fmt))
+			continue;
+
+		if (first) {
+			first = false;
+		} else {
+			ret = scnprintf(dummy_hpp.buf, dummy_hpp.size, " / ");
+			if (advance_hpp_check(&dummy_hpp, ret))
+				break;
+		}
+
+		ret = fmt->header(fmt, &dummy_hpp, hists_to_evsel(hists));
+		dummy_hpp.buf[ret] = '\0';
+		rtrim(dummy_hpp.buf);
+
+		ret = strlen(dummy_hpp.buf);
+		if (advance_hpp_check(&dummy_hpp, ret))
+			break;
+	}
+
+	return ret;
+}
+
 static void hist_browser__show_headers(struct hist_browser *browser)
 {
 	char headers[1024];
 
-	hists_browser__scnprintf_headers(browser, headers, sizeof(headers));
+	if (symbol_conf.report_hierarchy)
+		hists_browser__scnprintf_hierarchy_headers(browser, headers,
+							   sizeof(headers));
+	else
+		hists_browser__scnprintf_headers(browser, headers,
+						 sizeof(headers));
 	ui_browser__gotorc(&browser->b, 0, 0);
 	ui_browser__set_color(&browser->b, HE_COLORSET_ROOT);
 	ui_browser__write_nstring(&browser->b, headers, browser->b.width + 1);
-- 
2.7.1

[toc] | [prev] | [next] | [standalone]

#1335442 — [PATCH v6 03/25] perf callchain: Check return value of fill_node()

Memory allocation in the fill_node() can fail so change its return type
to int and check it in add_child() too.

Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/callchain.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index 134d88b33fc1..a82ea6f6fc0f 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -416,7 +416,7 @@ create_child(struct callchain_node *parent, bool inherit_children)
 /*
  * Fill the node with callchain values
  */
-static void
+static int
 fill_node(struct callchain_node *node, struct callchain_cursor *cursor)
 {
 	struct callchain_cursor_node *cursor_node;
@@ -433,7 +433,7 @@ fill_node(struct callchain_node *node, struct callchain_cursor *cursor)
 		call = zalloc(sizeof(*call));
 		if (!call) {
 			perror("not enough memory for the code path tree");
-			return;
+			return -1;
 		}
 		call->ip = cursor_node->ip;
 		call->ms.sym = cursor_node->sym;
@@ -443,6 +443,7 @@ fill_node(struct callchain_node *node, struct callchain_cursor *cursor)
 		callchain_cursor_advance(cursor);
 		cursor_node = callchain_cursor_current(cursor);
 	}
+	return 0;
 }
 
 static struct callchain_node *
@@ -456,7 +457,16 @@ add_child(struct callchain_node *parent,
 	if (new == NULL)
 		return NULL;
 
-	fill_node(new, cursor);
+	if (fill_node(new, cursor) < 0) {
+		struct callchain_list *call, *tmp;
+
+		list_for_each_entry_safe(call, tmp, &new->val, list) {
+			list_del(&call->list);
+			free(call);
+		}
+		free(new);
+		return NULL;
+	}
 
 	new->children_hit = 0;
 	new->hit = period;
-- 
2.7.1

[toc] | [prev] | [next] | [standalone]

#1338721 — [tip:perf/core] perf callchain: Check return value of fill_node()

Commit-ID:  8451cbb9b174a9b6e016d7f1bff81ff12dbd1990
Gitweb:     http://git.kernel.org/tip/8451cbb9b174a9b6e016d7f1bff81ff12dbd1990
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Tue, 16 Feb 2016 23:08:21 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 19 Feb 2016 19:13:21 -0300

perf callchain: Check return value of fill_node()

Memory allocation in the fill_node() can fail so change its return type
to int and check it in add_child() too.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1455631723-17345-4-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/callchain.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index 134d88b..a82ea6f 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -416,7 +416,7 @@ create_child(struct callchain_node *parent, bool inherit_children)
 /*
  * Fill the node with callchain values
  */
-static void
+static int
 fill_node(struct callchain_node *node, struct callchain_cursor *cursor)
 {
 	struct callchain_cursor_node *cursor_node;
@@ -433,7 +433,7 @@ fill_node(struct callchain_node *node, struct callchain_cursor *cursor)
 		call = zalloc(sizeof(*call));
 		if (!call) {
 			perror("not enough memory for the code path tree");
-			return;
+			return -1;
 		}
 		call->ip = cursor_node->ip;
 		call->ms.sym = cursor_node->sym;
@@ -443,6 +443,7 @@ fill_node(struct callchain_node *node, struct callchain_cursor *cursor)
 		callchain_cursor_advance(cursor);
 		cursor_node = callchain_cursor_current(cursor);
 	}
+	return 0;
 }
 
 static struct callchain_node *
@@ -456,7 +457,16 @@ add_child(struct callchain_node *parent,
 	if (new == NULL)
 		return NULL;
 
-	fill_node(new, cursor);
+	if (fill_node(new, cursor) < 0) {
+		struct callchain_list *call, *tmp;
+
+		list_for_each_entry_safe(call, tmp, &new->val, list) {
+			list_del(&call->list);
+			free(call);
+		}
+		free(new);
+		return NULL;
+	}
 
 	new->children_hit = 0;
 	new->hit = period;

[toc] | [prev] | [next] | [standalone]

#1335443 — [PATCH v6 02/25] perf callchain: Check return value of add_child()

The create_child() in add_child() can return NULL in case of memory
allocation failure.  So check the return value and bail out.  The proper
error handling will be added later.

Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/callchain.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index 53c43eb9489e..134d88b33fc1 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -453,6 +453,9 @@ add_child(struct callchain_node *parent,
 	struct callchain_node *new;
 
 	new = create_child(parent, false);
+	if (new == NULL)
+		return NULL;
+
 	fill_node(new, cursor);
 
 	new->children_hit = 0;
@@ -524,6 +527,8 @@ split_add_child(struct callchain_node *parent,
 
 		node = callchain_cursor_current(cursor);
 		new = add_child(parent, cursor, period);
+		if (new == NULL)
+			return;
 
 		/*
 		 * This is second child since we moved parent's children
@@ -585,6 +590,9 @@ append_chain_children(struct callchain_node *root,
 	}
 	/* nothing in children, add to the current node */
 	rnode = add_child(root, cursor, period);
+	if (rnode == NULL)
+		return;
+
 	rb_link_node(&rnode->rb_node_in, parent, p);
 	rb_insert_color(&rnode->rb_node_in, &root->rb_root_in);
 
-- 
2.7.1

[toc] | [prev] | [next] | [standalone]

#1338719 — [tip:perf/core] perf callchain: Check return value of add_child()

Commit-ID:  7565bd39c1a63c82350d26a66ea1a1f1bb49ad2e
Gitweb:     http://git.kernel.org/tip/7565bd39c1a63c82350d26a66ea1a1f1bb49ad2e
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Tue, 16 Feb 2016 23:08:20 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 19 Feb 2016 19:12:52 -0300

perf callchain: Check return value of add_child()

The create_child() in add_child() can return NULL in case of memory
allocation failure.  So check the return value and bail out.  The proper
error handling will be added later.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1455631723-17345-3-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/callchain.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index 53c43eb..134d88b 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -453,6 +453,9 @@ add_child(struct callchain_node *parent,
 	struct callchain_node *new;
 
 	new = create_child(parent, false);
+	if (new == NULL)
+		return NULL;
+
 	fill_node(new, cursor);
 
 	new->children_hit = 0;
@@ -524,6 +527,8 @@ split_add_child(struct callchain_node *parent,
 
 		node = callchain_cursor_current(cursor);
 		new = add_child(parent, cursor, period);
+		if (new == NULL)
+			return;
 
 		/*
 		 * This is second child since we moved parent's children
@@ -585,6 +590,9 @@ append_chain_children(struct callchain_node *root,
 	}
 	/* nothing in children, add to the current node */
 	rnode = add_child(root, cursor, period);
+	if (rnode == NULL)
+		return;
+
 	rb_link_node(&rnode->rb_node_in, parent, p);
 	rb_insert_color(&rnode->rb_node_in, &root->rb_root_in);
 

[toc] | [prev] | [next] | [standalone]

#1335445 — [PATCH v6 25/25] perf top: Add --hierarchy option

Support hierarchy output for perf-top using --hierarchy option.

Acked-by: Pekka Enberg <penberg@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Documentation/perf-top.txt |  3 +++
 tools/perf/builtin-top.c              | 15 +++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/tools/perf/Documentation/perf-top.txt b/tools/perf/Documentation/perf-top.txt
index b0e60e17db38..19f046f027cd 100644
--- a/tools/perf/Documentation/perf-top.txt
+++ b/tools/perf/Documentation/perf-top.txt
@@ -233,6 +233,9 @@ Default is to monitor all CPUS.
 --raw-trace::
 	When displaying traceevent output, do not use print fmt or plugins.
 
+--hierarchy::
+	Enable hierarchy output.
+
 INTERACTIVE PROMPTING KEYS
 --------------------------
 
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index a75de3940b97..b86b623e8799 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1214,6 +1214,8 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
 		     parse_branch_stack),
 	OPT_BOOLEAN(0, "raw-trace", &symbol_conf.raw_trace,
 		    "Show raw trace event output (do not use print fmt or plugins)"),
+	OPT_BOOLEAN(0, "hierarchy", &symbol_conf.report_hierarchy,
+		    "Show entries in a hierarchy"),
 	OPT_END()
 	};
 	const char * const top_usage[] = {
@@ -1241,6 +1243,19 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
 		goto out_delete_evlist;
 	}
 
+	if (symbol_conf.report_hierarchy) {
+		/* disable incompatible options */
+		symbol_conf.event_group = false;
+		symbol_conf.cumulate_callchain = false;
+
+		if (field_order) {
+			pr_err("Error: --hierarchy and --fields options cannot be used together\n");
+			parse_options_usage(top_usage, options, "fields", 0);
+			parse_options_usage(NULL, options, "hierarchy", 0);
+			goto out_delete_evlist;
+		}
+	}
+
 	sort__mode = SORT_MODE__TOP;
 	/* display thread wants entries to be collapsed in a different tree */
 	sort__need_collapse = 1;
-- 
2.7.1

[toc] | [prev] | [next] | [standalone]

#1338816 — Re: [PATCH v6 25/25] perf top: Add --hierarchy option

On Tue, Feb 16, 2016 at 11:08:43PM +0900, Namhyung Kim wrote:
> Support hierarchy output for perf-top using --hierarchy option.
> 
> Acked-by: Pekka Enberg <penberg@kernel.org>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/Documentation/perf-top.txt |  3 +++
>  tools/perf/builtin-top.c              | 15 +++++++++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/tools/perf/Documentation/perf-top.txt b/tools/perf/Documentation/perf-top.txt
> index b0e60e17db38..19f046f027cd 100644
> --- a/tools/perf/Documentation/perf-top.txt
> +++ b/tools/perf/Documentation/perf-top.txt
> @@ -233,6 +233,9 @@ Default is to monitor all CPUS.
>  --raw-trace::
>  	When displaying traceevent output, do not use print fmt or plugins.
>  
> +--hierarchy::
> +	Enable hierarchy output.
> +
>  INTERACTIVE PROMPTING KEYS
>  --------------------------
>  
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index a75de3940b97..b86b623e8799 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -1214,6 +1214,8 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
>  		     parse_branch_stack),
>  	OPT_BOOLEAN(0, "raw-trace", &symbol_conf.raw_trace,
>  		    "Show raw trace event output (do not use print fmt or plugins)"),
> +	OPT_BOOLEAN(0, "hierarchy", &symbol_conf.report_hierarchy,
> +		    "Show entries in a hierarchy"),

hum, I'm not getting --children (default) output with hierarchy?

I can't see this was intentional.. haven't found the reason yet

thanks,
jirka

[toc] | [prev] | [next] | [standalone]

#1338927 — Re: [PATCH v6 25/25] perf top: Add --hierarchy option

On Sun, Feb 21, 2016 at 12:19:29AM +0100, Jiri Olsa wrote:
> On Tue, Feb 16, 2016 at 11:08:43PM +0900, Namhyung Kim wrote:
> > Support hierarchy output for perf-top using --hierarchy option.
> > 
> > Acked-by: Pekka Enberg <penberg@kernel.org>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/Documentation/perf-top.txt |  3 +++
> >  tools/perf/builtin-top.c              | 15 +++++++++++++++
> >  2 files changed, 18 insertions(+)
> > 
> > diff --git a/tools/perf/Documentation/perf-top.txt b/tools/perf/Documentation/perf-top.txt
> > index b0e60e17db38..19f046f027cd 100644
> > --- a/tools/perf/Documentation/perf-top.txt
> > +++ b/tools/perf/Documentation/perf-top.txt
> > @@ -233,6 +233,9 @@ Default is to monitor all CPUS.
> >  --raw-trace::
> >  	When displaying traceevent output, do not use print fmt or plugins.
> >  
> > +--hierarchy::
> > +	Enable hierarchy output.
> > +
> >  INTERACTIVE PROMPTING KEYS
> >  --------------------------
> >  
> > diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> > index a75de3940b97..b86b623e8799 100644
> > --- a/tools/perf/builtin-top.c
> > +++ b/tools/perf/builtin-top.c
> > @@ -1214,6 +1214,8 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
> >  		     parse_branch_stack),
> >  	OPT_BOOLEAN(0, "raw-trace", &symbol_conf.raw_trace,
> >  		    "Show raw trace event output (do not use print fmt or plugins)"),
> > +	OPT_BOOLEAN(0, "hierarchy", &symbol_conf.report_hierarchy,
> > +		    "Show entries in a hierarchy"),
> 
> hum, I'm not getting --children (default) output with hierarchy?
> 
> I can't see this was intentional.. haven't found the reason yet

Currently the hierarchy mode disables the children mode.  This is for
simplicity and I didn't test the combination thoroughly although it
would work.  Maybe we can remove the restriction later, but I think
it'd be better to start with a simple choice.

Thanks,
Namhyung

[toc] | [prev] | [next] | [standalone]

#1335447 — [PATCH v6 16/25] perf ui/stdio: Implement hierarchy output mode

The hierarchy output mode is to group entries for each level so that
user can see higher level picture more easily.  It also helps to find
out which component is most costly.  The output will look like below:

      15.11%     swapper
         14.97%     [kernel.vmlinux]
          0.09%     [libahci]
          0.05%     [iwlwifi]
      10.29%     irq/33-iwlwifi
          6.45%     [kernel.vmlinux]
          1.41%     [mac80211]
          1.15%     [iwldvm]
          1.14%     [iwlwifi]
          0.14%     [cfg80211]
       4.81%     firefox
          3.92%     libxul.so
          0.34%     [kernel.vmlinux]

Acked-by: Pekka Enberg <penberg@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/stdio/hist.c | 74 +++++++++++++++++++++++++++++++++++++++++++++-
 tools/perf/util/hist.h     |  2 ++
 2 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 87b022ff03d8..0d5640e0df63 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -410,6 +410,71 @@ static int hist_entry__snprintf(struct hist_entry *he, struct perf_hpp *hpp)
 	return hpp->buf - start;
 }
 
+static int hist_entry__hierarchy_fprintf(struct hist_entry *he,
+					 struct perf_hpp *hpp,
+					 int nr_sort_key, struct hists *hists,
+					 FILE *fp)
+{
+	const char *sep = symbol_conf.field_sep;
+	struct perf_hpp_fmt *fmt;
+	char *buf = hpp->buf;
+	int ret, printed = 0;
+	bool first = true;
+
+	if (symbol_conf.exclude_other && !he->parent)
+		return 0;
+
+	ret = scnprintf(hpp->buf, hpp->size, "%*s", he->depth * HIERARCHY_INDENT, "");
+	advance_hpp(hpp, ret);
+
+	hists__for_each_format(he->hists, fmt) {
+		if (perf_hpp__is_sort_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
+			break;
+
+		/*
+		 * If there's no field_sep, we still need
+		 * to display initial '  '.
+		 */
+		if (!sep || !first) {
+			ret = scnprintf(hpp->buf, hpp->size, "%s", sep ?: "  ");
+			advance_hpp(hpp, ret);
+		} else
+			first = false;
+
+		if (perf_hpp__use_color() && fmt->color)
+			ret = fmt->color(fmt, hpp, he);
+		else
+			ret = fmt->entry(fmt, hpp, he);
+
+		advance_hpp(hpp, ret);
+	}
+
+	if (sep)
+		ret = scnprintf(hpp->buf, hpp->size, "%s", sep);
+	else
+		ret = scnprintf(hpp->buf, hpp->size, "%*s",
+				(nr_sort_key - 1) * HIERARCHY_INDENT + 2, "");
+	advance_hpp(hpp, ret);
+
+	fmt = he->fmt;
+	if (perf_hpp__use_color() && fmt->color)
+		fmt->color(fmt, hpp, he);
+	else
+		fmt->entry(fmt, hpp, he);
+
+	printed += fprintf(fp, "%s\n", buf);
+
+	if (symbol_conf.use_callchain && he->leaf) {
+		u64 total = hists__total_period(hists);
+
+		printed += hist_entry_callchain__fprintf(he, total, 0, fp);
+		goto out;
+	}
+
+out:
+	return printed;
+}
+
 static int hist_entry__fprintf(struct hist_entry *he, size_t size,
 			       struct hists *hists,
 			       char *bf, size_t bfsz, FILE *fp)
@@ -424,6 +489,13 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
 	if (size == 0 || size > bfsz)
 		size = hpp.size = bfsz;
 
+	if (symbol_conf.report_hierarchy) {
+		int nr_sort = hists->hpp_list->nr_sort_keys;
+
+		return hist_entry__hierarchy_fprintf(he, &hpp, nr_sort,
+						     hists, fp);
+	}
+
 	hist_entry__snprintf(he, &hpp);
 
 	ret = fprintf(fp, "%s\n", bf);
@@ -522,7 +594,7 @@ print_entries:
 		goto out;
 	}
 
-	for (nd = rb_first(&hists->entries); nd; nd = rb_next(nd)) {
+	for (nd = rb_first(&hists->entries); nd; nd = __rb_hierarchy_next(nd, HMD_FORCE_CHILD)) {
 		struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
 		float percent;
 
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 5fc30dc54b5a..a736a94f369e 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -434,4 +434,6 @@ static inline struct rb_node *rb_hierarchy_next(struct rb_node *node)
 	return __rb_hierarchy_next(node, HMD_NORMAL);
 }
 
+#define HIERARCHY_INDENT  3
+
 #endif	/* __PERF_HIST_H */
-- 
2.7.1

[toc] | [prev] | [next] | [standalone]

#1338820 — Re: [PATCH v6 16/25] perf ui/stdio: Implement hierarchy output mode

On Tue, Feb 16, 2016 at 11:08:34PM +0900, Namhyung Kim wrote:

SNIP

> +static int hist_entry__hierarchy_fprintf(struct hist_entry *he,
> +					 struct perf_hpp *hpp,
> +					 int nr_sort_key, struct hists *hists,
> +					 FILE *fp)
> +{
> +	const char *sep = symbol_conf.field_sep;
> +	struct perf_hpp_fmt *fmt;
> +	char *buf = hpp->buf;
> +	int ret, printed = 0;
> +	bool first = true;
> +
> +	if (symbol_conf.exclude_other && !he->parent)
> +		return 0;
> +
> +	ret = scnprintf(hpp->buf, hpp->size, "%*s", he->depth * HIERARCHY_INDENT, "");
> +	advance_hpp(hpp, ret);
> +
> +	hists__for_each_format(he->hists, fmt) {
> +		if (perf_hpp__is_sort_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
> +			break;
> +
> +		/*
> +		 * If there's no field_sep, we still need
> +		 * to display initial '  '.
> +		 */
> +		if (!sep || !first) {
> +			ret = scnprintf(hpp->buf, hpp->size, "%s", sep ?: "  ");
> +			advance_hpp(hpp, ret);
> +		} else
> +			first = false;
> +
> +		if (perf_hpp__use_color() && fmt->color)
> +			ret = fmt->color(fmt, hpp, he);
> +		else
> +			ret = fmt->entry(fmt, hpp, he);
> +
> +		advance_hpp(hpp, ret);

there's new hist_entry__snprintf_alignment function for
proper alignment now used in hist_entry__hierarchy_fprintf

you might wat to use it in here as well

jirka

[toc] | [prev] | [next] | [standalone]

#1338919 — Re: [PATCH v6 16/25] perf ui/stdio: Implement hierarchy output mode

On Sun, Feb 21, 2016 at 12:18:56AM +0100, Jiri Olsa wrote:
> On Tue, Feb 16, 2016 at 11:08:34PM +0900, Namhyung Kim wrote:
> 
> SNIP
> 
> > +static int hist_entry__hierarchy_fprintf(struct hist_entry *he,
> > +					 struct perf_hpp *hpp,
> > +					 int nr_sort_key, struct hists *hists,
> > +					 FILE *fp)
> > +{
> > +	const char *sep = symbol_conf.field_sep;
> > +	struct perf_hpp_fmt *fmt;
> > +	char *buf = hpp->buf;
> > +	int ret, printed = 0;
> > +	bool first = true;
> > +
> > +	if (symbol_conf.exclude_other && !he->parent)
> > +		return 0;
> > +
> > +	ret = scnprintf(hpp->buf, hpp->size, "%*s", he->depth * HIERARCHY_INDENT, "");
> > +	advance_hpp(hpp, ret);
> > +
> > +	hists__for_each_format(he->hists, fmt) {
> > +		if (perf_hpp__is_sort_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
> > +			break;
> > +
> > +		/*
> > +		 * If there's no field_sep, we still need
> > +		 * to display initial '  '.
> > +		 */
> > +		if (!sep || !first) {
> > +			ret = scnprintf(hpp->buf, hpp->size, "%s", sep ?: "  ");
> > +			advance_hpp(hpp, ret);
> > +		} else
> > +			first = false;
> > +
> > +		if (perf_hpp__use_color() && fmt->color)
> > +			ret = fmt->color(fmt, hpp, he);
> > +		else
> > +			ret = fmt->entry(fmt, hpp, he);
> > +
> > +		advance_hpp(hpp, ret);
> 
> there's new hist_entry__snprintf_alignment function for
> proper alignment now used in hist_entry__hierarchy_fprintf
> 
> you might wat to use it in here as well

Ah missed that, will change.

Btw it seems it's not strictly needed here since the hierarchy mode
makes each column as the last though.  But I agree that it's a general
change and can be applied here as well.

Thanks,
Namhyung

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web