From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754359AbcEPQhY (ORCPT ); Mon, 16 May 2016 12:37:24 -0400 Received: from mail-yw0-f177.google.com ([209.85.161.177]:33565 "EHLO mail-yw0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753654AbcEPQhX (ORCPT ); Mon, 16 May 2016 12:37:23 -0400 MIME-Version: 1.0 In-Reply-To: <20160516060132.GB10877@krava> References: <1463182576-6035-1-git-send-email-mathieu.poirier@linaro.org> <20160516060132.GB10877@krava> Date: Mon, 16 May 2016 10:37:22 -0600 Message-ID: Subject: Re: [RFC PATCH] perf session: fixing uninitialised cpumode From: Mathieu Poirier To: Jiri Olsa Cc: Arnaldo Carvalho de Melo , Alexander Shishkin , jolsa@kernel.org, "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 16 May 2016 at 00:01, Jiri Olsa 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 >> --- >> 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 >>