From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752404AbbCTPcg (ORCPT ); Fri, 20 Mar 2015 11:32:36 -0400 Received: from mail.kernel.org ([198.145.29.136]:54051 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751222AbbCTPcd (ORCPT ); Fri, 20 Mar 2015 11:32:33 -0400 Date: Fri, 20 Mar 2015 12:32:26 -0300 From: Arnaldo Carvalho de Melo To: David Ahern Cc: linux-kernel@vger.kernel.org, Steven Rostedt , Namhyung Kim , Jiri Olsa Subject: Re: [PATCH] perf trace: Handle legacy syscalls Message-ID: <20150320153226.GI16485@kernel.org> References: <1426790181-19118-1-git-send-email-dsahern@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1426790181-19118-1-git-send-email-dsahern@gmail.com> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Em Thu, Mar 19, 2015 at 12:36:21PM -0600, David Ahern escreveu: > Currently the code skips the first field with the expectation that it is > 'nr'. But older kernels do not have the 'nr' field: > > field:int nr; offset:8; size:4; signed:1; > > Change perf-trace to drop the field if it exists after parsing the format > file. > > This fixes the off-by-one problem with older kernels (e.g., RHEL6). e.g, > perf-trace shows this for write: > > 1.515 ( 0.006 ms): dd/4245 write(buf: 2, count: 140733837536224 ) = 26 > > where 2 is really the fd, the huge number is really the buf address, etc. > With this patch you get the more appropriate: > > 1.813 ( 0.003 ms): dd/6330 write(fd: 2, buf: 0x7fff22fc81f0, count: 25 ) = 25 > > Signed-off-by: David Ahern > --- > tools/lib/traceevent/event-parse.c | 11 ++++++++--- > tools/lib/traceevent/event-parse.h | 1 + > tools/perf/builtin-trace.c | 16 +++++++++++++--- > 3 files changed, 22 insertions(+), 6 deletions(-) > > diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c > index afe20ed9fac8..e8a29e730dfb 100644 > --- a/tools/lib/traceevent/event-parse.c > +++ b/tools/lib/traceevent/event-parse.c > @@ -6228,15 +6228,20 @@ void pevent_ref(struct pevent *pevent) > pevent->ref_count++; > } > > +void free_format_field(struct format_field *field) > +{ > + free(field->type); > + free(field->name); > + free(field); > +} > + > static void free_format_fields(struct format_field *field) > { > struct format_field *next; > > while (field) { > next = field->next; > - free(field->type); > - free(field->name); > - free(field); > + free_format_field(field); > field = next; > } > } > diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h > index 5b4efc062320..a548fac646f6 100644 > --- a/tools/lib/traceevent/event-parse.h > +++ b/tools/lib/traceevent/event-parse.h > @@ -619,6 +619,7 @@ enum pevent_errno pevent_parse_format(struct pevent *pevent, > const char *buf, > unsigned long size, const char *sys); > void pevent_free_format(struct event_format *event); > +void free_format_field(struct format_field *field); > > void *pevent_get_field_raw(struct trace_seq *s, struct event_format *event, > const char *name, struct pevent_record *record, > diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c > index dcd950ef2fd7..5caeefeda48a 100644 Up to here we should have a separate patch, one that I would like to have an Acked-by: Rostedt, ok? But then I don't think we need to do that, i.e. we can just have a boolean we set at some point to tell that we need to skip the first entry. I'll try to cook up a patch for that. - Arnaldo > --- a/tools/perf/builtin-trace.c > +++ b/tools/perf/builtin-trace.c > @@ -1442,14 +1442,24 @@ static int syscall__set_arg_fmts(struct syscall *sc) > struct format_field *field; > int idx = 0; > > - sc->arg_scnprintf = calloc(sc->tp_format->format.nr_fields - 1, sizeof(void *)); > + field = sc->tp_format->format.fields; > + /* drop nr field - not relevant here; does not exist on older kernels */ > + if (field && strcmp(field->name, "nr") == 0) { > + struct format_field *next = field->next; > + > + free_format_field(field); > + sc->tp_format->format.fields = next; > + sc->tp_format->format.nr_fields--; > + } > + > + sc->arg_scnprintf = calloc(sc->tp_format->format.nr_fields, sizeof(void *)); > if (sc->arg_scnprintf == NULL) > return -1; > > if (sc->fmt) > sc->arg_parm = sc->fmt->arg_parm; > > - for (field = sc->tp_format->format.fields->next; field; field = field->next) { > + for (field = sc->tp_format->format.fields; field; field = field->next) { > if (sc->fmt && sc->fmt->arg_scnprintf[idx]) > sc->arg_scnprintf[idx] = sc->fmt->arg_scnprintf[idx]; > else if (field->flags & FIELD_IS_POINTER) > @@ -1547,7 +1557,7 @@ static size_t syscall__scnprintf_args(struct syscall *sc, char *bf, size_t size, > .thread = thread, > }; > > - for (field = sc->tp_format->format.fields->next; field; > + for (field = sc->tp_format->format.fields; field; > field = field->next, ++arg.idx, bit <<= 1) { > if (arg.mask & bit) > continue; > -- > 2.2.1