[PATCH] perf: make perf report -D print sampled CPU

[PATCH] perf: make perf report -D print sampled CPU

[Stephane Eranian]

It is useful to know on which CPU a sample was captured on.
The information is captured with perf record -R but it was
not printed out by perf report -D. This patch adds this.

When -R is not used, cpu is set to -1to indicate that
the CPU is unknown (it is not captured).

Signed-off-by: Stephane Eranian <eranian@google.com>

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 3592057..207da18 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -157,8 +157,9 @@ static int process_sample_event(event_t *event, struct perf_session *session)
 
 	event__parse_sample(event, session->sample_type, &data);
 
-	dump_printf("(IP, %d): %d/%d: %#Lx period: %Ld\n", event->header.misc,
-		    data.pid, data.tid, data.ip, data.period);
+	dump_printf("(IP, %d): %d/%d: %#Lx period: %Ld cpu:%d\n",
+		    event->header.misc, data.pid, data.tid, data.ip,
+		    data.period, data.cpu);
 
 	if (session->sample_type & PERF_SAMPLE_CALLCHAIN) {
 		unsigned int i;
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 50771b5..58cb96b 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -770,7 +770,8 @@ int event__parse_sample(event_t *event, u64 type, struct sample_data *data)
 		u32 *p = (u32 *)array;
 		data->cpu = *p;
 		array++;
-	}
+	} else
+		data->cpu = -1;
 
 	if (type & PERF_SAMPLE_PERIOD) {
 		data->period = *array;

Re: [PATCH] perf: make perf report -D print sampled CPU

[Arnaldo Carvalho de Melo]

Em Fri, May 28, 2010 at 12:08:01PM +0200, Stephane Eranian escreveu:
> It is useful to know on which CPU a sample was captured on.
> The information is captured with perf record -R but it was
> not printed out by perf report -D. This patch adds this.
> 
> When -R is not used, cpu is set to -1to indicate that
> the CPU is unknown (it is not captured).
> 
> Signed-off-by: Stephane Eranian <eranian@google.com>

It conflicts with Arun's patch for sorting by CPU that I started playing with
yesterday, but in a good way, i.e. printing the CPU at that dump_printf spot
was one of the changs I made in Arun's patch as well.

[root@emilia linux-2.6-tip]# perf report -D | grep SAMPLE | head -6
0x10240 [0x28]: PERF_RECORD_SAMPLE(IP, 1): 2 17252/17252: 0xffffffff81253362 period: 1894446
0x10268 [0x28]: PERF_RECORD_SAMPLE(IP, 1): 2 17252/17252: 0xffffffff81253338 period: 1894446
0x10290 [0x28]: PERF_RECORD_SAMPLE(IP, 1): 2 17252/17252: 0xffffffff8125300d period: 1905103
0x102b8 [0x28]: PERF_RECORD_SAMPLE(IP, 1): 0 0/0: 0xffffffff8109117d period: 1838260
0x102e0 [0x28]: PERF_RECORD_SAMPLE(IP, 1): 0 293/293: 0xffffffff8101ace6 period: 352697
0x10308 [0x28]: PERF_RECORD_SAMPLE(IP, 1): 0 1763/1763: 0xffffffff8101ace6 period: 352697
[root@emilia linux-2.6-tip]# 

I added it just before the pid/tid but prefixing it with "cpu: " is as good as
doing it that way.

I'm applying both patches after some tests,

Thanks!

- Arnaldo

Re: [PATCH] perf: make perf report -D print sampled CPU

[Stephane Eranian]

On Fri, May 28, 2010 at 2:57 PM, Arnaldo Carvalho de Melo
<acme@infradead.org> wrote:
> Em Fri, May 28, 2010 at 12:08:01PM +0200, Stephane Eranian escreveu:
>> It is useful to know on which CPU a sample was captured on.
>> The information is captured with perf record -R but it was
>> not printed out by perf report -D. This patch adds this.
>>
>> When -R is not used, cpu is set to -1to indicate that
>> the CPU is unknown (it is not captured).
>>
>> Signed-off-by: Stephane Eranian <eranian@google.com>
>
> It conflicts with Arun's patch for sorting by CPU that I started playing with
> yesterday, but in a good way, i.e. printing the CPU at that dump_printf spot
> was one of the changs I made in Arun's patch as well.
>
Yes, I realized that afterwards.

> [root@emilia linux-2.6-tip]# perf report -D | grep SAMPLE | head -6
> 0x10240 [0x28]: PERF_RECORD_SAMPLE(IP, 1): 2 17252/17252: 0xffffffff81253362 period: 1894446
> 0x10268 [0x28]: PERF_RECORD_SAMPLE(IP, 1): 2 17252/17252: 0xffffffff81253338 period: 1894446
> 0x10290 [0x28]: PERF_RECORD_SAMPLE(IP, 1): 2 17252/17252: 0xffffffff8125300d period: 1905103
> 0x102b8 [0x28]: PERF_RECORD_SAMPLE(IP, 1): 0 0/0: 0xffffffff8109117d period: 1838260
> 0x102e0 [0x28]: PERF_RECORD_SAMPLE(IP, 1): 0 293/293: 0xffffffff8101ace6 period: 352697
> 0x10308 [0x28]: PERF_RECORD_SAMPLE(IP, 1): 0 1763/1763: 0xffffffff8101ace6 period: 352697
> [root@emilia linux-2.6-tip]#
>
> I added it just before the pid/tid but prefixing it with "cpu: " is as good as
> doing it that way.
>
Fine. I think there needs to be some documenation explaining the structure
of the line here. I had to dig into the code to understand it.

> I'm applying both patches after some tests,
>
Thanks.

Re: [PATCH] perf: make perf report -D print sampled CPU

[Arnaldo Carvalho de Melo]

Em Fri, May 28, 2010 at 03:03:15PM +0200, Stephane Eranian escreveu:
> On Fri, May 28, 2010 at 2:57 PM, Arnaldo Carvalho de Melo
> <acme@infradead.org> wrote:
> > Em Fri, May 28, 2010 at 12:08:01PM +0200, Stephane Eranian escreveu:
> >> When -R is not used, cpu is set to -1to indicate that
> >> the CPU is unknown (it is not captured).

> > It conflicts with Arun's patch for sorting by CPU that I started playing with
> > yesterday, but in a good way, i.e. printing the CPU at that dump_printf spot
> > was one of the changs I made in Arun's patch as well.

> Yes, I realized that afterwards.

> > [root@emilia linux-2.6-tip]# perf report -D | grep SAMPLE | head -6
> > 0x10240 [0x28]: PERF_RECORD_SAMPLE(IP, 1): 2 17252/17252: 0xffffffff81253362 period: 1894446
> > 0x10268 [0x28]: PERF_RECORD_SAMPLE(IP, 1): 2 17252/17252: 0xffffffff81253338 period: 1894446
> > 0x10290 [0x28]: PERF_RECORD_SAMPLE(IP, 1): 2 17252/17252: 0xffffffff8125300d period: 1905103
> > 0x102b8 [0x28]: PERF_RECORD_SAMPLE(IP, 1): 0 0/0: 0xffffffff8109117d period: 1838260
> > 0x102e0 [0x28]: PERF_RECORD_SAMPLE(IP, 1): 0 293/293: 0xffffffff8101ace6 period: 352697
> > 0x10308 [0x28]: PERF_RECORD_SAMPLE(IP, 1): 0 1763/1763: 0xffffffff8101ace6 period: 352697
> > [root@emilia linux-2.6-tip]#

> > I added it just before the pid/tid but prefixing it with "cpu: " is as good as
> > doing it that way.

> Fine. I think there needs to be some documenation explaining the structure
> of the line here. I had to dig into the code to understand it.

Right, this so far is as ad-hoc as it can be :-)

