TRACE_EVENT_ABI ?

TRACE_EVENT_ABI ?

[Arjan van de Ven]

Hi,

Ingo suggested to make a TRACE_EVENT_ABI, which is equivalent in
implementation to TRACE_EVENT, except that it signals a stable
interface.

In trying to implement this I'm running into a wall where TRACE_EVENT
keeps being defined in many different ways all over the place, making
this a really nasty hack to make it just an alias.

Do you have any clever ideas on how to make this an alias without
fouling up the whole tracing system?

-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: TRACE_EVENT_ABI ?

[Steven Rostedt]

On Mon, 2009-09-21 at 09:36 +0200, Arjan van de Ven wrote:
> Hi,
> 
> Ingo suggested to make a TRACE_EVENT_ABI, which is equivalent in
> implementation to TRACE_EVENT, except that it signals a stable
> interface.
> 
> In trying to implement this I'm running into a wall where TRACE_EVENT
> keeps being defined in many different ways all over the place, making
> this a really nasty hack to make it just an alias.
> 
> Do you have any clever ideas on how to make this an alias without
> fouling up the whole tracing system?

I don't know what you've done so far, but can't you simply in
tracepoint.h define:

#define TRACE_EVENT_API(a,b,c,d,e) TRACE_EVENT(a,b,c,d,e)

And every time TRACE_EVENT_API is used, it will simply do whatever
TRACE_EVENT is at that time?

I've done similar tricks with other code.

-- Steve

Re: TRACE_EVENT_ABI ?

[Steven Rostedt]

[ Resending due to wrong SMTP server ]


On Mon, 21 Sep 2009, Arjan van de Ven wrote:

> Hi,
> 
> Ingo suggested to make a TRACE_EVENT_ABI, which is equivalent in
> implementation to TRACE_EVENT, except that it signals a stable
> interface.
> 
> In trying to implement this I'm running into a wall where TRACE_EVENT
> keeps being defined in many different ways all over the place, making
> this a really nasty hack to make it just an alias.
> 
> Do you have any clever ideas on how to make this an alias without
> fouling up the whole tracing system?
> 

I don't know what you've done so far, but can't you simply in
tracepoint.h define:

#define TRACE_EVENT_API(a,b,c,d,e) TRACE_EVENT(a,b,c,d,e)

And every time TRACE_EVENT_API is used, it will simply do whatever
TRACE_EVENT is at that time?

I've done similar tricks with other code.

-- Steve

Re: TRACE_EVENT_ABI ?

[Arjan van de Ven]

On Mon, 21 Sep 2009 09:26:41 -0400 (EDT)
Steven Rostedt <rostedt@goodmis.org> wrote:

> 
> [ Resending due to wrong SMTP server ]
> 
> 
> On Mon, 21 Sep 2009, Arjan van de Ven wrote:
> 
> > Hi,
> > 
> > Ingo suggested to make a TRACE_EVENT_ABI, which is equivalent in
> > implementation to TRACE_EVENT, except that it signals a stable
> > interface.
> > 
> > In trying to implement this I'm running into a wall where
> > TRACE_EVENT keeps being defined in many different ways all over the
> > place, making this a really nasty hack to make it just an alias.
> > 
> > Do you have any clever ideas on how to make this an alias without
> > fouling up the whole tracing system?
> > 
> 
> I don't know what you've done so far, but can't you simply in
> tracepoint.h define:
> 
> #define TRACE_EVENT_API(a,b,c,d,e) TRACE_EVENT(a,b,c,d,e)

unfortunately, due to the preprocessor trickery around TRACE_EVENT this
completely barfs.


-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: TRACE_EVENT_ABI ?

[Steven Rostedt]

On Mon, 2009-09-21 at 15:28 +0200, Arjan van de Ven wrote:
> On Mon, 21 Sep 2009 09:26:41 -0400 (EDT)
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > 
> > [ Resending due to wrong SMTP server ]
> > 
> > 
> > On Mon, 21 Sep 2009, Arjan van de Ven wrote:
> > 
> > > Hi,
> > > 
> > > Ingo suggested to make a TRACE_EVENT_ABI, which is equivalent in
> > > implementation to TRACE_EVENT, except that it signals a stable
> > > interface.
> > > 
> > > In trying to implement this I'm running into a wall where
> > > TRACE_EVENT keeps being defined in many different ways all over the
> > > place, making this a really nasty hack to make it just an alias.
> > > 
> > > Do you have any clever ideas on how to make this an alias without
> > > fouling up the whole tracing system?
> > > 
> > 
> > I don't know what you've done so far, but can't you simply in
> > tracepoint.h define:
> > 
> > #define TRACE_EVENT_API(a,b,c,d,e) TRACE_EVENT(a,b,c,d,e)
> 
> unfortunately, due to the preprocessor trickery around TRACE_EVENT this
> completely barfs.

