lkml.org 
[lkml]   [2009]   [Oct]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC][PATCH 1/9] tracing/events: Add 'signed' field to format files

* Steven Rostedt <rostedt@goodmis.org> wrote:

> On Wed, 2009-10-07 at 00:04 -0500, Tom Zanussi wrote:
> > On Tue, 2009-10-06 at 21:06 -0400, Steven Rostedt wrote:
> > > On Tue, 2009-10-06 at 01:09 -0500, Tom Zanussi wrote:
> > > > The sign info used for filters in the kernel is also useful to
> > > > applications that process the trace stream. Add it to the format
> > > > files and make it available to userspace.
> > > >
> > > > Signed-off-by: Tom Zanussi <tzanussi@gmail.com>
> > > > ---
> > > > include/trace/ftrace.h | 15 +++++++++------
> > > > kernel/trace/ring_buffer.c | 15 +++++++++------
> > > > kernel/trace/trace_events.c | 24 ++++++++++++------------
> > > > kernel/trace/trace_export.c | 25 ++++++++++++++-----------
> > > > kernel/trace/trace_syscalls.c | 20 +++++++++++++-------
> > > > tools/perf/util/trace-event-parse.c | 24 ++++++++++++++++++++++++
> > > > tools/perf/util/trace-event.h | 1 +
> > > > 7 files changed, 82 insertions(+), 42 deletions(-)
> > > >
> > > > diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
> > > > index cc0d966..c9bbcab 100644
> > > > --- a/include/trace/ftrace.h
> > > > +++ b/include/trace/ftrace.h
> > > > @@ -120,9 +120,10 @@
> > > > #undef __field
> > > > #define __field(type, item) \
> > > > ret = trace_seq_printf(s, "\tfield:" #type " " #item ";\t" \
> > > > - "offset:%u;\tsize:%u;\n", \
> > > > + "offset:%u;\tsize:%u;\tsigned:%u;\n", \
> > > > (unsigned int)offsetof(typeof(field), item), \
> > > > - (unsigned int)sizeof(field.item)); \
> > > > + (unsigned int)sizeof(field.item), \
> > > > + (unsigned int)is_signed_type(type)); \
> > > > if (!ret) \
> > > > return 0;
> > >
> > > I don't mind this change, but it makes me nervous. We really need to
> > > solidify the output format file. This is adding a new field and will
> > > already break the parsers in perf and trace_cmd.
> > >
> > > Is there anything else that is needed? I really want to make sure that
> > > we don't need to modify the output of the format files any more.
> > >
> >
> > One of the things I had in mind when writing this patchset was to make
> > sure that what was there already was enough to write something real on
> > top of. The only thing I found I needed to add was the signed field,
> > and that was enough at least for the Perl case, where everything can be
> > mapped into 3 types - signed, unsigned and string.
> >
> > Other languages might demand more - I haven't looked into it - so I
> > couldn't say whether anything else is needed, but it would seem to
> > me that the current combination of type description strings, sizes
> > and signs for each field should be enough information to allow any
> > field to be passed into any scripting interpreter. Actually, you
> > could probably forget about signs and sizes and just manage on type
> > description strings alone, if you had a typemap that mapped from the
> > C type descriptions to the types expected by a given interpreter.
> > That might be what some language implementations would end up doing,
> > but at least for the Perl implementation, it's much more convenient
> > to have the is_signed field.
>
> Actually I was thinking that the print format part of the format file
> would give enough to know if the object was signed or not, or even the
> type (unsigned long, etc). But we are still early in this game, and I
> would like to think that we are still early enough to be able to
> change these formats. But it's getting close that they will soon be
> locked in stone.

Note, the format _must_ be extensible, even if existing bits are cast
into stone. I.e. if a new type or a new sub-type comes up, we want to be
able to add it - and we want to fix all parsers to be ready for that.

Ingo


\
 
 \ /
  Last update: 2009-10-11 11:05    [W:0.076 / U:0.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site