Hi,
> From: Arnaldo Carvalho de Melo <[email protected]>
>
> Since now symbol__addr_inc_samples() does the auto alloc, no need to
> do it prior to calling hist_entry__inc_addr_samples.
perf annotate on a ppc64 build (no TUI) is failing. I get zero output.
I haven't had a chance to look closer, but I used the following git
bisect test script to isolate:
#!/bin/sh
cd tools/perf
make || exit 125
PERF=./perf
$PERF record -a sleep 5
if [ -z "`$PERF annotate`" ]; then
exit 1
else
exit 0
fi
Anton
Em Wed, Feb 12, 2014 at 06:23:16PM +1100, Anton Blanchard escreveu:
> Hi,
>
> > From: Arnaldo Carvalho de Melo <[email protected]>
> >
> > Since now symbol__addr_inc_samples() does the auto alloc, no need to
> > do it prior to calling hist_entry__inc_addr_samples.
>
> perf annotate on a ppc64 build (no TUI) is failing. I get zero output.
> I haven't had a chance to look closer, but I used the following git
> bisect test script to isolate:
Can you try the following patch?
It should fix another problem, i.e. we were allocating, but annotation
would fail in the !TUI case, as it would return at
symbol__inc_addr_samples when use_browser != 1, now it will allocate and
mark the right bucket.
I'll have this in perf/urgent and will do the optimization of not
allocating those buckets in the report case when not doing integrated
annotation, i.e. report --stdio doesn't provide a way to go to the
annotation --stdio, so no point on allocating the buckets. Just on
'annotate --stdio' we should allocate it, etc.
- Arnaldo
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 469eb679fb9d..7cf522523c12 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -489,7 +489,7 @@ static int symbol__inc_addr_samples(struct symbol *sym, struct map *map,
{
struct annotation *notes;
- if (sym == NULL || use_browser != 1 || !sort__has_sym)
+ if (sym == NULL || !sort__has_sym)
return 0;
notes = symbol__annotation(sym);
Hi Arnaldo,
> Can you try the following patch?
>
> It should fix another problem, i.e. we were allocating, but annotation
> would fail in the !TUI case, as it would return at
> symbol__inc_addr_samples when use_browser != 1, now it will allocate
> and mark the right bucket.
>
> I'll have this in perf/urgent and will do the optimization of not
> allocating those buckets in the report case when not doing integrated
> annotation, i.e. report --stdio doesn't provide a way to go to the
> annotation --stdio, so no point on allocating the buckets. Just on
> 'annotate --stdio' we should allocate it, etc.
This fixes the issue, thanks!
Tested-by: Anton Blanchard <[email protected]>
Anton
--
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 469eb679fb9d..7cf522523c12 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -489,7 +489,7 @@ static int symbol__inc_addr_samples(struct symbol
> *sym, struct map *map, {
> struct annotation *notes;
>
> - if (sym == NULL || use_browser != 1 || !sort__has_sym)
> + if (sym == NULL || !sort__has_sym)
> return 0;
>
> notes = symbol__annotation(sym);
>
Hi,
> > Can you try the following patch?
> >
> > It should fix another problem, i.e. we were allocating, but
> > annotation would fail in the !TUI case, as it would return at
> > symbol__inc_addr_samples when use_browser != 1, now it will allocate
> > and mark the right bucket.
> >
> > I'll have this in perf/urgent and will do the optimization of not
> > allocating those buckets in the report case when not doing
> > integrated annotation, i.e. report --stdio doesn't provide a way to
> > go to the annotation --stdio, so no point on allocating the
> > buckets. Just on 'annotate --stdio' we should allocate it, etc.
>
> This fixes the issue, thanks!
After some more testing, perf report can SEGV with this patch:
Program received signal SIGSEGV, Segmentation fault.
__symbol__inc_addr_samples (addr=569128, evidx=0, notes=0x1023af80, map=0x10191ef0, sym=0x1023afb0) at util/annotate.c:477
477 util/annotate.c: No such file or directory.
(gdb) backtrace
#0 __symbol__inc_addr_samples (addr=569128, evidx=0, notes=0x1023af80, map=0x10191ef0, sym=0x1023afb0) at util/annotate.c:477
#1 symbol__inc_addr_samples (addr=<optimised out>, evidx=<optimised out>, map=0x10191ef0, sym=0x1023afb0) at util/annotate.c:501
#2 hist_entry__inc_addr_samples (he=<optimised out>, evidx=<optimised out>, ip=569128) at util/annotate.c:511
#3 0x00000000100183b8 in report__add_hist_entry (sample=0x3fffffffd970, al=0x3fffffffd770, evsel=0x10190f10, tool=<optimised out>) at builtin-report.c:208
#4 process_sample_event (tool=<optimised out>, event=<optimised out>, sample=0x3fffffffd970, evsel=0x10190f10, machine=<optimised out>) at builtin-report.c:250
#5 0x000000001007eac8 in perf_session_deliver_event (session=0x10190330, event=<optimised out>, sample=<optimised out>, tool=<optimised out>,
file_offset=<optimised out>) at util/session.c:985
#6 0x000000001007f40c in flush_sample_queue (s=0x10190330, tool=0x3fffffffdf90) at util/session.c:505
#7 0x0000000010081064 in __perf_session__process_events (session=0x10190330, data_offset=<optimised out>, data_size=<optimised out>, file_size=331392,
tool=0x3fffffffdf90) at util/session.c:1355
(gdb) print notes->src
$3 = (struct annotated_source *) 0x51
Anton
Hi Anton,
On Thu, 13 Feb 2014 04:09:13 +1100, Anton Blanchard wrote:
> Hi,
>
>> > Can you try the following patch?
>> >
>> > It should fix another problem, i.e. we were allocating, but
>> > annotation would fail in the !TUI case, as it would return at
>> > symbol__inc_addr_samples when use_browser != 1, now it will allocate
>> > and mark the right bucket.
>> >
>> > I'll have this in perf/urgent and will do the optimization of not
>> > allocating those buckets in the report case when not doing
>> > integrated annotation, i.e. report --stdio doesn't provide a way to
>> > go to the annotation --stdio, so no point on allocating the
>> > buckets. Just on 'annotate --stdio' we should allocate it, etc.
>>
>> This fixes the issue, thanks!
>
> After some more testing, perf report can SEGV with this patch:
I think we need to separate the check for annotate and report. The
check was for the report case only and annotate always needs to increate
sample info. Does patch below fix your problem?
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index d882b6f96411..bab762bdeb0d 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -75,6 +75,11 @@ static int report__config(const char *var, const char *value, void *cb)
return perf_default_config(var, value, cb);
}
+static bool report_needs_annotate(void)
+{
+ return use_browser == 1 && sort__has_sym;
+}
+
static int report__add_mem_hist_entry(struct report *rep, struct addr_location *al,
struct perf_sample *sample, struct perf_evsel *evsel)
{
@@ -110,14 +115,16 @@ static int report__add_mem_hist_entry(struct report *rep, struct addr_location *
if (!he)
return -ENOMEM;
- err = hist_entry__inc_addr_samples(he, evsel->idx, al->addr);
- if (err)
- goto out;
+ if (report_needs_annotate()) {
+ err = hist_entry__inc_addr_samples(he, evsel->idx, al->addr);
+ if (err)
+ goto out;
- mx = he->mem_info;
- err = addr_map_symbol__inc_samples(&mx->daddr, evsel->idx);
- if (err)
- goto out;
+ mx = he->mem_info;
+ err = addr_map_symbol__inc_samples(&mx->daddr, evsel->idx);
+ if (err)
+ goto out;
+ }
evsel->hists.stats.total_period += cost;
hists__inc_nr_events(&evsel->hists, PERF_RECORD_SAMPLE);
@@ -159,14 +166,18 @@ static int report__add_branch_hist_entry(struct report *rep, struct addr_locatio
he = __hists__add_entry(&evsel->hists, al, parent, &bi[i], NULL,
1, 1, 0);
if (he) {
- bx = he->branch_info;
- err = addr_map_symbol__inc_samples(&bx->from, evsel->idx);
- if (err)
- goto out;
-
- err = addr_map_symbol__inc_samples(&bx->to, evsel->idx);
- if (err)
- goto out;
+ if (report_needs_annotate()) {
+ bx = he->branch_info;
+ err = addr_map_symbol__inc_samples(&bx->from,
+ evsel->idx);
+ if (err)
+ goto out;
+
+ err = addr_map_symbol__inc_samples(&bx->to,
+ evsel->idx);
+ if (err)
+ goto out;
+ }
evsel->hists.stats.total_period += 1;
hists__inc_nr_events(&evsel->hists, PERF_RECORD_SAMPLE);
@@ -199,7 +210,9 @@ static int report__add_hist_entry(struct report *rep, struct perf_evsel *evsel,
if (err)
goto out;
- err = hist_entry__inc_addr_samples(he, evsel->idx, al->addr);
+ if (report_needs_annotate())
+ err = hist_entry__inc_addr_samples(he, evsel->idx, al->addr);
+
evsel->hists.stats.total_period += sample->period;
hists__inc_nr_events(&evsel->hists, PERF_RECORD_SAMPLE);
out:
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 469eb679fb9d..6fcada625c86 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -489,7 +489,7 @@ static int symbol__inc_addr_samples(struct symbol *sym, struct map *map,
{
struct annotation *notes;
- if (sym == NULL || use_browser != 1 || !sort__has_sym)
+ if (sym == NULL)
return 0;
notes = symbol__annotation(sym);