From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752416AbbCTPkR (ORCPT ); Fri, 20 Mar 2015 11:40:17 -0400 Received: from smtprelay0191.hostedemail.com ([216.40.44.191]:58963 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750967AbbCTPkO (ORCPT ); Fri, 20 Mar 2015 11:40:14 -0400 X-Session-Marker: 726F737465647440676F6F646D69732E6F7267 X-Spam-Summary: 2,0,0,,d41d8cd98f00b204,rostedt@goodmis.org,:::::::::,RULES_HIT:41:355:379:421:541:599:800:960:966:973:988:989:1260:1277:1311:1313:1314:1345:1359:1431:1437:1515:1516:1518:1534:1543:1593:1594:1605:1711:1712:1730:1747:1777:1792:1801:2196:2199:2393:2553:2559:2562:2895:3138:3139:3140:3141:3142:3622:3865:3866:3867:3871:3872:3873:3874:4321:4385:4605:5007:6119:6261:7208:7514:7875:7903:8603:9113:10004:10226:10400:10848:10967:11026:11232:11658:11914:12043:12296:12438:12517:12519:12555:12663:12740:13138:13231:13870:14096:14097:21080,0,RBL:none,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:fn,MSBL:0,DNSBL:none,Custom_rules:0:0:0 X-HE-Tag: hands18_1de5c0ab9830 X-Filterd-Recvd-Size: 4216 Date: Fri, 20 Mar 2015 11:40:11 -0400 From: Steven Rostedt To: Arnaldo Carvalho de Melo Cc: David Ahern , linux-kernel@vger.kernel.org, Namhyung Kim , Jiri Olsa Subject: Re: [PATCH] perf trace: Handle legacy syscalls Message-ID: <20150320114011.4e25167c@gandalf.local.home> In-Reply-To: <20150320153226.GI16485@kernel.org> References: <1426790181-19118-1-git-send-email-dsahern@gmail.com> <20150320153226.GI16485@kernel.org> X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.25; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 20 Mar 2015 12:32:26 -0300 Arnaldo Carvalho de Melo wrote: > 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); Can we make it "pevent_free_format_field" to keep consistency. > > > > 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? I'm fine with the change if the name of the function is updated. -- Steve > > 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.