Messages in this thread | | | Date | Fri, 13 Mar 2009 00:49:12 +0100 | From | Ingo Molnar <> | Subject | Re: [Patch 2/2] tracepoints for softirq entry/exit - tracepoints |
| |
* Frederic Weisbecker <fweisbec@gmail.com> wrote:
> On Thu, Mar 12, 2009 at 02:36:03PM -0400, Jason Baron wrote: > > > > introduce softirq entry/exit tracepoints. These are useful for > > augmenting existing tracers, and to figure out softirq frequencies and > > timings. > > > > Signed-off-by: Jason Baron <jbaron@redhat.com> > > > > --- > > > > include/trace/irq_event_types.h | 12 ++++++++++++ > > kernel/softirq.c | 7 ++++++- > > 2 files changed, 18 insertions(+), 1 deletions(-) > > > > > > diff --git a/include/trace/irq_event_types.h b/include/trace/irq_event_types.h > > index 214bb92..38b4bdd 100644 > > --- a/include/trace/irq_event_types.h > > +++ b/include/trace/irq_event_types.h > > @@ -40,4 +40,16 @@ TRACE_EVENT(irq_handler_exit, > > __entry->irq, __entry->ret ? "handled" : "unhandled") > > ); > > > > +TRACE_FORMAT(irq_softirq_entry, > > + TP_PROTO(struct softirq_action *h, struct softirq_action *vec), > > + TP_ARGS(h, vec), > > + TP_FMT("softirq=%d action=%s", h - vec, softirq_to_name[h-vec]) > > + ); > > + > > +TRACE_FORMAT(irq_softirq_exit, > > + TP_PROTO(struct softirq_action *h, struct softirq_action *vec), > > + TP_ARGS(h, vec), > > + TP_FMT("softirq=%d action=%s", h - vec, softirq_to_name[h-vec]) > > + ); > > + > > #undef TRACE_SYSTEM > > diff --git a/kernel/softirq.c b/kernel/softirq.c > > index ba1511f..c378d53 100644 > > --- a/kernel/softirq.c > > +++ b/kernel/softirq.c > > @@ -24,6 +24,7 @@ > > #include <linux/ftrace.h> > > #include <linux/smp.h> > > #include <linux/tick.h> > > +#include <trace/irq.h> > > > > #include <asm/irq.h> > > /* > > @@ -186,6 +187,9 @@ EXPORT_SYMBOL(local_bh_enable_ip); > > */ > > #define MAX_SOFTIRQ_RESTART 10 > > > > +DEFINE_TRACE(irq_softirq_entry); > > +DEFINE_TRACE(irq_softirq_exit); > > > Just one nit here. The "irq_" prefix seems to me too much. > On the trace we have: > > /* irq_softirq_entry: softirq=nb action=nb_to_logical_name */ > > It's even too much words that says the same things. > Moreover, we have the logical name, the number seems not very useful > because we have its logical translation just after. > > I would suggest to have just: > > /* softirq_entry: nb_to_logical_name */ > > ie: > > /* softirq_entry: SCHED_SOFTIRQ */ > > Don't you think it's more clear and obvious?
yeah.
> Other than that, I think these tracepoints are a good idea.
agreed.
Acked-by: Ingo Molnar <mingo@elte.hu>
Ingo
| |