Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1610639 > unrolled thread
| Started by | Taeung Song <treeze.taeung@gmail.com> |
|---|---|
| First post | 2017-03-28 14:20 +0200 |
| Last post | 2017-04-05 08:00 +0200 |
| Articles | 8 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH v4] perf annotate: Fix missing number of samples for source_line_samples Taeung Song <treeze.taeung@gmail.com> - 2017-03-28 14:20 +0200
Re: [PATCH v4] perf annotate: Fix missing number of samples for source_line_samples Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-03-28 15:50 +0200
Re: [PATCH v4] perf annotate: Fix missing number of samples for source_line_samples Taeung Song <treeze.taeung@gmail.com> - 2017-03-28 16:30 +0200
Re: [PATCH v4] perf annotate: Fix missing number of samples for source_line_samples Taeung Song <treeze.taeung@gmail.com> - 2017-03-28 16:30 +0200
Re: [PATCH v4] perf annotate: Fix missing number of samples for source_line_samples Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-03-28 17:30 +0200
Re: [PATCH v4] perf annotate: Fix missing number of samples for source_line_samples Taeung Song <treeze.taeung@gmail.com> - 2017-03-28 18:20 +0200
Re: [PATCH v4] perf annotate: Fix missing number of samples for source_line_samples Taeung Song <taeung@kosslab.kr> - 2017-03-29 11:00 +0200
[tip:perf/core] perf annotate: Fix missing number of samples for source_line_samples tip-bot for Taeung Song <tipbot@zytor.com> - 2017-04-05 08:00 +0200
| From | Taeung Song <treeze.taeung@gmail.com> |
|---|---|
| Date | 2017-03-28 14:20 +0200 |
| Subject | [PATCH v4] perf annotate: Fix missing number of samples for source_line_samples |
| Message-ID | <tpYgr-7p2-61@gated-at.bofh.it> |
The option 'show-total-period' works fine without a option '-l'.
But 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 it was missed to set number of samples
of source_line_samples, 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>
Cc: Martin Liska <mliska@suse.cz>
Fixes: 0c4a5bcea460 ("perf annotate: Display total number of samples with --show-total-period")
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-28 15:50 +0200 |
| Subject | Re: [PATCH v4] perf annotate: Fix missing number of samples for source_line_samples |
| Message-ID | <tpZFy-8kZ-71@gated-at.bofh.it> |
| In reply to | #1610639 |
Em Tue, Mar 28, 2017 at 09:12:05PM +0900, Taeung Song escreveu:
> The option 'show-total-period' works fine without a option '-l'.
> But if running 'perf annotate --stdio -l --show-total-period',
> you can see a problem showing only zero '0' for number of samples.
If you do that at that point, don't you have to change the TUI not to do
it again?
- Arnaldo
> 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 it was missed to set number of samples
> of source_line_samples, 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>
> Cc: Martin Liska <mliska@suse.cz>
> Fixes: 0c4a5bcea460 ("perf annotate: Display total number of samples with --show-total-period")
> 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 16:30 +0200 |
| Subject | Re: [PATCH v4] perf annotate: Fix missing number of samples for source_line_samples |
| Message-ID | <tq0id-ss-5@gated-at.bofh.it> |
| In reply to | #1610954 |
On 03/28/2017 10:48 PM, Arnaldo Carvalho de Melo wrote:
> Em Tue, Mar 28, 2017 at 09:12:05PM +0900, Taeung Song escreveu:
>> The option 'show-total-period' works fine without a option '-l'.
>> But if running 'perf annotate --stdio -l --show-total-period',
>> you can see a problem showing only zero '0' for number of samples.
>
> If you do that at that point, don't you have to change the TUI not to do
> it again?
>
> - Arnaldo
>
Yes, there isn't the same problem when using 't' on TUI of 'perf annotate'.
(As you know, 't' is show-total-period option)
In 'annotate' TUI, we can use the 't' functionality without the problem.
Because of the code (util/annotate.c:911 and ui/browsers/annotate.c:412)
as below.
881 double disasm__calc_percent(struct annotation *notes, int evidx,
s64 offset,
882 s64 end, const char **path, u64
*nr_samples)
883 {
...
910 if (h->sum) {
911 *nr_samples = hits;
And,
378 static void annotate_browser__calc_percent(struct annotate_browser
*browser,
379 struct perf_evsel *evsel)
380 {
...
407 bpos->samples[i].percent =
disasm__calc_percent(notes,
408 evsel->idx + i,
409 pos->offset,
410 next ?
next->offset : len,
411 &path, &nr_samples);
412 bpos->samples[i].nr = nr_samples;
And 'perf annotate --stdio --show-total-period' hasn't the problem, too.
Because of the same code (util/annotate.c:911).
But if with '-l', i.e. 'perf annotate --stdio -l --show-total-period',
the problem happen. Because 'nr' of source_line_samples aren't
ordinarily set
in symbol__get_source_line() (util/annotate.c:1634~..)
,so fix it as this patch
Are there something I missed or misunderstand ?
Thanks,
Taeung
>> 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 it was missed to set number of samples
>> of source_line_samples, 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>
>> Cc: Martin Liska <mliska@suse.cz>
>> Fixes: 0c4a5bcea460 ("perf annotate: Display total number of samples with --show-total-period")
>> 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 16:30 +0200 |
| Subject | Re: [PATCH v4] perf annotate: Fix missing number of samples for source_line_samples |
| Message-ID | <tq0id-ss-13@gated-at.bofh.it> |
| In reply to | #1611023 |
On 03/28/2017 11:21 PM, Taeung Song wrote:
>
>
> On 03/28/2017 10:48 PM, Arnaldo Carvalho de Melo wrote:
>> Em Tue, Mar 28, 2017 at 09:12:05PM +0900, Taeung Song escreveu:
>>> The option 'show-total-period' works fine without a option '-l'.
>>> But if running 'perf annotate --stdio -l --show-total-period',
>>> you can see a problem showing only zero '0' for number of samples.
>>
>> If you do that at that point, don't you have to change the TUI not to do
>> it again?
>>
>> - Arnaldo
>>
>
> Yes, there isn't the same problem when using 't' on TUI of 'perf annotate'.
Sorry, missay the reply.
I don't change the TUI to fix same problem of this patch.
The reason are like below..
> (As you know, 't' is show-total-period option)
>
> In 'annotate' TUI, we can use the 't' functionality without the problem.
> Because of the code (util/annotate.c:911 and ui/browsers/annotate.c:412)
> as below.
>
> 881 double disasm__calc_percent(struct annotation *notes, int evidx,
> s64 offset,
> 882 s64 end, const char **path, u64
> *nr_samples)
> 883 {
> ...
> 910 if (h->sum) {
> 911 *nr_samples = hits;
>
> And,
>
> 378 static void annotate_browser__calc_percent(struct annotate_browser
> *browser,
> 379 struct perf_evsel *evsel)
> 380 {
> ...
> 407 bpos->samples[i].percent =
> disasm__calc_percent(notes,
> 408 evsel->idx + i,
> 409 pos->offset,
> 410 next ? next->offset
> : len,
> 411 &path, &nr_samples);
> 412 bpos->samples[i].nr = nr_samples;
>
>
> And 'perf annotate --stdio --show-total-period' hasn't the problem, too.
> Because of the same code (util/annotate.c:911).
>
>
> But if with '-l', i.e. 'perf annotate --stdio -l --show-total-period',
> the problem happen. Because 'nr' of source_line_samples aren't
> ordinarily set
> in symbol__get_source_line() (util/annotate.c:1634~..)
> ,so fix it as this patch
>
> Are there something I missed or misunderstand ?
>
>
> Thanks,
> Taeung
>
>
>>> 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 it was missed to set number of samples
>>> of source_line_samples, 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>
>>> Cc: Martin Liska <mliska@suse.cz>
>>> Fixes: 0c4a5bcea460 ("perf annotate: Display total number of samples
>>> with --show-total-period")
>>> 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 | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2017-03-28 17:30 +0200 |
| Subject | Re: [PATCH v4] perf annotate: Fix missing number of samples for source_line_samples |
| Message-ID | <tq1ej-1a1-35@gated-at.bofh.it> |
| In reply to | #1611023 |
Em Tue, Mar 28, 2017 at 11:21:33PM +0900, Taeung Song escreveu:
>
>
> On 03/28/2017 10:48 PM, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Mar 28, 2017 at 09:12:05PM +0900, Taeung Song escreveu:
> > > The option 'show-total-period' works fine without a option '-l'.
> > > But if running 'perf annotate --stdio -l --show-total-period',
> > > you can see a problem showing only zero '0' for number of samples.
> >
> > If you do that at that point, don't you have to change the TUI not to do
> > it again?
> >
> > - Arnaldo
> >
>
> Yes, there isn't the same problem when using 't' on TUI of 'perf annotate'.
> (As you know, 't' is show-total-period option)
Yeah, there is no problem on the TUI, but since you do it in a function
taht the TUI uses, it may not be needed to do this in
tools/perf/util/annotate.c _and_ in tools/perf/ui/browsers/annotate.c,
right?
- Arnaldo
> In 'annotate' TUI, we can use the 't' functionality without the problem.
> Because of the code (util/annotate.c:911 and ui/browsers/annotate.c:412) as
> below.
>
> 881 double disasm__calc_percent(struct annotation *notes, int evidx, s64
> offset,
> 882 s64 end, const char **path, u64
> *nr_samples)
> 883 {
> ...
> 910 if (h->sum) {
> 911 *nr_samples = hits;
>
> And,
>
> 378 static void annotate_browser__calc_percent(struct annotate_browser
> *browser,
> 379 struct perf_evsel *evsel)
> 380 {
> ...
> 407 bpos->samples[i].percent =
> disasm__calc_percent(notes,
> 408 evsel->idx + i,
> 409 pos->offset,
> 410 next ? next->offset :
> len,
> 411 &path, &nr_samples);
> 412 bpos->samples[i].nr = nr_samples;
>
>
> And 'perf annotate --stdio --show-total-period' hasn't the problem, too.
> Because of the same code (util/annotate.c:911).
>
>
> But if with '-l', i.e. 'perf annotate --stdio -l --show-total-period',
> the problem happen. Because 'nr' of source_line_samples aren't ordinarily
> set
> in symbol__get_source_line() (util/annotate.c:1634~..)
> ,so fix it as this patch
>
> Are there something I missed or misunderstand ?
>
>
> Thanks,
> Taeung
>
>
> > > 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 it was missed to set number of samples
> > > of source_line_samples, 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>
> > > Cc: Martin Liska <mliska@suse.cz>
> > > Fixes: 0c4a5bcea460 ("perf annotate: Display total number of samples with --show-total-period")
> > > 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 18:20 +0200 |
| Subject | Re: [PATCH v4] perf annotate: Fix missing number of samples for source_line_samples |
| Message-ID | <tq20G-1Mi-15@gated-at.bofh.it> |
| In reply to | #1611100 |
On 03/29/2017 12:28 AM, Arnaldo Carvalho de Melo wrote:
> Em Tue, Mar 28, 2017 at 11:21:33PM +0900, Taeung Song escreveu:
>>
>>
>> On 03/28/2017 10:48 PM, Arnaldo Carvalho de Melo wrote:
>>> Em Tue, Mar 28, 2017 at 09:12:05PM +0900, Taeung Song escreveu:
>>>> The option 'show-total-period' works fine without a option '-l'.
>>>> But if running 'perf annotate --stdio -l --show-total-period',
>>>> you can see a problem showing only zero '0' for number of samples.
>>>
>>> If you do that at that point, don't you have to change the TUI not to do
>>> it again?
>>>
>>> - Arnaldo
>>>
>>
>> Yes, there isn't the same problem when using 't' on TUI of 'perf annotate'.
>> (As you know, 't' is show-total-period option)
>
> Yeah, there is no problem on the TUI, but since you do it in a function
> taht the TUI uses, it may not be needed to do this in
No, TUI don't use the function I modified in this patch at all.
(i.e. symbol__get_source_line())
Only symbol__tty_annotate() use symbol__get_source_line().
(As you know, annotate TUI use symbol__tui_annotate() )
> tools/perf/util/annotate.c _and_ in tools/perf/ui/browsers/annotate.c,
> right?
>
> - Arnaldo
Are there something I missed or misunderstand ?
Thanks,
Taeung
>
>> In 'annotate' TUI, we can use the 't' functionality without the problem.
>> Because of the code (util/annotate.c:911 and ui/browsers/annotate.c:412) as
>> below.
>>
>> 881 double disasm__calc_percent(struct annotation *notes, int evidx, s64
>> offset,
>> 882 s64 end, const char **path, u64
>> *nr_samples)
>> 883 {
>> ...
>> 910 if (h->sum) {
>> 911 *nr_samples = hits;
>>
>> And,
>>
>> 378 static void annotate_browser__calc_percent(struct annotate_browser
>> *browser,
>> 379 struct perf_evsel *evsel)
>> 380 {
>> ...
>> 407 bpos->samples[i].percent =
>> disasm__calc_percent(notes,
>> 408 evsel->idx + i,
>> 409 pos->offset,
>> 410 next ? next->offset :
>> len,
>> 411 &path, &nr_samples);
>> 412 bpos->samples[i].nr = nr_samples;
>>
>>
>> And 'perf annotate --stdio --show-total-period' hasn't the problem, too.
>> Because of the same code (util/annotate.c:911).
>>
>>
>> But if with '-l', i.e. 'perf annotate --stdio -l --show-total-period',
>> the problem happen. Because 'nr' of source_line_samples aren't ordinarily
>> set
>> in symbol__get_source_line() (util/annotate.c:1634~..)
>> ,so fix it as this patch
>>
>> Are there something I missed or misunderstand ?
>>
>>
>> Thanks,
>> Taeung
>>
>>
>>>> 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 it was missed to set number of samples
>>>> of source_line_samples, 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>
>>>> Cc: Martin Liska <mliska@suse.cz>
>>>> Fixes: 0c4a5bcea460 ("perf annotate: Display total number of samples with --show-total-period")
>>>> 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 <taeung@kosslab.kr> |
|---|---|
| Date | 2017-03-29 11:00 +0200 |
| Subject | Re: [PATCH v4] perf annotate: Fix missing number of samples for source_line_samples |
| Message-ID | <tqhCq-4pL-15@gated-at.bofh.it> |
| In reply to | #1611161 |
Arnaldo,
I think we had a little communication problem.
So would you mind if I finally sum up this situation ?
Currently there are the two calculation setting for 'number of samples'.
(At util/annotate.c, not TUI code ui/browsers/annotate.c)
A) disasm__calc_percent() -> util/annotate.c:881~
B) symbol__get_source_line() -> util/annotate.c:1634~
If testing them as the three cases,
1) $ perf annotate --stdio -l --show-total-period
2) $ perf annotate --stdio --show-total-period
3) $ perf annotate # using 'k' on TUI
The result is,
1) NG
2) OK
3) OK
The problem of '1)' is to just show '0' zero values for number of samples.
Because 'B)' has a bug missing 'nr' of source_line_samples.
So I fix it.
the case '1)' uses 'B)' and 'A)'
the case '2)' uses only 'A)'
the case '3)' uses only 'A)'
I read and analyzed all about 'number of samples' of perf-anntoate.
with the cset 0c4a5bcea460 you gave me.
This is my conclusion. What do you think about it ?
Thanks,
Taeung
NOTE: 'A)'(i.e. disasm__calc_percent()) has two alternative logic
when using '-l' option or not as below.
888 if (src_line) {
...
903 } else {
...
914 }
On 03/29/2017 01:15 AM, Taeung Song wrote:
>
>
> On 03/29/2017 12:28 AM, Arnaldo Carvalho de Melo wrote:
>> Em Tue, Mar 28, 2017 at 11:21:33PM +0900, Taeung Song escreveu:
>>>
>>>
>>> On 03/28/2017 10:48 PM, Arnaldo Carvalho de Melo wrote:
>>>> Em Tue, Mar 28, 2017 at 09:12:05PM +0900, Taeung Song escreveu:
>>>>> The option 'show-total-period' works fine without a option '-l'.
>>>>> But if running 'perf annotate --stdio -l --show-total-period',
>>>>> you can see a problem showing only zero '0' for number of samples.
>>>>
>>>> If you do that at that point, don't you have to change the TUI not
>>>> to do
>>>> it again?
>>>>
>>>> - Arnaldo
>>>>
>>>
>>> Yes, there isn't the same problem when using 't' on TUI of 'perf
>>> annotate'.
>>> (As you know, 't' is show-total-period option)
>>
>> Yeah, there is no problem on the TUI, but since you do it in a function
>> taht the TUI uses, it may not be needed to do this in
>
> No, TUI don't use the function I modified in this patch at all.
> (i.e. symbol__get_source_line())
>
> Only symbol__tty_annotate() use symbol__get_source_line().
>
> (As you know, annotate TUI use symbol__tui_annotate() )
>
>> tools/perf/util/annotate.c _and_ in tools/perf/ui/browsers/annotate.c,
>> right?
>>
>> - Arnaldo
>
>
> Are there something I missed or misunderstand ?
>
> Thanks,
> Taeung
>
SNIP
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Taeung Song <tipbot@zytor.com> |
|---|---|
| Date | 2017-04-05 08:00 +0200 |
| Subject | [tip:perf/core] perf annotate: Fix missing number of samples for source_line_samples |
| Message-ID | <tsM94-6Yl-13@gated-at.bofh.it> |
| In reply to | #1610639 |
Commit-ID: 99094a5e941fe88d95cbd594e6a41bee24003ecb
Gitweb: http://git.kernel.org/tip/99094a5e941fe88d95cbd594e6a41bee24003ecb
Author: Taeung Song <treeze.taeung@gmail.com>
AuthorDate: Tue, 28 Mar 2017 21:12:05 +0900
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 4 Apr 2017 21:08:00 -0300
perf annotate: Fix missing number of samples for source_line_samples
The option 'show-total-period' works fine without a option '-l'. But 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 it was missed to set number of samples of
source_line_samples, 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
...
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Martin Liska <mliska@suse.cz>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Fixes: 0c4a5bcea460 ("perf annotate: Display total number of samples with --show-total-period")
Link: http://lkml.kernel.org/r/1490703125-13643-1-git-send-email-treeze.taeung@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.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 {
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web