lkml.org 
[lkml]   [2009]   [Jul]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH -tip] tracing: use defined fields to print formats

On Tue, 21 Jul 2009, Lai Jiangshan wrote:
>
> There're 4 differences:
>
> 1) the type of "common_type" is changed.
> It's brought by a bug `__common_field(int, type, 1);`
> It'll be fixed.
>
> 2) Type Item[Len] ==> Type[Len] Item.
> (There are the same, )But it can't not be recovered as before.
> Because it's brought by `trace_define_field(event_call, #type "[" #len "]", #item,`
> I'll fix trace-cmd and make it supports these two style together.

I'll wait for your next patch and see if I can take a crack at getting it
back to the old way.

>
> 3) double quotation marks for the format is missing.
> It'll be fixed.
>
> 4) a line in events/ftrace/print/format is missing.
> It'll be fixed.
>
> At the end(fixed patch), the only change is "Type Item[Len] ==> Type[Len] Item".
>
> The values of this patch are:
> 1) A big function ftrace_format_##call() is defined for every event type.
> it waste a lot of memory. this patch saves these memory.

I agree.

>
> 2) reduce coupling: ftrace_format_##call() and
> ftrace_define_fields_##call() are almost the same.
> We need to maintain them together, it's not good design.

I agree.

>
> The difference between ftrace_format_##call() and
> ftrace_define_fields_##call() implies a bug.
>
> Example: `__common_field(int, type, 1);` in
> ftrace_define_fields_##call() is a bug.

The problem is that this will not make it until 32, and 31 already has the
type item[x] format. Which means tools will be expecting it that way. We
can make the tools handle both, but I would really like to prevent that.

I totally agree that your patch is a nice clean up, and saves space and
prevents more bugs. On the other hand, we are changing an API to users
where all tools that parse this must be able to cope.

I'll wait for your next patch and see if I can't pull something out of my
CPP magic hat that can save the day ;-)

-- Steve



\
 
 \ /
  Last update: 2009-07-21 03:23    [W:1.702 / U:0.004 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site