Re: [RFC PATCH] perf session: fixing uninitialised cpumode

From: Jiri Olsa <jolsa@redhat.com>
To: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: acme@kernel.org, alexander.shishkin@linux.intel.com,
	jolsa@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH] perf session: fixing uninitialised cpumode
Date: Mon, 16 May 2016 08:01:32 +0200	[thread overview]
Message-ID: <20160516060132.GB10877@krava> (raw)
In-Reply-To: <1463182576-6035-1-git-send-email-mathieu.poirier@linaro.org>

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
> 