All of lore.kernel.org
 help / color / mirror / Atom feed
From: Frederic Weisbecker <fweisbec@gmail.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Ian Munsie <imunsie@au1.ibm.com>,
	linux-kernel@vger.kernel.org,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Paul Mackerras <paulus@samba.org>, Ingo Molnar <mingo@elte.hu>,
	Arnaldo Carvalho de Melo <acme@redhat.com>,
	Tom Zanussi <tzanussi@gmail.com>
Subject: Re: [PATCH 2/7] perf trace: Correctly handle arrays
Date: Sat, 15 May 2010 05:20:34 +0200	[thread overview]
Message-ID: <20100515032032.GB8150@nowhere> (raw)
In-Reply-To: <1273768145.27703.1095.camel@gandalf.stny.rr.com>

On Thu, May 13, 2010 at 12:29:05PM -0400, Steven Rostedt wrote:
> On Thu, 2010-05-13 at 16:03 +1000, Ian Munsie wrote:
> > From: Ian Munsie <imunsie@au1.ibm.com>
> > 
> > Previously, perf was assuming that an array from an ftrace event was an
> > array of longs, which will not always be the case. Additionally,
> > long_size is not even being initialised, so it would fail to read the
> > data and would erroneously report that the array was filled with zeroes.
> > 
> > This patch adds two extra entries into the field structure - arraylen
> > and elementsize, so that the code can easily look them up instead of
> > assuming that the elements are long_size. The element size is currently
> > derived by the size of the entire array and the number of elements
> > within the array.
> > 
> > This problem can be demonstrated with:
> > perf record -e raw_syscalls:sys_enter -a sleep 0.1
> > perf trace
> > 
> 
> I'm added this to trace-cmd too.
> 
> > Without this patch, output similar to the following is produced:
> >  perf-4355  [004]  1871.504685: sys_enter: NR 319 (0, 0, 0, 0, 0, 0)
> >  perf-4355  [004]  1871.504723: sys_enter: NR 3 (0, 0, 0, 0, 0, 0)
> >  perf-4355  [004]  1871.504733: sys_enter: NR 204 (0, 0, 0, 0, 0, 0)
> > 
> > After applying this patch, the output looks like:
> >  perf-4355  [004]  1871.504685: sys_enter: NR 319 (10247ac0, ffffffff, 5, ffffffff, 0, 107a80d8)
> >  perf-4355  [004]  1871.504723: sys_enter: NR 3 (a, ffb6fcf0, 20, ffffffff, 0, 10112c20)
> >  perf-4355  [004]  1871.504733: sys_enter: NR 204 (a, 4, 800, 6, 0, 10112c20)
> > 
> > Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
> > ---
> >  tools/perf/util/trace-event-parse.c |   11 ++++++++++-
> >  tools/perf/util/trace-event.h       |    2 ++
> >  2 files changed, 12 insertions(+), 1 deletions(-)
> > 
> > diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
> > index 8f470f6..f360f75 100644
> > --- a/tools/perf/util/trace-event-parse.c
> > +++ b/tools/perf/util/trace-event-parse.c
> > @@ -852,6 +852,9 @@ static int event_read_fields(struct event *event, struct format_field **fields)
> >  			field->flags |= FIELD_IS_ARRAY;
> >  
> >  			type = read_token(&token);
> > +			if (test_type(type, EVENT_ITEM))
> > +				goto fail;
> 
> Note this will incorrectly fail on:
> 
> 	field:__data_loc char[] name;
> 
> -- Steve



Nice patch, but indeed this part needs to be fixed.

Thanks.


  reply	other threads:[~2010-05-15  3:20 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-13  6:03 perf trace: Fix array handling & value truncation Ian Munsie
2010-05-13  6:03 ` [PATCH 1/7] perf trace: Defensive programming Ian Munsie
2010-05-13  6:03 ` [PATCH 2/7] perf trace: Correctly handle arrays Ian Munsie
2010-05-13 16:29   ` Steven Rostedt
2010-05-15  3:20     ` Frederic Weisbecker [this message]
2010-05-13 16:32   ` Steven Rostedt
2010-05-14 10:39     ` Peter Zijlstra
2010-05-14 12:54       ` Steven Rostedt
2010-05-13  6:03 ` [PATCH 3/7] Revert "perf: Fix warning while reading ring buffer headers" Ian Munsie
2010-05-15  3:39   ` Frederic Weisbecker
2010-05-13  6:03 ` [PATCH 4/7] perf trace: Rewind pointer in case field in header_page is missing Ian Munsie
2010-05-13  6:03 ` [PATCH 5/7] perf trace: use long_size from trace-event-read Ian Munsie
2010-05-13  6:03 ` [PATCH 6/7] perf trace: Fix value truncation with 64bit kernel and 32bit userspace Ian Munsie
2010-05-13  6:03 ` [PATCH 7/7] perf trace test: Test cases for kernel->host format string conversion Ian Munsie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100515032032.GB8150@nowhere \
    --to=fweisbec@gmail.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@redhat.com \
    --cc=imunsie@au1.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=paulus@samba.org \
    --cc=rostedt@goodmis.org \
    --cc=tzanussi@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.