One idea that is in my TODO list is to generate some XML format people
say that oprofile produces and there is also some other format that a
valgrind tool uses that should be supported.

I also plan to have it in a spreadsheet TUI widget, allowing lots of
navigation gimmicks to help in debugging, like a popup that offers the
list of mmaps for the thread where the sample below the cursor happened,
etc.

- Arnaldo

Re: [PATCH] perf: make perf report -D print sampled CPU

[Stephane Eranian]

On Fri, May 28, 2010 at 3:37 PM, Arnaldo Carvalho de Melo
<acme@infradead.org> wrote:
> Em Fri, May 28, 2010 at 03:03:15PM +0200, Stephane Eranian escreveu:
>> On Fri, May 28, 2010 at 2:57 PM, Arnaldo Carvalho de Melo
>> <acme@infradead.org> wrote:
>> > Em Fri, May 28, 2010 at 12:08:01PM +0200, Stephane Eranian escreveu:
>> >> When -R is not used, cpu is set to -1to indicate that
>> >> the CPU is unknown (it is not captured).
>
>> > It conflicts with Arun's patch for sorting by CPU that I started playing with
>> > yesterday, but in a good way, i.e. printing the CPU at that dump_printf spot
>> > was one of the changs I made in Arun's patch as well.
>
>> Yes, I realized that afterwards.
>
>> > [root@emilia linux-2.6-tip]# perf report -D | grep SAMPLE | head -6
>> > 0x10240 [0x28]: PERF_RECORD_SAMPLE(IP, 1): 2 17252/17252: 0xffffffff81253362 period: 1894446
>> > 0x10268 [0x28]: PERF_RECORD_SAMPLE(IP, 1): 2 17252/17252: 0xffffffff81253338 period: 1894446
>> > 0x10290 [0x28]: PERF_RECORD_SAMPLE(IP, 1): 2 17252/17252: 0xffffffff8125300d period: 1905103
>> > 0x102b8 [0x28]: PERF_RECORD_SAMPLE(IP, 1): 0 0/0: 0xffffffff8109117d period: 1838260
>> > 0x102e0 [0x28]: PERF_RECORD_SAMPLE(IP, 1): 0 293/293: 0xffffffff8101ace6 period: 352697
>> > 0x10308 [0x28]: PERF_RECORD_SAMPLE(IP, 1): 0 1763/1763: 0xffffffff8101ace6 period: 352697
>> > [root@emilia linux-2.6-tip]#
>
>> > I added it just before the pid/tid but prefixing it with "cpu: " is as good as
>> > doing it that way.
>
>> Fine. I think there needs to be some documenation explaining the structure
>> of the line here. I had to dig into the code to understand it.
>
> Right, this so far is as ad-hoc as it can be :-)
>
> One idea that is in my TODO list is to generate some XML format people
> say that oprofile produces and there is also some other format that a
> valgrind tool uses that should be supported.
>
I think for a lot of people what matter is that the format be easily parseable
by Python/Perl scripts. XML may be fine too.

