lkml.org 
[lkml]   [2009]   [Feb]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH tip 1/1] trace: assign defaults at register_ftrace_event
    On Thu, Feb 05, 2009 at 02:55:47AM +0100, Ingo Molnar wrote:
    >
    > * Frederic Weisbecker <fweisbec@gmail.com> wrote:
    >
    > > On Thu, Feb 05, 2009 at 02:24:30AM +0100, Ingo Molnar wrote:
    > > >
    > > > * Arnaldo Carvalho de Melo <acme@redhat.com> wrote:
    > > >
    > > > > Em Thu, Feb 05, 2009 at 12:41:51AM +0100, Frederic Weisbecker escreveu:
    > > > > > On Wed, Feb 04, 2009 at 08:16:39PM -0200, Arnaldo Carvalho de Melo wrote:
    > > > > > > Impact: simplification of tracers
    > > > > > >
    > > > > > > As all tracers are doing this we might as well do it in
    > > > > > > register_ftrace_event and save one branch each time we call these
    > > > > > > callbacks.
    > > > > > >
    > > > > > > Cc: Ingo Molnar <mingo@elte.hu>
    > > > > > > Cc: Frédéric Weisbecker <fweisbec@gmail.com>
    > > > > > > Cc: Jens Axboe <jens.axboe@oracle.com>
    > > > > > > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
    > > > > > > ---
    > > > > > > block/blktrace.c | 2 --
    > > > > > > kernel/trace/trace.c | 13 +++++--------
    > > > > > > kernel/trace/trace_branch.c | 3 ---
    > > > > > > kernel/trace/trace_output.c | 13 +++++++++++--
    > > > > > > 4 files changed, 16 insertions(+), 15 deletions(-)
    > > > > > >
    > > > > > > diff --git a/block/blktrace.c b/block/blktrace.c
    > > > > > > index c7698d1..1ebd068 100644
    > > > > > > --- a/block/blktrace.c
    > > > > > > +++ b/block/blktrace.c
    > > > > > > @@ -1243,8 +1243,6 @@ static struct trace_event trace_blk_event = {
    > > > > > > .type = TRACE_BLK,
    > > > > > > .trace = blk_trace_event_print,
    > > > > > > .latency_trace = blk_trace_event_print,
    > > > > > > - .raw = trace_nop_print,
    > > > > > > - .hex = trace_nop_print,
    > > > > > > .binary = blk_trace_event_print_binary,
    > > > > > > };
    > > > > > >
    > > > > > > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
    > > > > > > index fd51cf0..a5e4c0a 100644
    > > > > > > --- a/kernel/trace/trace.c
    > > > > > > +++ b/kernel/trace/trace.c
    > > > > > > @@ -1412,7 +1412,7 @@ static enum print_line_t print_lat_fmt(struct trace_iterator *iter)
    > > > > > > goto partial;
    > > > > > > }
    > > > > > >
    > > > > > > - if (event && event->latency_trace)
    > > > > > > + if (event)
    > > > > > > return event->latency_trace(iter, sym_flags);
    > > > > > >
    > > > > > > if (!trace_seq_printf(s, "Unknown type %d\n", entry->type))
    > > > > > > @@ -1441,7 +1441,7 @@ static enum print_line_t print_trace_fmt(struct trace_iterator *iter)
    > > > > > > goto partial;
    > > > > > > }
    > > > > > >
    > > > > > > - if (event && event->trace)
    > > > > > > + if (event)
    > > > > > > return event->trace(iter, sym_flags);
    > > > > > >
    > > > > > > if (!trace_seq_printf(s, "Unknown type %d\n", entry->type))
    > > > > > > @@ -1467,7 +1467,7 @@ static enum print_line_t print_raw_fmt(struct trace_iterator *iter)
    > > > > > > }
    > > > > > >
    > > > > > > event = ftrace_find_event(entry->type);
    > > > > > > - if (event && event->raw)
    > > > > > > + if (event)
    > > > > > > return event->raw(iter, 0);
    > > > > > >
    > > > > > > if (!trace_seq_printf(s, "%d ?\n", entry->type))
    > > > > > > @@ -1494,7 +1494,7 @@ static enum print_line_t print_hex_fmt(struct trace_iterator *iter)
    > > > > > > }
    > > > > > >
    > > > > > > event = ftrace_find_event(entry->type);
    > > > > > > - if (event && event->hex) {
    > > > > > > + if (event) {
    > > > > > > enum print_line_t ret = event->hex(iter, 0);
    > > > > > > if (ret != TRACE_TYPE_HANDLED)
    > > > > > > return ret;
    > > > > > > @@ -1536,10 +1536,7 @@ static enum print_line_t print_bin_fmt(struct trace_iterator *iter)
    > > > > > > }
    > > > > > >
    > > > > > > event = ftrace_find_event(entry->type);
    > > > > > > - if (event && event->binary)
    > > > > > > - return event->binary(iter, 0);
    > > > > > > -
    > > > > > > - return TRACE_TYPE_HANDLED;
    > > > > > > + return event ? event->binary(iter, 0) : TRACE_TYPE_HANDLED;
    > > > > > > }
    > > > > > >
    > > > > > > static int trace_empty(struct trace_iterator *iter)
    > > > > > > diff --git a/kernel/trace/trace_branch.c b/kernel/trace/trace_branch.c
    > > > > > > index 7ac72a4..297deb2 100644
    > > > > > > --- a/kernel/trace/trace_branch.c
    > > > > > > +++ b/kernel/trace/trace_branch.c
    > > > > > > @@ -182,9 +182,6 @@ static struct trace_event trace_branch_event = {
    > > > > > > .type = TRACE_BRANCH,
    > > > > > > .trace = trace_branch_print,
    > > > > > > .latency_trace = trace_branch_print,
    > > > > > > - .raw = trace_nop_print,
    > > > > > > - .hex = trace_nop_print,
    > > > > > > - .binary = trace_nop_print,
    > > > > > > };
    > > > > > >
    > > > > > > static struct tracer branch_trace __read_mostly =
    > > > > > > diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
    > > > > > > index b7380ee..b6e99af 100644
    > > > > > > --- a/kernel/trace/trace_output.c
    > > > > > > +++ b/kernel/trace/trace_output.c
    > > > > > > @@ -435,6 +435,17 @@ int register_ftrace_event(struct trace_event *event)
    > > > > > > if (ftrace_find_event(event->type))
    > > > > > > goto out;
    > > > > > >
    > > > > > > + if (event->trace == NULL)
    > > > > > > + event->trace = trace_nop_print;
    > > > > > > + if (event->latency_trace == NULL)
    > > > > > > + event->latency_trace = trace_nop_print;
    > > > > > > + if (event->raw == NULL)
    > > > > > > + event->raw = trace_nop_print;
    > > > > > > + if (event->hex == NULL)
    > > > > > > + event->hex = trace_nop_print;
    > > > > > > + if (event->binary == NULL)
    > > > > > > + event->binary = trace_nop_print;
    > > > > > > +
    > > > > > > key = event->type & (EVENT_HASHSIZE - 1);
    > > > > > >
    > > > > > > hlist_add_head_rcu(&event->node, &event_hash[key]);
    > > > > > > @@ -874,8 +885,6 @@ static struct trace_event trace_print_event = {
    > > > > > > .trace = trace_print_print,
    > > > > > > .latency_trace = trace_print_print,
    > > > > > > .raw = trace_print_raw,
    > > > > > > - .hex = trace_nop_print,
    > > > > > > - .binary = trace_nop_print,
    > > > > > > };
    > > > > > >
    > > > > > > static struct trace_event *events[] __initdata = {
    > > > > > > --
    > > > > > > 1.6.0.6
    > > > > > >
    > > > > >
    > > > > >
    > > > > > Nice! This avoids one branch for each entry printed.
    > > > >
    > > > > Is that an Acked-by in response to that CC? :-)
    > > >
    > > > yes, i generally treat such replies as an implicit Acked-by and add them -
    > > > already added Fredric's acks to two other commits of yours earlier today. An
    > > > explicit Acked-by is even more helpful though :-)
    > > >
    > > > Ingo
    > >
    > >
    > > Heh, I'm not a tracing/core/ftrace maintainer, so I wouldn't dare put my
    > > Acked-by unless it touches a file I started :-) Concerning
    > > tracing/core/ftrace/tracers, I review the patches that come if I have the
    > > time to and tell what I think about it. But in my opinion, the Acked-by on
    > > this area are more appropriate if they come from Steven :-)
    >
    > if you read a patch, in an area of code that you know well (which you do in
    > this case), and if you find the patch it a step forward with no defects then
    > feel free to post an Acked-by.
    >
    > You absolutely dont have to be the primary author of a file to do that. All
    > such help makes maintenance easier and is much appreciated.
    >
    > Ingo


    Ok, I will remember that.
    Thanks!

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2009-02-05 03:09    [W:0.042 / U:0.084 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site