On Fri, 20 Mar 2015 12:32:26 -0300 Arnaldo Carvalho de Melo <[email protected]> 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</dev/pts/0>, 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</dev/pts/0>, buf: > > 0x7fff22fc81f0, count: 25 ) = 25 > > > > Signed-off-by: David Ahern <[email protected]> > > --- > > 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
