All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Taeung Song <treeze.taeung@gmail.com>
Cc: linux-kernel@vger.kernel.org, "Jiri Olsa" <jolsa@kernel.org>,
	"Namhyung Kim" <namhyung@kernel.org>,
	"Ingo Molnar" <mingo@kernel.org>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Wang Nan" <wangnan0@huawei.com>,
	"Masami Hiramatsu" <mhiramat@kernel.org>,
	"Jiri Olsa" <jolsa@redhat.com>, "Martin Liška" <mliska@suse.cz>
Subject: Re: [PATCH v3 3/3] perf annotate: Fix missing number of samples
Date: Mon, 27 Mar 2017 15:26:31 -0300	[thread overview]
Message-ID: <20170327182631.GC27431@kernel.org> (raw)
In-Reply-To: <1490598638-13947-4-git-send-email-treeze.taeung@gmail.com>

Em Mon, Mar 27, 2017 at 04:10:38PM +0900, Taeung Song escreveu:
> If running 'perf annotate --stdio -l --show-total-period',
> you can see a problem showing only zero '0' for number of samples.
> 
> Before:
>     $ perf annotate --stdio -l --show-total-period
> ...
>        0 :        400816:       push   %rbp
>        0 :        400817:       mov    %rsp,%rbp
>        0 :        40081a:       mov    %edi,-0x24(%rbp)
>        0 :        40081d:       mov    %rsi,-0x30(%rbp)
>        0 :        400821:       mov    -0x24(%rbp),%eax
>        0 :        400824:       mov    -0x30(%rbp),%rdx
>        0 :        400828:       mov    (%rdx),%esi
>        0 :        40082a:       mov    $0x0,%edx
> ...
> 
> The reason is number of samples aren't set
> in symbol__get_source_line(). so set it ordinarily.

Can you please take a look at:

  0c4a5bcea460 ("perf annotate: Display total number of samples with --show-total-period")

that introduced the --show-total-period code and take it into account in
this fix?

I.e. from a quick look it did the calculation setting that field in the
TUI code, where it should have done in the util/annotate.c file, so that
all UIs would be able to use it.

After your analysis, please add a Fixes: that cset, ok?

I applied the other two patches and added Martin to the CC list, as he
is the author of that patch and may have something to say here.

- Arnaldo

 
> After:
>     $ perf annotate --stdio -l --show-total-period
> ...
>        3 :        400816:       push   %rbp
>        4 :        400817:       mov    %rsp,%rbp
>        0 :        40081a:       mov    %edi,-0x24(%rbp)
>        0 :        40081d:       mov    %rsi,-0x30(%rbp)
>        1 :        400821:       mov    -0x24(%rbp),%eax
>        2 :        400824:       mov    -0x30(%rbp),%rdx
>        0 :        400828:       mov    (%rdx),%esi
>        1 :        40082a:       mov    $0x0,%edx
> ...
> 
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> ---
>  tools/perf/util/annotate.c | 6 ++++--
>  tools/perf/util/annotate.h | 2 +-
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 11af5f0..a37032b 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1665,7 +1665,7 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map,
>  	start = map__rip_2objdump(map, sym->start);
>  
>  	for (i = 0; i < len; i++) {
> -		u64 offset;
> +		u64 offset, nr_samples;
>  		double percent_max = 0.0;
>  
>  		src_line->nr_pcnt = nr_pcnt;
> @@ -1674,12 +1674,14 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map,
>  			double percent = 0.0;
>  
>  			h = annotation__histogram(notes, evidx + k);
> +			nr_samples = h->addr[i];
>  			if (h->sum)
> -				percent = 100.0 * h->addr[i] / h->sum;
> +				percent = 100.0 * nr_samples / h->sum;
>  
>  			if (percent > percent_max)
>  				percent_max = percent;
>  			src_line->samples[k].percent = percent;
> +			src_line->samples[k].nr = nr_samples;
>  		}
>  
>  		if (percent_max <= 0.5)
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index 09776b5..948aa8e 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -98,7 +98,7 @@ struct cyc_hist {
>  struct source_line_samples {
>  	double		percent;
>  	double		percent_sum;
> -	double          nr;
> +	u64		nr;
>  };
>  
>  struct source_line {
> -- 
> 2.7.4

  reply	other threads:[~2017-03-27 18:27 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-27  7:10 [PATCH v3 0/3] perf annotate: Simple bugfixes Taeung Song
2017-03-27  7:10 ` [PATCH v3 1/3] perf annotate: Fix a bug reading link name from a build-id file Taeung Song
2017-03-27 18:01   ` Arnaldo Carvalho de Melo
2017-03-28 11:54     ` Taeung Song
2017-03-28  5:55   ` [tip:perf/core] perf annotate: Fix a bug following symbolic link of " tip-bot for Taeung Song
2017-03-27  7:10 ` [PATCH v3 2/3] perf annotate: Fix a bug of division by zero when calculating percent Taeung Song
2017-03-28  5:55   ` [tip:perf/core] " tip-bot for Taeung Song
2017-03-27  7:10 ` [PATCH v3 3/3] perf annotate: Fix missing number of samples Taeung Song
2017-03-27 18:26   ` Arnaldo Carvalho de Melo [this message]
2017-03-28 12:09     ` Taeung Song
2017-03-28 14:38       ` Taeung Song

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=20170327182631.GC27431@kernel.org \
    --to=acme@kernel.org \
    --cc=jolsa@kernel.org \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=mingo@kernel.org \
    --cc=mliska@suse.cz \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=treeze.taeung@gmail.com \
    --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 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.