I forgot to tell you about PARAMS.

This patch compiled for me;

diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 63a3f7a..9a983d6 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -280,6 +280,10 @@ static inline void tracepoint_synchronize_unregister(void)
  * TRACE_EVENT_FN to perform any (un)registration work.
  */
 
+#define TRACE_EVENT_API(name, proto, args, tstruct, assign, print)	\
+	TRACE_EVENT(name, PARAMS(proto), PARAMS(args),			\
+		    PARAMS(tstruct), PARAMS(assign), PARAMS(print))
+
 #define TRACE_EVENT(name, proto, args, struct, assign, print)	\
 	DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
 #define TRACE_EVENT_FN(name, proto, args, struct,		\
diff --git a/include/trace/events/sched.h b/include/trace/events/sched.h
index b48f1ad..0ee1eb6 100644
--- a/include/trace/events/sched.h
+++ b/include/trace/events/sched.h
@@ -149,7 +149,7 @@ TRACE_EVENT(sched_wakeup_new,
  * (NOTE: the 'rq' argument is not used by generic trace events,
  *        but used by the latency tracer plugin. )
  */
-TRACE_EVENT(sched_switch,
+TRACE_EVENT_API(sched_switch,
 
 	TP_PROTO(struct rq *rq, struct task_struct *prev,
 		 struct task_struct *next),

Re: TRACE_EVENT_ABI ?

[Frederic Weisbecker]

On Mon, Sep 21, 2009 at 12:46:07PM -0400, Steven Rostedt wrote:
> On Mon, 2009-09-21 at 15:28 +0200, Arjan van de Ven wrote:
> > On Mon, 21 Sep 2009 09:26:41 -0400 (EDT)
> > Steven Rostedt <rostedt@goodmis.org> wrote:
> > 
> > > 
> > > [ Resending due to wrong SMTP server ]
> > > 
> > > 
> > > On Mon, 21 Sep 2009, Arjan van de Ven wrote:
> > > 
> > > > Hi,
> > > > 
> > > > Ingo suggested to make a TRACE_EVENT_ABI, which is equivalent in
> > > > implementation to TRACE_EVENT, except that it signals a stable
> > > > interface.
> > > > 
> > > > In trying to implement this I'm running into a wall where
> > > > TRACE_EVENT keeps being defined in many different ways all over the
> > > > place, making this a really nasty hack to make it just an alias.
> > > > 
> > > > Do you have any clever ideas on how to make this an alias without
> > > > fouling up the whole tracing system?
> > > > 
> > > 
> > > I don't know what you've done so far, but can't you simply in
> > > tracepoint.h define:
> > > 
> > > #define TRACE_EVENT_API(a,b,c,d,e) TRACE_EVENT(a,b,c,d,e)
> > 
> > unfortunately, due to the preprocessor trickery around TRACE_EVENT this
> > completely barfs.
> 
> I forgot to tell you about PARAMS.
> 
> This patch compiled for me;



Oh! PARAMS!

I've needed something to "contain" cpp args by the past so that they
are not substituted too early, which is needed while relaying cpp args
from a macro to another. I needed this to fix TRACE_EVENT_FN: it barfed
for the exact same reason.

So I've created __cpparg(), which I'm just discovering, does the exact
same thing than PARAM... :-/

I'm going to s/__cpparg/PARAM for the consistency.


 
> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> index 63a3f7a..9a983d6 100644
> --- a/include/linux/tracepoint.h
> +++ b/include/linux/tracepoint.h
> @@ -280,6 +280,10 @@ static inline void tracepoint_synchronize_unregister(void)
>   * TRACE_EVENT_FN to perform any (un)registration work.
>   */
>  
> +#define TRACE_EVENT_API(name, proto, args, tstruct, assign, print)	\
> +	TRACE_EVENT(name, PARAMS(proto), PARAMS(args),			\
> +		    PARAMS(tstruct), PARAMS(assign), PARAMS(print))
> +
>  #define TRACE_EVENT(name, proto, args, struct, assign, print)	\
>  	DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
>  #define TRACE_EVENT_FN(name, proto, args, struct,		\
> diff --git a/include/trace/events/sched.h b/include/trace/events/sched.h
> index b48f1ad..0ee1eb6 100644
> --- a/include/trace/events/sched.h
> +++ b/include/trace/events/sched.h
> @@ -149,7 +149,7 @@ TRACE_EVENT(sched_wakeup_new,
>   * (NOTE: the 'rq' argument is not used by generic trace events,
>   *        but used by the latency tracer plugin. )
>   */
> -TRACE_EVENT(sched_switch,
> +TRACE_EVENT_API(sched_switch,
>  
>  	TP_PROTO(struct rq *rq, struct task_struct *prev,
>  		 struct task_struct *next),
> 
> 

Re: TRACE_EVENT_ABI ?

[Arjan van de Ven]

On Mon, 21 Sep 2009 12:46:07 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Mon, 2009-09-21 at 15:28 +0200, Arjan van de Ven wrote:
> > On Mon, 21 Sep 2009 09:26:41 -0400 (EDT)
> > Steven Rostedt <rostedt@goodmis.org> wrote:
> > 
> > > 
> > > [ Resending due to wrong SMTP server ]
> > > 
> > > 
> > > On Mon, 21 Sep 2009, Arjan van de Ven wrote:
> > > 
> > > > Hi,
> > > > 
> > > > Ingo suggested to make a TRACE_EVENT_ABI, which is equivalent in
> > > > implementation to TRACE_EVENT, except that it signals a stable
> > > > interface.
> > > > 
> > > > In trying to implement this I'm running into a wall where
> > > > TRACE_EVENT keeps being defined in many different ways all over
> > > > the place, making this a really nasty hack to make it just an
> > > > alias.
> > > > 
> > > > Do you have any clever ideas on how to make this an alias
> > > > without fouling up the whole tracing system?
> > > > 
> > > 
> > > I don't know what you've done so far, but can't you simply in
> > > tracepoint.h define:
> > > 
> > > #define TRACE_EVENT_API(a,b,c,d,e) TRACE_EVENT(a,b,c,d,e)
> > 
> > unfortunately, due to the preprocessor trickery around TRACE_EVENT
> > this completely barfs.
> 
> I forgot to tell you about PARAMS.
> 
> This patch compiled for me;
> 

your magic skills exceed mine ;-)

mind sending me a signoff for this ?



-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: TRACE_EVENT_ABI ?

[Steven Rostedt]

On Mon, 2009-09-21 at 20:00 +0200, Arjan van de Ven wrote:
> On Mon, 21 Sep 2009 12:46:07 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > On Mon, 2009-09-21 at 15:28 +0200, Arjan van de Ven wrote:

> > This patch compiled for me;
> > 
> 
> your magic skills exceed mine ;-)
> 
> mind sending me a signoff for this ?

Sure,

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

[patch] introduce TRACE_EVENT_ABI (was Re: TRACE_EVENT_ABI ?)

[Arjan van de Ven]

On Mon, 21 Sep 2009 17:20:31 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Mon, 2009-09-21 at 20:00 +0200, Arjan van de Ven wrote:
> > On Mon, 21 Sep 2009 12:46:07 -0400
> > Steven Rostedt <rostedt@goodmis.org> wrote:
> > 
> > > On Mon, 2009-09-21 at 15:28 +0200, Arjan van de Ven wrote:
> 
> > > This patch compiled for me;
> > > 
> > 
> > your magic skills exceed mine ;-)
> > 
> > mind sending me a signoff for this ?
> 
> Sure,
> 
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>


>From e2c647ae2d3ddc25b804f0419956caf40d89c606 Mon Sep 17 00:00:00 2001
From: Steven Rostedt <rostedt@goodmis.org>
Date: Mon, 21 Sep 2009 20:14:53 +0200
Subject: [PATCH] trace: introduce TRACE_EVENT_ABI

Some trace events are suitable to form a stable userspace ABI;
this patch introduces infrastructure to mark them as such,
and marks the first few tracepoints this way

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 include/linux/tracepoint.h   |    4 ++++
 include/trace/events/power.h |    6 +++---
 include/trace/events/sched.h |    4 ++--
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 63a3f7a..4cb454f 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -280,6 +280,10 @@ static inline void tracepoint_synchronize_unregister(void)
  * TRACE_EVENT_FN to perform any (un)registration work.
  */
 
+#define TRACE_EVENT_ABI(name, proto, args, tstruct, assign, print)	\
+	TRACE_EVENT(name, PARAMS(proto), PARAMS(args),			\
+		    PARAMS(tstruct), PARAMS(assign), PARAMS(print))
+
 #define TRACE_EVENT(name, proto, args, struct, assign, print)	\
 	DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
 #define TRACE_EVENT_FN(name, proto, args, struct,		\
diff --git a/include/trace/events/power.h b/include/trace/events/power.h
index ea6d579..7f0e86e 100644
--- a/include/trace/events/power.h
+++ b/include/trace/events/power.h
@@ -18,7 +18,7 @@ enum {
 
 
 
-TRACE_EVENT(power_start,
+TRACE_EVENT_ABI(power_start,
 
 	TP_PROTO(unsigned int type, unsigned int state),
 
@@ -37,7 +37,7 @@ TRACE_EVENT(power_start,
 	TP_printk("type=%lu state=%lu", (unsigned long)__entry->type, (unsigned long)__entry->state)
 );
 
-TRACE_EVENT(power_end,
+TRACE_EVENT_ABI(power_end,
 
 	TP_PROTO(int dummy),
 
@@ -56,7 +56,7 @@ TRACE_EVENT(power_end,
 );
 
 
-TRACE_EVENT(power_frequency,
+TRACE_EVENT_ABI(power_frequency,
 
 	TP_PROTO(unsigned int type, unsigned int state),
 
diff --git a/include/trace/events/sched.h b/include/trace/events/sched.h
index 4069c43..93a3e17 100644
--- a/include/trace/events/sched.h
+++ b/include/trace/events/sched.h
@@ -83,7 +83,7 @@ TRACE_EVENT(sched_wait_task,
  * (NOTE: the 'rq' argument is not used by generic trace events,
  *        but used by the latency tracer plugin. )
  */
-TRACE_EVENT(sched_wakeup,
+TRACE_EVENT_ABI(sched_wakeup,
 
 	TP_PROTO(struct rq *rq, struct task_struct *p, int success),
 
@@ -149,7 +149,7 @@ TRACE_EVENT(sched_wakeup_new,
  * (NOTE: the 'rq' argument is not used by generic trace events,
  *        but used by the latency tracer plugin. )
  */
-TRACE_EVENT(sched_switch,
+TRACE_EVENT_ABI(sched_switch,
 
 	TP_PROTO(struct rq *rq, struct task_struct *prev,
 		 struct task_struct *next),
-- 
1.6.0.6



-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [patch] introduce TRACE_EVENT_ABI (was Re: TRACE_EVENT_ABI ?)

[Frédéric Weisbecker]

2009/9/23, Arjan van de Ven <arjan@infradead.org>:
> From e2c647ae2d3ddc25b804f0419956caf40d89c606 Mon Sep 17 00:00:00 2001
> From: Steven Rostedt <rostedt@goodmis.org>
> Date: Mon, 21 Sep 2009 20:14:53 +0200
> Subject: [PATCH] trace: introduce TRACE_EVENT_ABI
>
> Some trace events are suitable to form a stable userspace ABI;
> this patch introduces infrastructure to mark them as such,
> and marks the first few tracepoints this way
>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
> ---
>  include/linux/tracepoint.h   |    4 ++++
>  include/trace/events/power.h |    6 +++---
>  include/trace/events/sched.h |    4 ++--
>  3 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> index 63a3f7a..4cb454f 100644
> --- a/include/linux/tracepoint.h
> +++ b/include/linux/tracepoint.h
> @@ -280,6 +280,10 @@ static inline void tracepoint_synchronize_unregister(void)
>  * TRACE_EVENT_FN to perform any (un)registration work.
>  */
>
> +#define TRACE_EVENT_ABI(name, proto, args, tstruct, assign, print)     \
> +       TRACE_EVENT(name, PARAMS(proto), PARAMS(args),                  \
> +                   PARAMS(tstruct), PARAMS(assign), PARAMS(print))
> +


You may probably need to undefine it from trace/define_trace.h
once every cpp magic have been done (ie: once trace/ftrace.h
have been included.)

Because some files include several tracepoint/trace event headers,
then linux/tracepoint.h gets included several times and eventually
TRACE_EVENT_ABI() would be redefined (and then crash).

That's why we have:

#ifdef CONFIG_EVENT_TRACING
#include <trace/ftrace.h>
#endif

#undef TRACE_EVENT
#undef TRACE_EVENT_FN
#undef TRACE_HEADER_MULTI_READ

in define_trace.h: to allow such re-inclusion of tracepoint.h
and avoid redefinitions of TRACE_EVENT and TRACE_EVENT_FN.

So you just need to add:

 #undef TRACE_EVENT
 #undef TRACE_EVENT_FN
+#undef TRACE_EVENT_ABI
 #undef TRACE_HEADER_MULTI_READ


The problem is visible if you build napi that has such
multiple tracepoint headers included in a single C file.

Thanks.