* [PATCH tip 1/1] trace: assign defaults at register_ftrace_event
@ 2009-02-04 22:16 Arnaldo Carvalho de Melo
2009-02-04 23:41 ` Frederic Weisbecker
2009-02-05 13:36 ` Ingo Molnar
0 siblings, 2 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2009-02-04 22:16 UTC (permalink / raw)
To: Steven Rostedt
Cc: Ingo Molnar, Frédéric Weisbecker, Jens Axboe,
Linux Kernel Mailing List
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH tip 1/1] trace: assign defaults at register_ftrace_event
2009-02-04 22:16 [PATCH tip 1/1] trace: assign defaults at register_ftrace_event Arnaldo Carvalho de Melo
@ 2009-02-04 23:41 ` Frederic Weisbecker
2009-02-05 1:22 ` Arnaldo Carvalho de Melo
2009-02-05 13:36 ` Ingo Molnar
1 sibling, 1 reply; 10+ messages in thread
From: Frederic Weisbecker @ 2009-02-04 23:41 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Steven Rostedt, Ingo Molnar, Jens Axboe,
Linux Kernel Mailing List
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH tip 1/1] trace: assign defaults at register_ftrace_event
2009-02-04 23:41 ` Frederic Weisbecker
@ 2009-02-05 1:22 ` Arnaldo Carvalho de Melo
2009-02-05 1:24 ` Ingo Molnar
0 siblings, 1 reply; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2009-02-05 1:22 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: Steven Rostedt, Ingo Molnar, Jens Axboe,
Linux Kernel Mailing List
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? :-)
- Arnaldo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH tip 1/1] trace: assign defaults at register_ftrace_event
2009-02-05 1:22 ` Arnaldo Carvalho de Melo
@ 2009-02-05 1:24 ` Ingo Molnar
2009-02-05 1:37 ` Arnaldo Carvalho de Melo
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Ingo Molnar @ 2009-02-05 1:24 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Frederic Weisbecker, Steven Rostedt, Jens Axboe,
Linux Kernel Mailing List
* 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH tip 1/1] trace: assign defaults at register_ftrace_event
2009-02-05 1:24 ` Ingo Molnar
@ 2009-02-05 1:37 ` Arnaldo Carvalho de Melo
2009-02-05 1:41 ` Frederic Weisbecker
2009-02-05 1:44 ` Steven Rostedt
2 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2009-02-05 1:37 UTC (permalink / raw)
To: Ingo Molnar
Cc: Arnaldo Carvalho de Melo, Frederic Weisbecker, Steven Rostedt,
Jens Axboe, Linux Kernel Mailing List
Em Thu, Feb 05, 2009 at 02:24:30AM +0100, Ingo Molnar escreveu:
> * Arnaldo Carvalho de Melo <acme@redhat.com> wrote:
> > Em Thu, Feb 05, 2009 at 12:41:51AM +0100, Frederic Weisbecker escreveu:
> > > 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 :-)
I really appreciate that, this fine granularity on who is doing what is
really refreshing, deserved and encouraging.
- Arnaldo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH tip 1/1] trace: assign defaults at register_ftrace_event
2009-02-05 1:24 ` Ingo Molnar
2009-02-05 1:37 ` Arnaldo Carvalho de Melo
@ 2009-02-05 1:41 ` Frederic Weisbecker
2009-02-05 1:55 ` Ingo Molnar
2009-02-05 1:44 ` Steven Rostedt
2 siblings, 1 reply; 10+ messages in thread
From: Frederic Weisbecker @ 2009-02-05 1:41 UTC (permalink / raw)
To: Ingo Molnar
Cc: Arnaldo Carvalho de Melo, Steven Rostedt, Jens Axboe,
Linux Kernel Mailing List
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 :-)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH tip 1/1] trace: assign defaults at register_ftrace_event
2009-02-05 1:24 ` Ingo Molnar
2009-02-05 1:37 ` Arnaldo Carvalho de Melo
2009-02-05 1:41 ` Frederic Weisbecker
@ 2009-02-05 1:44 ` Steven Rostedt
2 siblings, 0 replies; 10+ messages in thread
From: Steven Rostedt @ 2009-02-05 1:44 UTC (permalink / raw)
To: Ingo Molnar
Cc: Arnaldo Carvalho de Melo, Frederic Weisbecker, Jens Axboe,
Linux Kernel Mailing List
On Thu, 5 Feb 2009, Ingo Molnar wrote:
> > >
> > > 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 :-)
Although it is probably too late, I'll join in on the fun.
Acked-by: Steven Rostedt <srostedt@redhat.com>
-- Steve
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH tip 1/1] trace: assign defaults at register_ftrace_event
2009-02-05 1:41 ` Frederic Weisbecker
@ 2009-02-05 1:55 ` Ingo Molnar
2009-02-05 2:05 ` Frederic Weisbecker
0 siblings, 1 reply; 10+ messages in thread
From: Ingo Molnar @ 2009-02-05 1:55 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: Arnaldo Carvalho de Melo, Steven Rostedt, Jens Axboe,
Linux Kernel Mailing List
* 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH tip 1/1] trace: assign defaults at register_ftrace_event
2009-02-05 1:55 ` Ingo Molnar
@ 2009-02-05 2:05 ` Frederic Weisbecker
0 siblings, 0 replies; 10+ messages in thread
From: Frederic Weisbecker @ 2009-02-05 2:05 UTC (permalink / raw)
To: Ingo Molnar
Cc: Arnaldo Carvalho de Melo, Steven Rostedt, Jens Axboe,
Linux Kernel Mailing List
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!
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH tip 1/1] trace: assign defaults at register_ftrace_event
2009-02-04 22:16 [PATCH tip 1/1] trace: assign defaults at register_ftrace_event Arnaldo Carvalho de Melo
2009-02-04 23:41 ` Frederic Weisbecker
@ 2009-02-05 13:36 ` Ingo Molnar
1 sibling, 0 replies; 10+ messages in thread
From: Ingo Molnar @ 2009-02-05 13:36 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Steven Rostedt, Frédéric Weisbecker, Jens Axboe,
Linux Kernel Mailing List
* Arnaldo Carvalho de Melo <acme@redhat.com> 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(-)
Applied to tip/tracing/ftrace, thanks guys!
Ingo
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2009-02-05 13:37 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-02-04 22:16 [PATCH tip 1/1] trace: assign defaults at register_ftrace_event Arnaldo Carvalho de Melo
2009-02-04 23:41 ` Frederic Weisbecker
2009-02-05 1:22 ` Arnaldo Carvalho de Melo
2009-02-05 1:24 ` Ingo Molnar
2009-02-05 1:37 ` Arnaldo Carvalho de Melo
2009-02-05 1:41 ` Frederic Weisbecker
2009-02-05 1:55 ` Ingo Molnar
2009-02-05 2:05 ` Frederic Weisbecker
2009-02-05 1:44 ` Steven Rostedt
2009-02-05 13:36 ` Ingo Molnar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox