* [RFC PATCH] perf session: fixing uninitialised cpumode
@ 2016-05-13 23:36 Mathieu Poirier
2016-05-16 6:01 ` Jiri Olsa
0 siblings, 1 reply; 3+ messages in thread
From: Mathieu Poirier @ 2016-05-13 23:36 UTC (permalink / raw)
To: acme; +Cc: alexander.shishkin, jolsa, linux-kernel
The following patch[1] adds a new "cpumode" to the perf_sample
structure that gets initialised as events are read from the data
event file.
With the advent of HW tracers and more specifically the decoding of
the traces they generate, function perf_session__deliver_synth_event()
gets called directly from the infrastructure that decodes the traces,
for example[2].
As such initialisation of perf_sample::cpumode doesn't get done, which
prevents the shared object name from being printed in the perf report
output.
Before this patch:
4.13% 4.13% uname [unknown] [H] 0x0000007fae8b4758
3.74% 3.74% uname [unknown] [H] 0x0000007fae8b4e50
2.06% 2.06% uname [unknown] [H] 0x0000007fae938af4
1.65% 1.65% uname [unknown] [H] 0x0000007fae938ae4
1.59% 1.59% uname [unknown] [H] 0x0000007fae98f7f4
1.50% 1.50% uname [unknown] [H] 0x0000007fae8b4e40
1.43% 1.43% uname [unknown] [H] 0x0000007fae938ac4
1.31% 1.31% uname [unknown] [H] 0x0000007fae86b0c0
1.26% 1.26% uname [unknown] [H] 0x0000007fae99b888
And after this patch:
4.13% 4.13% uname libc-2.21.so [.] 0x0000000000078758
3.74% 3.74% uname libc-2.21.so [.] 0x0000000000078e50
2.06% 2.06% uname libc-2.21.so [.] 0x00000000000fcaf4
1.65% 1.65% uname libc-2.21.so [.] 0x00000000000fcae4
1.59% 1.59% uname ld-2.21.so [.] 0x000000000000a7f4
1.50% 1.50% uname libc-2.21.so [.] 0x0000000000078e40
1.43% 1.43% uname libc-2.21.so [.] 0x00000000000fcac4
1.31% 1.31% uname libc-2.21.so [.] 0x000000000002f0c0
1.26% 1.26% uname ld-2.21.so [.] 0x0000000000016888
This was tested using CoreSight but the same is very likely to happen on
IntelPT. I'm pretty sure this isn't the right solution but we can start
building from that.
Comments welcomed.
[1]. commit 473398a21d28 ("perf tools: Add cpumode to struct perf_sample")
[2]. http://lxr.free-electrons.com/source/tools/perf/util/intel-pt.c#L1806
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
tools/perf/util/session.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 4abd85c6346d..28cc405e7245 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1352,6 +1352,10 @@ int perf_session__deliver_synth_event(struct perf_session *session,
{
struct perf_evlist *evlist = session->evlist;
struct perf_tool *tool = session->tool;
+ u8 mode = event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK;
+
+ if (sample)
+ sample->cpumode = mode;
events_stats__inc(&evlist->stats, event->header.type);
--
2.5.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [RFC PATCH] perf session: fixing uninitialised cpumode
2016-05-13 23:36 [RFC PATCH] perf session: fixing uninitialised cpumode Mathieu Poirier
@ 2016-05-16 6:01 ` Jiri Olsa
2016-05-16 16:37 ` Mathieu Poirier
0 siblings, 1 reply; 3+ messages in thread
From: Jiri Olsa @ 2016-05-16 6:01 UTC (permalink / raw)
To: Mathieu Poirier; +Cc: acme, alexander.shishkin, jolsa, linux-kernel
On Fri, May 13, 2016 at 05:36:16PM -0600, Mathieu Poirier wrote:
> The following patch[1] adds a new "cpumode" to the perf_sample
> structure that gets initialised as events are read from the data
> event file.
>
> With the advent of HW tracers and more specifically the decoding of
> the traces they generate, function perf_session__deliver_synth_event()
> gets called directly from the infrastructure that decodes the traces,
> for example[2].
hum, but this one ends up calling perf_session__deliver_synth_event
with sample == NULL right?
I checked callers of perf_session__deliver_synth_event and it seems
all either init sample->cpumode or pass sample as NULL.. what do I miss?
thanks,
jirka
>
> As such initialisation of perf_sample::cpumode doesn't get done, which
> prevents the shared object name from being printed in the perf report
> output.
>
> Before this patch:
>
> 4.13% 4.13% uname [unknown] [H] 0x0000007fae8b4758
> 3.74% 3.74% uname [unknown] [H] 0x0000007fae8b4e50
> 2.06% 2.06% uname [unknown] [H] 0x0000007fae938af4
> 1.65% 1.65% uname [unknown] [H] 0x0000007fae938ae4
> 1.59% 1.59% uname [unknown] [H] 0x0000007fae98f7f4
> 1.50% 1.50% uname [unknown] [H] 0x0000007fae8b4e40
> 1.43% 1.43% uname [unknown] [H] 0x0000007fae938ac4
> 1.31% 1.31% uname [unknown] [H] 0x0000007fae86b0c0
> 1.26% 1.26% uname [unknown] [H] 0x0000007fae99b888
>
> And after this patch:
>
> 4.13% 4.13% uname libc-2.21.so [.] 0x0000000000078758
> 3.74% 3.74% uname libc-2.21.so [.] 0x0000000000078e50
> 2.06% 2.06% uname libc-2.21.so [.] 0x00000000000fcaf4
> 1.65% 1.65% uname libc-2.21.so [.] 0x00000000000fcae4
> 1.59% 1.59% uname ld-2.21.so [.] 0x000000000000a7f4
> 1.50% 1.50% uname libc-2.21.so [.] 0x0000000000078e40
> 1.43% 1.43% uname libc-2.21.so [.] 0x00000000000fcac4
> 1.31% 1.31% uname libc-2.21.so [.] 0x000000000002f0c0
> 1.26% 1.26% uname ld-2.21.so [.] 0x0000000000016888
>
> This was tested using CoreSight but the same is very likely to happen on
> IntelPT. I'm pretty sure this isn't the right solution but we can start
> building from that.
>
> Comments welcomed.
>
> [1]. commit 473398a21d28 ("perf tools: Add cpumode to struct perf_sample")
> [2]. http://lxr.free-electrons.com/source/tools/perf/util/intel-pt.c#L1806
>
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
> tools/perf/util/session.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 4abd85c6346d..28cc405e7245 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -1352,6 +1352,10 @@ int perf_session__deliver_synth_event(struct perf_session *session,
> {
> struct perf_evlist *evlist = session->evlist;
> struct perf_tool *tool = session->tool;
> + u8 mode = event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK;
> +
> + if (sample)
> + sample->cpumode = mode;
>
> events_stats__inc(&evlist->stats, event->header.type);
>
> --
> 2.5.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFC PATCH] perf session: fixing uninitialised cpumode
2016-05-16 6:01 ` Jiri Olsa
@ 2016-05-16 16:37 ` Mathieu Poirier
0 siblings, 0 replies; 3+ messages in thread
From: Mathieu Poirier @ 2016-05-16 16:37 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, Alexander Shishkin, jolsa, linux-kernel
On 16 May 2016 at 00:01, Jiri Olsa <jolsa@redhat.com> wrote:
> On Fri, May 13, 2016 at 05:36:16PM -0600, Mathieu Poirier wrote:
>> The following patch[1] adds a new "cpumode" to the perf_sample
>> structure that gets initialised as events are read from the data
>> event file.
>>
>> With the advent of HW tracers and more specifically the decoding of
>> the traces they generate, function perf_session__deliver_synth_event()
>> gets called directly from the infrastructure that decodes the traces,
>> for example[2].
>
> hum, but this one ends up calling perf_session__deliver_synth_event
> with sample == NULL right?
Wrong example - this highlights the problem [1]. Here "sample" is
created from scratch and "->cpumode" isn't initialised. Looking at
things with a rested head this morning it is apparent to me that
adding something like:
sample.cpumode = event->header.misc;
to line 1048 is probably a better solution.
Thanks,
Mathieu
[1]. http://lxr.free-electrons.com/source/tools/perf/util/intel-pt.c#L1070
>
> I checked callers of perf_session__deliver_synth_event and it seems
> all either init sample->cpumode or pass sample as NULL.. what do I miss?
>
> thanks,
> jirka
>
>>
>> As such initialisation of perf_sample::cpumode doesn't get done, which
>> prevents the shared object name from being printed in the perf report
>> output.
>>
>> Before this patch:
>>
>> 4.13% 4.13% uname [unknown] [H] 0x0000007fae8b4758
>> 3.74% 3.74% uname [unknown] [H] 0x0000007fae8b4e50
>> 2.06% 2.06% uname [unknown] [H] 0x0000007fae938af4
>> 1.65% 1.65% uname [unknown] [H] 0x0000007fae938ae4
>> 1.59% 1.59% uname [unknown] [H] 0x0000007fae98f7f4
>> 1.50% 1.50% uname [unknown] [H] 0x0000007fae8b4e40
>> 1.43% 1.43% uname [unknown] [H] 0x0000007fae938ac4
>> 1.31% 1.31% uname [unknown] [H] 0x0000007fae86b0c0
>> 1.26% 1.26% uname [unknown] [H] 0x0000007fae99b888
>>
>> And after this patch:
>>
>> 4.13% 4.13% uname libc-2.21.so [.] 0x0000000000078758
>> 3.74% 3.74% uname libc-2.21.so [.] 0x0000000000078e50
>> 2.06% 2.06% uname libc-2.21.so [.] 0x00000000000fcaf4
>> 1.65% 1.65% uname libc-2.21.so [.] 0x00000000000fcae4
>> 1.59% 1.59% uname ld-2.21.so [.] 0x000000000000a7f4
>> 1.50% 1.50% uname libc-2.21.so [.] 0x0000000000078e40
>> 1.43% 1.43% uname libc-2.21.so [.] 0x00000000000fcac4
>> 1.31% 1.31% uname libc-2.21.so [.] 0x000000000002f0c0
>> 1.26% 1.26% uname ld-2.21.so [.] 0x0000000000016888
>>
>> This was tested using CoreSight but the same is very likely to happen on
>> IntelPT. I'm pretty sure this isn't the right solution but we can start
>> building from that.
>>
>> Comments welcomed.
>>
>> [1]. commit 473398a21d28 ("perf tools: Add cpumode to struct perf_sample")
>> [2]. http://lxr.free-electrons.com/source/tools/perf/util/intel-pt.c#L1806
>>
>> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>> ---
>> tools/perf/util/session.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
>> index 4abd85c6346d..28cc405e7245 100644
>> --- a/tools/perf/util/session.c
>> +++ b/tools/perf/util/session.c
>> @@ -1352,6 +1352,10 @@ int perf_session__deliver_synth_event(struct perf_session *session,
>> {
>> struct perf_evlist *evlist = session->evlist;
>> struct perf_tool *tool = session->tool;
>> + u8 mode = event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK;
>> +
>> + if (sample)
>> + sample->cpumode = mode;
>>
>> events_stats__inc(&evlist->stats, event->header.type);
>>
>> --
>> 2.5.0
>>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-05-16 16:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-13 23:36 [RFC PATCH] perf session: fixing uninitialised cpumode Mathieu Poirier
2016-05-16 6:01 ` Jiri Olsa
2016-05-16 16:37 ` Mathieu Poirier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).