All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arun Sharma <asharma@fb.com>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@elte.hu>,
	Arnaldo Carvalho de Melo <acme@redhat.com>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Mike Galbraith <efault@gmx.de>, Paul Mackerras <paulus@samba.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Stephane Eranian <eranian@google.com>,
	Namhyung Kim <namhyung.kim@lge.com>,
	Tom Zanussi <tzanussi@gmail.com>,
	linux-perf-users@vger.kernel.org
Subject: Re: [PATCH] perf: Add a new sort order: SORT_INCLUSIVE (v6)
Date: Wed, 8 Aug 2012 12:16:30 -0700	[thread overview]
Message-ID: <5022BB0E.8050706@fb.com> (raw)
In-Reply-To: <4F769976.3070203@fb.com>

On 3/30/12 10:43 PM, Arun Sharma wrote:
> [ Meant to include v6 ChangeLog as well. Technical difficulties.. ]
>
> v6 ChangeLog:
>
> rebased to tip:perf/core and fixed a minor problem in computing
> the total period in hists__remove_entry_filter(). Needed to
> use period_self instead of period.

This patch breaks perf top (symptom: percentages > 100%). Fixed by the 
following patch.

Namhyung: if you're still working on forward porting this, please add 
this fix to your queue.

  -Arun

commit 75a1c409a529c9741f8a2f493868d1fc7ce7e06d
Author: Arun Sharma <asharma@fb.com>
Date:   Wed Aug 8 11:47:02 2012 -0700

    perf: update period_self as well on collapsing
       When running perf top, we have a series of incoming samples,
    which get aggregated in various user specified ways.
       Suppose function "foo" had the following samples:
    101, 103, 99, 105, ...
       ->period for the corresponding entry looks as follows:
    101, 204, 303, 408, ...
       However, due to this bug, ->period_self contains:
    101, 103, 99, 105, ...
       and therefore breaks the invariant period == period_self
    in the default mode (no sort inclusive).
       Since total_period is computed by summing up period_self,
       period/total_period can be > 100%
       Fix the bug by updating period_self as well.
       Signed-off-by: Arun Sharma <asharma@fb.com>

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index a2a8d91..adc891e 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -462,6 +462,7 @@ static bool hists__collapse_insert_entry(struct 
hists *hists,

		if (!cmp) {
			iter->period += he->period;
+			iter->period_self += he->period_self;
			iter->nr_events += he->nr_events;
			if (symbol_conf.use_callchain) {
				callchain_cursor_reset(&hists->callchain_cursor);

WARNING: multiple messages have this Message-ID (diff)
From: Arun Sharma <asharma@fb.com>
To: <linux-kernel@vger.kernel.org>
Cc: Ingo Molnar <mingo@elte.hu>,
	Arnaldo Carvalho de Melo <acme@redhat.com>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Mike Galbraith <efault@gmx.de>, Paul Mackerras <paulus@samba.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Stephane Eranian <eranian@google.com>,
	Namhyung Kim <namhyung.kim@lge.com>,
	Tom Zanussi <tzanussi@gmail.com>,
	<linux-perf-users@vger.kernel.org>
Subject: Re: [PATCH] perf: Add a new sort order: SORT_INCLUSIVE (v6)
Date: Wed, 8 Aug 2012 12:16:30 -0700	[thread overview]
Message-ID: <5022BB0E.8050706@fb.com> (raw)
In-Reply-To: <4F769976.3070203@fb.com>

On 3/30/12 10:43 PM, Arun Sharma wrote:
> [ Meant to include v6 ChangeLog as well. Technical difficulties.. ]
>
> v6 ChangeLog:
>
> rebased to tip:perf/core and fixed a minor problem in computing
> the total period in hists__remove_entry_filter(). Needed to
> use period_self instead of period.

This patch breaks perf top (symptom: percentages > 100%). Fixed by the 
following patch.

Namhyung: if you're still working on forward porting this, please add 
this fix to your queue.

  -Arun

commit 75a1c409a529c9741f8a2f493868d1fc7ce7e06d
Author: Arun Sharma <asharma@fb.com>
Date:   Wed Aug 8 11:47:02 2012 -0700

    perf: update period_self as well on collapsing
       When running perf top, we have a series of incoming samples,
    which get aggregated in various user specified ways.
       Suppose function "foo" had the following samples:
    101, 103, 99, 105, ...
       ->period for the corresponding entry looks as follows:
    101, 204, 303, 408, ...
       However, due to this bug, ->period_self contains:
    101, 103, 99, 105, ...
       and therefore breaks the invariant period == period_self
    in the default mode (no sort inclusive).
       Since total_period is computed by summing up period_self,
       period/total_period can be > 100%
       Fix the bug by updating period_self as well.
       Signed-off-by: Arun Sharma <asharma@fb.com>

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index a2a8d91..adc891e 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -462,6 +462,7 @@ static bool hists__collapse_insert_entry(struct 
hists *hists,

		if (!cmp) {
			iter->period += he->period;
+			iter->period_self += he->period_self;
			iter->nr_events += he->nr_events;
			if (symbol_conf.use_callchain) {
				callchain_cursor_reset(&hists->callchain_cursor);

  reply	other threads:[~2012-08-08 19:17 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-31  5:40 [PATCH] perf: Add a new sort order: SORT_INCLUSIVE (v6) Arun Sharma
2012-03-31  5:43 ` Arun Sharma
2012-03-31  5:43   ` Arun Sharma
2012-08-08 19:16   ` Arun Sharma [this message]
2012-08-08 19:16     ` Arun Sharma
2012-08-08 22:37     ` Arun Sharma
2012-08-08 22:37       ` Arun Sharma
2012-08-09  0:33     ` Namhyung Kim
2012-08-09  0:33       ` Namhyung Kim

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=5022BB0E.8050706@fb.com \
    --to=asharma@fb.com \
    --cc=acme@redhat.com \
    --cc=efault@gmx.de \
    --cc=eranian@google.com \
    --cc=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=namhyung.kim@lge.com \
    --cc=paulus@samba.org \
    --cc=peterz@infradead.org \
    --cc=tzanussi@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.