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: Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Ingo Molnar <mingo@kernel.org>, Paul Mackerras <paulus@samba.org>,
	Namhyung Kim <namhyung.kim@lge.com>,
	Namhyung Kim <namhyung@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>, Jiri Olsa <jolsa@redhat.com>,
	David Ahern <dsahern@gmail.com>, Andi Kleen <andi@firstfloor.org>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Arun Sharma <asharma@fb.com>,
	Rodrigo Campos <rodrigo@sdfg.com.ar>
Subject: [PATCH 3/3] perf callchain: Prune misleading callchains for self entries
Date: Thu, 14 Aug 2014 15:01:40 +0900	[thread overview]
Message-ID: <1407996100-6359-4-git-send-email-namhyung@kernel.org> (raw)
In-Reply-To: <1407996100-6359-1-git-send-email-namhyung@kernel.org>

The "perf report -g" displays callchains callee-first order.  That
means it can see the callers of the sample in a reverse order.  For
example, "intel_idle" entry shows following callchain on my data.

  Children      Self  Command          Shared Object       Symbol
  --------  --------  ---------------  ------------------  ----------------
    40.53%    40.53%  swapper          [kernel.kallsyms]   [k] intel_idle
              |
               --- intel_idle
                   cpuidle_enter
                   cpuidle_wrap_enter
                   cpuidle_enter_tk
                   cpuidle_idle_call
                   cpu_idle
                  |
                  |--85.25%-- start_secondary
                  |
                   --14.75%-- rest_init
                             start_kernel
                             x86_64_start_reservations
                             x86_64_start_kernel

So "intel_idle" was called by "cpuidle_enter", and in turn, it's
called by "cpuidle_wrap_enter" and so on.

When with -g "caller", it shows callchains in a reversed order -
"caller-first".  I know it's sometimes useful but never used it.

    40.53%    40.53%  swapper          [kernel.kallsyms]   [k] intel_idle
                    |
                    |--85.25%-- start_secondary
                    |          cpu_idle
                    |          cpuidle_idle_call
                    |          cpuidle_enter_tk
                    |          cpuidle_wrap_enter
                    |          cpuidle_enter
                    |          intel_idle
                    |
                     --14.75%-- x86_64_start_kernel
                               x86_64_start_reservations
                               start_kernel
                               rest_init
                               cpu_idle
                               cpuidle_idle_call
                               cpuidle_enter_tk
                               cpuidle_wrap_enter
                               cpuidle_enter
                               intel_idle

However, with --children feature added, it now can show all callees of
the entry.  For example, "start_kernel" entry now can display it calls
rest_init and in turn cpu_idle and then cpuidle_idle_call (95.72%).

     6.14%     0.00%  swapper          [kernel.kallsyms]   [k] start_kernel
                   |
                    --- start_kernel
                        rest_init
                        cpu_idle
                       |
                       |--97.52%-- cpuidle_idle_call
                       |          cpuidle_enter_tk
                       |          |
                       |          |--99.91%-- cpuidle_wrap_enter
                       |          |          cpuidle_enter
                       |          |          intel_idle
                       |           --0.09%-- [...]
                        --2.48%-- [...]

Note that start_kernel has no self overhead - meaning that it never
get sampled by itself but constructs such a nice callgraph.  But,
sadly, if an entry has self overhead, callchain will get confused with
generated callchain (like above) and self callchains (which reversed
order) like the eariler example.

To be consistent with other entries, I'd like to make it just to show
a single entry - itself - like below since it doesn't have callees
(children) at all.  But still use the whole callchain to construct
children entries (like the start_kernel) as usual.

    40.53%    40.53%  swapper          [kernel.kallsyms]   [k] intel_idle
                    |
                    --- intel_idle

Note that this is just for --children is enabled.  Without that, it
will do the original behavior.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/hist.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 30df6187ee02..67f249daf34e 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -716,7 +716,28 @@ 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);
+	if (callchain_param.order == ORDER_CALLER) {
+		struct callchain_cursor cursor;
+		int nr = callchain_cursor.nr;
+
+		/*
+		 * When --children with -g caller, it just adds noise to
+		 * self entries.  Just adding last node (self) is enough
+		 * and it'd privide a consistent view with other (cumulative)
+		 * entries.
+		 */
+		while (--nr) {
+			callchain_cursor_advance(&callchain_cursor);
+		}
+
+		BUG_ON(callchain_cursor.nr != callchain_cursor.pos + 1);
+
+		callchain_cursor_snapshot(&cursor, &callchain_cursor);
+
+		callchain_append(he->callchain, &cursor, sample->period);
+	} else {
+		callchain_append(he->callchain, &callchain_cursor, sample->period);
+	}
 
 	/*
 	 * We need to re-initialize the cursor since callchain_append()
-- 
2.0.0


  parent reply	other threads:[~2014-08-14  6:02 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-14  6:01 [RFC/PATCHSET 0/3] perf tools: Callchain improvement with --children and -g caller Namhyung Kim
2014-08-14  6:01 ` [PATCH 1/3] perf report: Relax -g option parsing not to limit the option order Namhyung Kim
2014-08-18  8:22   ` [tip:perf/core] " tip-bot for Namhyung Kim
2014-08-14  6:01 ` [PATCH 2/3] perf tools: Put callers above callee when callchain order is caller Namhyung Kim
2014-08-14  6:01 ` Namhyung Kim [this message]
2014-08-14 14:10   ` [PATCH 3/3] perf callchain: Prune misleading callchains for self entries Jiri Olsa
2014-08-15  1:57     ` Namhyung Kim
2014-08-15 19:51       ` Jiri Olsa
2014-08-16  2:26         ` Namhyung Kim
2014-08-18 11:31           ` Jiri Olsa
2014-08-19  5:51             ` Namhyung Kim
2014-08-19  7:10               ` Jiri Olsa
2014-08-19  8:20                 ` Namhyung Kim
2014-08-15 13:52 ` [RFC/PATCHSET 0/3] perf tools: Callchain improvement with --children and -g caller Arnaldo Carvalho de Melo

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=1407996100-6359-4-git-send-email-namhyung@kernel.org \
    --to=namhyung@kernel.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@kernel.org \
    --cc=andi@firstfloor.org \
    --cc=asharma@fb.com \
    --cc=dsahern@gmail.com \
    --cc=fweisbec@gmail.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung.kim@lge.com \
    --cc=paulus@samba.org \
    --cc=rodrigo@sdfg.com.ar \
    /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

Powered by JetHome