Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1609521 > unrolled thread
| Started by | Taeung Song <treeze.taeung@gmail.com> |
|---|---|
| First post | 2017-03-27 09:20 +0200 |
| Last post | 2017-03-28 16:50 +0200 |
| Articles | 4 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH v3 3/3] perf annotate: Fix missing number of samples Taeung Song <treeze.taeung@gmail.com> - 2017-03-27 09:20 +0200
Re: [PATCH v3 3/3] perf annotate: Fix missing number of samples Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-03-27 20:30 +0200
Re: [PATCH v3 3/3] perf annotate: Fix missing number of samples Taeung Song <treeze.taeung@gmail.com> - 2017-03-28 14:20 +0200
Re: [PATCH v3 3/3] perf annotate: Fix missing number of samples Taeung Song <treeze.taeung@gmail.com> - 2017-03-28 16:50 +0200
| From | Taeung Song <treeze.taeung@gmail.com> |
|---|---|
| Date | 2017-03-27 09:20 +0200 |
| Subject | [PATCH v3 3/3] perf annotate: Fix missing number of samples |
| Message-ID | <tpx6y-4iG-17@gated-at.bofh.it> |
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.
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
[toc] | [next] | [standalone]
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2017-03-27 20:30 +0200 |
| Message-ID | <tpHyV-3ND-1@gated-at.bofh.it> |
| In reply to | #1609521 |
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
[toc] | [prev] | [next] | [standalone]
| From | Taeung Song <treeze.taeung@gmail.com> |
|---|---|
| Date | 2017-03-28 14:20 +0200 |
| Message-ID | <tpYgr-7p2-53@gated-at.bofh.it> |
| In reply to | #1610047 |
Good morning, Arnaldo :)
On 03/28/2017 03:26 AM, Arnaldo Carvalho de Melo wrote:
> 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
>
Okey! I look into the cset 0c4a5bcea460.
It is fine but if running 'show-total-period' with '-l',
the problem happen. The reason is to miss setting number of samples
for source_line_samples, so will send v4 added Fixes: and Cc: Martin
(and a bit changed commit log message)
Thanks,
Taeung
[toc] | [prev] | [next] | [standalone]
| From | Taeung Song <treeze.taeung@gmail.com> |
|---|---|
| Date | 2017-03-28 16:50 +0200 |
| Message-ID | <tq0BB-A0-41@gated-at.bofh.it> |
| In reply to | #1610638 |
On 03/28/2017 09:09 PM, Taeung Song wrote:
> Good morning, Arnaldo :)
>
> On 03/28/2017 03:26 AM, Arnaldo Carvalho de Melo wrote:
>> 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.
>>
Sorry, I misunderstood what you said.
Already there is the calculation setting in util/annotate.c
Because of the cset e64aa75bf5 ("perf annotate browser: Use
disasm__calc_percent()") by Namhyung.
So I think that we don't need to do as you guess.
Thanks,
Taeung
>> 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
>>
>
> Okey! I look into the cset 0c4a5bcea460.
>
> It is fine but if running 'show-total-period' with '-l',
> the problem happen. The reason is to miss setting number of samples
> for source_line_samples, so will send v4 added Fixes: and Cc: Martin
> (and a bit changed commit log message)
>
> Thanks,
> Taeung
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web