Steven,

Thank you for reviewing my patchset.
I will update both patch 1/2 and 2/2 in accordance with your comment.

Seiji

> -----Original Message-----
> From: Steven Rostedt [mailto:rost...@goodmis.org]
> Sent: Tuesday, January 15, 2013 8:25 PM
> To: Seiji Aguchi
> Cc: x...@kernel.org; linux-kernel@vger.kernel.org; H. Peter Anvin 
> (h...@zytor.com); Thomas Gleixner (t...@linutronix.de);
> 'mi...@elte.hu' (mi...@elte.hu); Borislav Petkov (b...@alien8.de); Satoru 
> Moriya; dle-deve...@lists.sourceforge.net; linux-
> e...@vger.kernel.org; Luck, Tony (tony.l...@intel.com)
> Subject: Re: [RFC][PATCH v7 1/2] trace,x86: add x86 irq vector tracepoints
> 
> On Thu, 2013-01-10 at 17:33 +0000, Seiji Aguchi wrote:
> 
> > diff --git a/arch/x86/include/asm/trace/irq_vectors.h
> > b/arch/x86/include/asm/trace/irq_vectors.h
> > new file mode 100644
> > index 0000000..9bcb27b
> > --- /dev/null
> > +++ b/arch/x86/include/asm/trace/irq_vectors.h
> > @@ -0,0 +1,154 @@
> > +#undef TRACE_SYSTEM
> > +#define TRACE_SYSTEM irq_vectors
> > +
> > +#if !defined(_TRACE_IRQ_VECTORS_H) ||
> > +defined(TRACE_HEADER_MULTI_READ) #define _TRACE_IRQ_VECTORS_H
> > +
> > +#include <linux/tracepoint.h>
> > +
> > +extern void trace_irq_vector_regfunc(void); extern void
> > +trace_irq_vector_unregfunc(void);
> > +
> > +#define DECLARE_IRQ_VECTOR_EVENT(name)                             \
> > +TRACE_EVENT_FN(name,                                               \
> > +   TP_PROTO(int vector),                                   \
> > +                                                           \
> > +   TP_ARGS(vector),                                        \
> > +                                                           \
> > +   TP_STRUCT__entry(                                       \
> > +           __field(        int,    vector  )               \
> > +   ),                                                      \
> > +                                                           \
> > +   TP_fast_assign(                                         \
> > +           __entry->vector = vector;                       \
> > +   ),                                                      \
> > +                                                           \
> > +   TP_printk("vector=%d", __entry->vector),                \
> > +   trace_irq_vector_regfunc, trace_irq_vector_unregfunc    \
> > +);
> > +
> > +/*
> > + * local_timer_entry - called before entering a local timer interrupt
> > + * vector handler
> > + */
> > +DECLARE_IRQ_VECTOR_EVENT(local_timer_entry)
> 
> This is a great big waste of space. Add the below patch to your series and 
> then just do:
> 
> DECLARE_EVENT_CLASS(x86_irq_vector,
> 
>       TP_PROTO(int vector),
> 
>       TP_ARGS(vector),
> 
>       TP_STRUCT__entry(
>               __field(                int,    vector  )
>       ),
> 
>       TP_fast_assign(
>               __entry->vector = vector;
>       ),
> 
>       TP_printk("vector=%d", __entry->vector) );
> 
> #define DEFINE_IRQ_VECTOR_EVENT(name) \
> DEFINE_EVENT_FN(name, \
>       TP_PROTO(int vector), \
>       TP_ARGS(vector), \
>       trace_irq_vector_regfunc, \
>       trace_irq_vector_unregfunc);
> 
> And keep the rest the same.
> 
> > +
> > +/*
> > + * local_timer_exit - called immediately after the interrupt vector
> > + * handler returns
> > + */
> > +DECLARE_IRQ_VECTOR_EVENT(local_timer_exit)
> > +
> > +/*
> > + * reschedule_entry - called before entering a reschedule vector
> > +handler  */
> > +DECLARE_IRQ_VECTOR_EVENT(reschedule_entry)
> > +
> > +/*
> > + * reschedule_exit - called immediately after the interrupt vector
> > + * handler returns
> > + */
> > +DECLARE_IRQ_VECTOR_EVENT(reschedule_exit)
> > +
> > +/*
> > + * spurious_apic_entry - called before entering a spurious apic
> > +vector handler  */
> > +DECLARE_IRQ_VECTOR_EVENT(spurious_apic_entry)
> > +
> > +/*
> > + * spurious_apic_exit - called immediately after the interrupt vector
> > + * handler returns
> > + */
> > +DECLARE_IRQ_VECTOR_EVENT(spurious_apic_exit)
> > +
> > +/*
> > + * error_apic_entry - called before entering an error apic vector
> > +handler  */
> > +DECLARE_IRQ_VECTOR_EVENT(error_apic_entry)
> > +
> > +/*
> > + * error_apic_exit - called immediately after the interrupt vector
> > + * handler returns
> > + */
> > +DECLARE_IRQ_VECTOR_EVENT(error_apic_exit)
> > +
> > +/*
> > + * x86_platform_ipi_entry - called before entering a x86 platform ipi
> > +interrupt
> > + * vector handler
> > + */
> > +DECLARE_IRQ_VECTOR_EVENT(x86_platform_ipi_entry)
> > +
> > +/*
> > + * x86_platform_ipi_exit - called immediately after the interrupt
> > +vector
> > + * handler returns
> > + */
> > +DECLARE_IRQ_VECTOR_EVENT(x86_platform_ipi_exit)
> > +
> > +/*
> > + * irq_work_entry - called before entering a irq work interrupt
> > + * vector handler
> > + */
> > +DECLARE_IRQ_VECTOR_EVENT(irq_work_entry)
> > +
> > +/*
> > + * irq_work_exit - called immediately after the interrupt vector
> > + * handler returns
> > + */
> > +DECLARE_IRQ_VECTOR_EVENT(irq_work_exit)
> > +
> > +/*
> > + * call_function_entry - called before entering a call function
> > +interrupt
> > + * vector handler
> > + */
> > +DECLARE_IRQ_VECTOR_EVENT(call_function_entry)
> > +
> > +/*
> > + * call_function_exit - called immediately after the interrupt vector
> > + * handler returns
> > + */
> > +DECLARE_IRQ_VECTOR_EVENT(call_function_exit)
> > +
> > +/*
> > + * call_function_single_entry - called before entering a call
> > +function
> > + * single interrupt vector handler
> > + */
> > +DECLARE_IRQ_VECTOR_EVENT(call_function_single_entry)
> > +
> > +/*
> > + * call_function_single_exit - called immediately after the interrupt
> > +vector
> > + * handler returns
> > + */
> > +DECLARE_IRQ_VECTOR_EVENT(call_function_single_exit)
> > +
> > +/*
> > + * threshold_apic_entry - called before entering a threshold apic
> > +interrupt
> > + * vector handler
> > + */
> > +DECLARE_IRQ_VECTOR_EVENT(threshold_apic_entry)
> > +
> > +/*
> > + * threshold_apic_exit - called immediately after the interrupt
> > +vector
> > + * handler returns
> > + */
> > +DECLARE_IRQ_VECTOR_EVENT(threshold_apic_exit)
> > +
> > +/*
> > + * thermal_apic_entry - called before entering a thermal apic
> > +interrupt
> > + * vector handler
> > + */
> > +DECLARE_IRQ_VECTOR_EVENT(thermal_apic_entry)
> > +
> > +/*
> > + * thrmal_apic_exit - called immediately after the interrupt vector
> > + * handler returns
> > + */
> > +DECLARE_IRQ_VECTOR_EVENT(thermal_apic_exit)
> > +
> > +#undef TRACE_INCLUDE_PATH
> > +#define TRACE_INCLUDE_PATH ../../arch/x86/include/asm/trace #define
> > +TRACE_INCLUDE_FILE irq_vectors #endif /*  _TRACE_IRQ_VECTORS_H */
> > +
> > +/* This part must be outside protection */ #include
> > +<trace/define_trace.h>
> > +
> 
> ---
> tracing: Add DEFINE_EVENT_FN() macro
> 
> Each TRACE_EVENT() adds several helper functions. If two or more trace events 
> share the same structure and print format, they can
> also share most of these helper functions and save a lot of space from 
> duplicate code. This is why the DECLARE_EVENT_CLASS() and
> DEFINE_EVENT() were created.
> 
> Some events require a trigger to be called at registering and unregistering 
> of the event and to do so they use TRACE_EVENT_FN().
> 
> If multiple events require a trigger, they currently have no choice but to 
> use TRACE_EVENT_FN() as there's no DEFINE_EVENT_FN()
> available. This unfortunately causes a lot of wasted duplicate code created.
> 
> By adding a DEFINE_EVENT_FN(), these events can still use a
> DECLARE_EVENT_CLASS() and then define their own triggers.
> 
> Signed-off-by: Steven Rostedt <rost...@goodmis.org>
> 
> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h index 
> 2f322c3..9bf59e5 100644
> --- a/include/linux/tracepoint.h
> +++ b/include/linux/tracepoint.h
> @@ -378,6 +378,8 @@ static inline void tracepoint_synchronize_unregister(void)
>  #define DECLARE_EVENT_CLASS(name, proto, args, tstruct, assign, print)
>  #define DEFINE_EVENT(template, name, proto, args)            \
>       DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
> +#define DEFINE_EVENT_FN(template, name, proto, args, reg, unreg)\
> +     DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
>  #define DEFINE_EVENT_PRINT(template, name, proto, args, print)       \
>       DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
>  #define DEFINE_EVENT_CONDITION(template, name, proto,                \
> diff --git a/include/trace/define_trace.h b/include/trace/define_trace.h 
> index 1905ca8..02e1003 100644
> --- a/include/trace/define_trace.h
> +++ b/include/trace/define_trace.h
> @@ -44,6 +44,10 @@
>  #define DEFINE_EVENT(template, name, proto, args) \
>       DEFINE_TRACE(name)
> 
> +#undef DEFINE_EVENT_FN
> +#define DEFINE_EVENT_FN(template, name, proto, args, reg, unreg) \
> +     DEFINE_TRACE_FN(name, reg, unreg)
> +
>  #undef DEFINE_EVENT_PRINT
>  #define DEFINE_EVENT_PRINT(template, name, proto, args, print)       \
>       DEFINE_TRACE(name)
> @@ -91,6 +95,7 @@
>  #undef TRACE_EVENT_CONDITION
>  #undef DECLARE_EVENT_CLASS
>  #undef DEFINE_EVENT
> +#undef DEFINE_EVENT_FN
>  #undef DEFINE_EVENT_PRINT
>  #undef DEFINE_EVENT_CONDITION
>  #undef TRACE_HEADER_MULTI_READ
> diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h index 
> 40dc5e8..7bab676 100644
> --- a/include/trace/ftrace.h
> +++ b/include/trace/ftrace.h
> @@ -71,6 +71,10 @@
>       static struct ftrace_event_call __used          \
>       __attribute__((__aligned__(4))) event_##name
> 
> +#undef DEFINE_EVENT_FN
> +#define DEFINE_EVENT_FN(template, name, proto, args, reg, unreg)     \
> +     DEFINE_EVENT(template, name, PARAMS(proto), PARAMS(args))
> +
>  #undef DEFINE_EVENT_PRINT
>  #define DEFINE_EVENT_PRINT(template, name, proto, args, print)       \
>       DEFINE_EVENT(template, name, PARAMS(proto), PARAMS(args))
> 

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

Reply via email to