mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Jiri Olsa <jolsa@kernel.org>, LKML <linux-kernel@vger.kernel.org>,
	David Ahern <dsahern@gmail.com>, Andi Kleen <andi@firstfloor.org>,
	Stephane Eranian <eranian@google.com>,
	Wang Nan <wangnan0@huawei.com>
Subject: [PATCH v2 7/7] perf tools: Recalc total periods using top-level entries in hierarchy
Date: Wed,  9 Mar 2016 22:47:02 +0900	[thread overview]
Message-ID: <1457531222-18130-8-git-send-email-namhyung@kernel.org> (raw)
In-Reply-To: <1457531222-18130-1-git-send-email-namhyung@kernel.org>

When hierarchy mode is enabled, each entries in a same hierarchy shares
period.  IOW an upper level entry's period is a sum of lower level
entries.  Thus perf uses only one of them to calculate the total period
of hists.  It was lowest-level (leaf) entries but it has a problem when
it comes to filters.

If a filter is applied, entries in the same level will be filtered or
not.  But upper level entries still have period of their sum including
filtered one.  So total sum of upper level entries will not be same as
sum of lower level entries.

This resulted in entries having more than 100% of overhead and it can be
produced using perf top with filter(s).

Reported-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/hist.c | 44 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index a98f9345f686..290b3cbf6877 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -1453,6 +1453,31 @@ void hists__inc_stats(struct hists *hists, struct hist_entry *h)
 	hists->stats.total_period += h->stat.period;
 }
 
+static void hierarchy_recalc_total_periods(struct hists *hists)
+{
+	struct rb_node *node;
+	struct hist_entry *he;
+
+	node = rb_first(&hists->entries);
+
+	hists->stats.total_period = 0;
+	hists->stats.total_non_filtered_period = 0;
+
+	/*
+	 * recalculate total period using top-level entries only
+	 * since lower level entries only see non-filtered entries
+	 * but upper level entries have sum of both entries.
+	 */
+	while (node) {
+		he = rb_entry(node, struct hist_entry, rb_node);
+		node = rb_next(node);
+
+		hists->stats.total_period += he->stat.period;
+		if (!he->filtered)
+			hists->stats.total_non_filtered_period += he->stat.period;
+	}
+}
+
 static void hierarchy_insert_output_entry(struct rb_root *root,
 					  struct hist_entry *he)
 {
@@ -1518,11 +1543,6 @@ static void hists__hierarchy_output_resort(struct hists *hists,
 			continue;
 		}
 
-		/* only update stat for leaf entries to avoid duplication */
-		hists__inc_stats(hists, he);
-		if (!he->filtered)
-			hists__calc_col_len(hists, he);
-
 		if (!use_callchain)
 			continue;
 
@@ -1602,11 +1622,13 @@ static void output_resort(struct hists *hists, struct ui_progress *prog,
 	hists__reset_col_len(hists);
 
 	if (symbol_conf.report_hierarchy) {
-		return hists__hierarchy_output_resort(hists, prog,
-						      &hists->entries_collapsed,
-						      &hists->entries,
-						      min_callchain_hits,
-						      use_callchain);
+		hists__hierarchy_output_resort(hists, prog,
+					       &hists->entries_collapsed,
+					       &hists->entries,
+					       min_callchain_hits,
+					       use_callchain);
+		hierarchy_recalc_total_periods(hists);
+		return;
 	}
 
 	if (sort__need_collapse)
@@ -1927,6 +1949,8 @@ static void hists__filter_hierarchy(struct hists *hists, int type, const void *a
 		}
 	}
 
+	hierarchy_recalc_total_periods(hists);
+
 	/*
 	 * resort output after applying a new filter since filter in a lower
 	 * hierarchy can change periods in a upper hierarchy.
-- 
2.7.2

  parent reply	other threads:[~2016-03-09 13:49 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-09 13:46 [PATCHSET 0/7] perf tools: Fix and cleanups for hierarchy mode (v2) Namhyung Kim
2016-03-09 13:46 ` [PATCH v2 1/7] perf tools: Fix hist_entry__filter() for hierarchy Namhyung Kim
2016-03-11  8:48   ` [tip:perf/core] " tip-bot for Namhyung Kim
2016-03-09 13:46 ` [PATCH v2 2/7] perf tools: Add more sort entry check functions Namhyung Kim
2016-03-11  8:48   ` [tip:perf/core] " tip-bot for Namhyung Kim
2016-03-09 13:46 ` [PATCH v2 3/7] perf tools: Fix command line filters in hierarchy mode Namhyung Kim
2016-03-11  8:48   ` [tip:perf/core] " tip-bot for Namhyung Kim
2016-03-09 13:46 ` [PATCH v2 4/7] perf tools: Remove hist_entry->fmt field Namhyung Kim
2016-03-11  8:49   ` [tip:perf/core] " tip-bot for Namhyung Kim
2016-03-09 13:47 ` [PATCH v2 5/7] perf hists browser: Cleanup hist_browser__fprintf_hierarchy_entry() Namhyung Kim
2016-03-11  8:49   ` [tip:perf/core] " tip-bot for Namhyung Kim
2016-03-09 13:47 ` [PATCH v2 6/7] perf tools: Remove nr_sort_keys field Namhyung Kim
2016-03-11  8:50   ` [tip:perf/core] " tip-bot for Namhyung Kim
2016-03-09 13:47 ` Namhyung Kim [this message]
2016-03-11  8:50   ` [tip:perf/core] perf tools: Recalc total periods using top-level entries in hierarchy tip-bot for Namhyung Kim
2016-03-10 10:33 ` [PATCHSET 0/7] perf tools: Fix and cleanups for hierarchy mode (v2) Jiri Olsa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1457531222-18130-8-git-send-email-namhyung@kernel.org \
    --to=namhyung@kernel.org \
    --cc=acme@kernel.org \
    --cc=andi@firstfloor.org \
    --cc=dsahern@gmail.com \
    --cc=eranian@google.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=wangnan0@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®