> I also plan to have it in a spreadsheet TUI widget, allowing lots of
> navigation gimmicks to help in debugging, like a popup that offers the
> list of mmaps for the thread where the sample below the cursor happened,
> etc.
You may get tons of data. Not sure you could manage interactively.

Re: [PATCH] perf: make perf report -D print sampled CPU

[Arnaldo Carvalho de Melo]

Em Fri, May 28, 2010 at 03:44:31PM +0200, Stephane Eranian escreveu:
> On Fri, May 28, 2010 at 3:37 PM, Arnaldo Carvalho de Melo
> <acme@infradead.org> wrote:
> > Em Fri, May 28, 2010 at 03:03:15PM +0200, Stephane Eranian escreveu:
> >> On Fri, May 28, 2010 at 2:57 PM, Arnaldo Carvalho de Melo
> >> <acme@infradead.org> wrote:
> >> > Em Fri, May 28, 2010 at 12:08:01PM +0200, Stephane Eranian escreveu:
> >> Fine. I think there needs to be some documenation explaining the structure
> >> of the line here. I had to dig into the code to understand it.
> >
> > Right, this so far is as ad-hoc as it can be :-)
> >
> > One idea that is in my TODO list is to generate some XML format people
> > say that oprofile produces and there is also some other format that a
> > valgrind tool uses that should be supported.

> I think for a lot of people what matter is that the format be easily parseable
> by Python/Perl scripts. XML may be fine too.

Agreed, I mentioned the existing formats because they may be good enough
and would allow us to take advantage of existing tools that process
them.
 
> > I also plan to have it in a spreadsheet TUI widget, allowing lots of
> > navigation gimmicks to help in debugging, like a popup that offers
> > the list of mmaps for the thread where the sample below the cursor
> > happened, etc.

> You may get tons of data. Not sure you could manage interactively.

Right, it wouldn't support all cases, but I'm working on new TUI widgets
that try hard not to create several layers of data structures to browse
the perf.data file, i.e. filtering shouldn't traverse all the entries,
just set variables that will be inspected when we move to a different
page, etc.

- Arnaldo