[UST PATCH] Tracepoints: add wrapper tracepoint() macro

[UST PATCH] Tracepoints: add wrapper tracepoint() macro

[Mathieu Desnoyers]

** Instrumentation API change **

Moving tracepoints from

trace_name(args)
register_trace_name(...)
unregister_trace_name(...)

to

tracepoint(name, args)
register_tracepoint(name, ...)
unregister_tracepoint(name, ...)

This will allow doing macro tricks at the "tracepoint()" macro expansion
site. This will be useful for integration with SystemTAP probes, which
needs to expand an inline assembly with constraints on the arguments.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Nils Carlson <nils.carlson@ericsson.com>
CC: Steven Rostedt <srostedt@redhat.com>
CC: Josh Stone <jistone@redhat.com>
---
 include/ust/tracepoint.h                          |   22 +++++++++++++++++-----
 include/ust/ust_trace.h                           |    6 +++---
 tests/hello/hello.c                               |    2 +-
 tests/hello/tp.c                                  |    2 +-
 tests/register_test/register_test.c               |    6 +++---
 tests/trace_event/trace_event_test.c              |    2 +-
 tests/tracepoint/benchmark/tracepoint_benchmark.c |    4 ++--
 tests/tracepoint/tracepoint_test.c                |   12 ++++++------
 8 files changed, 34 insertions(+), 22 deletions(-)

Index: ust/include/ust/tracepoint.h
===================================================================
--- ust.orig/include/ust/tracepoint.h
+++ ust/include/ust/tracepoint.h
@@ -49,6 +49,18 @@ struct tracepoint {
 #define TP_PROTO(args...)	args
 #define TP_ARGS(args...)	args
 
+/*
+ * Tracepoints should be added to the instrumented code using the
+ * "tracepoint()" macro.
+ */
+#define tracepoint(name, args...)	__trace_##name(args)
+
+#define register_tracepoint(name, probe, data)			\
+		__register_trace_##name(probe, data)
+
+#define unregister_tracepoint(name, probe, data)		\
+		__unregister_trace_##name(probe, data)
+
 #define CONFIG_TRACEPOINTS
 #ifdef CONFIG_TRACEPOINTS
 
@@ -99,7 +111,7 @@ struct tracepoint {
  */
 #define __DECLARE_TRACE(name, proto, args, data_proto, data_args)	\
 	extern struct tracepoint __tracepoint_##name;			\
-	static inline void trace_##name(proto)				\
+	static inline void __trace_##name(proto)			\
 	{								\
 		__CHECK_TRACE(name, 0, TP_PROTO(data_proto),		\
 			      TP_ARGS(data_args));			\
@@ -110,14 +122,14 @@ struct tracepoint {
 			      TP_ARGS(data_args));			\
 	}								\
 	static inline int						\
-	register_trace_##name(void (*probe)(data_proto), void *data)	\
+	__register_trace_##name(void (*probe)(data_proto), void *data)	\
 	{								\
 		return tracepoint_probe_register(#name, (void *)probe,	\
 						 data);			\
 									\
 	}								\
 	static inline int						\
-	unregister_trace_##name(void (*probe)(data_proto), void *data)	\
+	__unregister_trace_##name(void (*probe)(data_proto), void *data)\
 	{								\
 		return tracepoint_probe_unregister(#name, (void *)probe, \
 						   data);		\
@@ -145,11 +157,11 @@ extern void tracepoint_update_probe_rang
 	{ }								\
 	static inline void _trace_##name(proto)				\
 	{ }								\
-	static inline int register_trace_##name(void (*probe)(proto), void *data)	\
+	static inline int __register_trace_##name(void (*probe)(proto), void *data)	\
 	{								\
 		return -ENOSYS;						\
 	}								\
-	static inline int unregister_trace_##name(void (*probe)(proto), void *data)	\
+	static inline int __unregister_trace_##name(void (*probe)(proto), void *data)	\
 	{								\
 		return -ENOSYS;						\
 	}
Index: ust/include/ust/ust_trace.h
===================================================================
--- ust.orig/include/ust/ust_trace.h
+++ ust/include/ust/ust_trace.h
@@ -70,11 +70,11 @@
 	}								\
 	static inline int register_event_##name(void *data)		\
 	{								\
-		return register_trace_##name(trace_printf_##name, data); \
+		return register_tracepoint(name, trace_printf_##name, data); \
 	}								\
 	static inline int unregister_event_##name(void *data)		\
 	{								\
-		return unregister_trace_##name(trace_printf_##name, data); \
+		return unregister_tracepoint(name, trace_printf_##name, data); \
 	}								\
 	struct trace_event __event_##name = {				\
 		__tpstrtab_##name,					\
@@ -88,7 +88,7 @@
 	static void __attribute__((constructor)) init_##name()		\
 	{								\
 		void *dummy = NULL;					\
-		register_trace_##name(trace_printf_##name, dummy);	\
+		register_tracepoint(name, trace_printf_##name, dummy);	\
 	}
 
 
Index: ust/tests/hello/hello.c
===================================================================
--- ust.orig/tests/hello/hello.c
+++ ust/tests/hello/hello.c
@@ -73,7 +73,7 @@ int main()
 	for(i=0; i<50; i++) {
 		trace_mark(bar, "str %s", "FOOBAZ");
 		trace_mark(bar2, "number1 %d number2 %d", 53, 9800);
-		trace_hello_tptest(i);
+		tracepoint(hello_tptest, i);
 		usleep(100000);
 	}
 
Index: ust/tests/hello/tp.c
===================================================================
--- ust.orig/tests/hello/tp.c
+++ ust/tests/hello/tp.c
@@ -40,5 +40,5 @@ void tptest_probe(void *data, int anint)
 static void __attribute__((constructor)) init()
 {
 	DBG("connecting tracepoint...\n");
-	register_trace_hello_tptest(tptest_probe, &hello_struct);
+	register_tracepoint(hello_tptest, tptest_probe, &hello_struct);
 }
Index: ust/tests/register_test/register_test.c
===================================================================
--- ust.orig/tests/register_test/register_test.c
+++ ust/tests/register_test/register_test.c
@@ -65,13 +65,13 @@ static void * register_thread_main(void
 	}
 
 	for (i=0; i<1000; i++) {
-		while (!register_trace_hello_tptest(tptest_probe,
+		while (!register_tracepoint(hello_tptest, tptest_probe,
 						    &hello[j%HELLO_LENGTH])) {
 			usleep(10);
 			j++;
 		}
 		printf("Registered all\n");
-		while (!unregister_trace_hello_tptest(tptest_probe,
+		while (!unregister_tracepoint(hello_tptest, tptest_probe,
 						      &hello[j%HELLO_LENGTH])) {
 			usleep(10);
 			j++;
@@ -89,7 +89,7 @@ int main(int argc, char **argv)
 
 	pthread_create(&register_thread, NULL, register_thread_main, NULL);
 	for(i=0; i<1000000; i++) {
-		trace_hello_tptest(i);
+		tracepoint(hello_tptest, i);
 	}
 
 	return 0;
Index: ust/tests/trace_event/trace_event_test.c
===================================================================
--- ust.orig/tests/trace_event/trace_event_test.c
+++ ust/tests/trace_event/trace_event_test.c
@@ -25,7 +25,7 @@ int main(int argc, char * argv[])
 	static unsigned long time, i;
 	for (i=0; i<10; i++) {
 		time=trace_clock_read64();
-		trace_test(time, i);
+		tracepoint(test, time, i);
 	}
 	return 0;
 }
Index: ust/tests/tracepoint/benchmark/tracepoint_benchmark.c
===================================================================
--- ust.orig/tests/tracepoint/benchmark/tracepoint_benchmark.c
+++ ust/tests/tracepoint/benchmark/tracepoint_benchmark.c
@@ -49,12 +49,12 @@ void tp_probe(void *data, unsigned int p
 
 static void __attribute__((constructor)) init()
 {
-	register_trace_ust_event(tp_probe, NULL);
+	register_tracepoint(ust_event, tp_probe, NULL);
 }
 
 void single_trace(unsigned int v)
 {
-	trace_ust_event(v);
+	tracepoint(ust_event, v);
 }
 
 void do_trace(void)
Index: ust/tests/tracepoint/tracepoint_test.c
===================================================================
--- ust.orig/tests/tracepoint/tracepoint_test.c
+++ ust/tests/tracepoint/tracepoint_test.c
@@ -90,18 +90,18 @@ void tp_probe(void *data, unsigned int p
 
 static void __attribute__((constructor)) init()
 {
-	register_trace_ust_event(tp_probe, NULL);
-	register_trace_ust_event(tp_probe2, NULL);
-	register_trace_ust_event(tp_probe3, &msg_probe3);
-	register_trace_ust_event2(tp_probe4, NULL);
+	register_tracepoint(ust_event, tp_probe, NULL);
+	register_tracepoint(ust_event, tp_probe2, NULL);
+	register_tracepoint(ust_event, tp_probe3, &msg_probe3);
+	register_tracepoint(ust_event2, tp_probe4, NULL);
 }
 
 int main(int argc, char **argv) {
 	unsigned int v = 42;
 	/* Tracepoint 1 : ust_event */
-	trace_ust_event(v);
+	tracepoint(ust_event, v);
 	/* Tracepoint 2 : ust_event2 */
-	trace_ust_event2(v);
+	tracepoint(ust_event2, v);
 
 	return 0;
 }

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [UST PATCH] Tracepoints: add wrapper tracepoint() macro

[Nils Carlson]

Hi,

Again, looks good, but maybe we should prefix with ust_ ?

I like the macro approach.

/Nils

On Mon, 11 Apr 2011, Mathieu Desnoyers wrote:

> ** Instrumentation API change **
>
> Moving tracepoints from
>
> trace_name(args)
> register_trace_name(...)
> unregister_trace_name(...)
>
> to
>
> tracepoint(name, args)
> register_tracepoint(name, ...)
> unregister_tracepoint(name, ...)
>
> This will allow doing macro tricks at the "tracepoint()" macro expansion
> site. This will be useful for integration with SystemTAP probes, which
> needs to expand an inline assembly with constraints on the arguments.
>
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> CC: Nils Carlson <nils.carlson@ericsson.com>
> CC: Steven Rostedt <srostedt@redhat.com>
> CC: Josh Stone <jistone@redhat.com>
> ---
> include/ust/tracepoint.h                          |   22 +++++++++++++++++-----
> include/ust/ust_trace.h                           |    6 +++---
> tests/hello/hello.c                               |    2 +-
> tests/hello/tp.c                                  |    2 +-
> tests/register_test/register_test.c               |    6 +++---
> tests/trace_event/trace_event_test.c              |    2 +-
> tests/tracepoint/benchmark/tracepoint_benchmark.c |    4 ++--
> tests/tracepoint/tracepoint_test.c                |   12 ++++++------
> 8 files changed, 34 insertions(+), 22 deletions(-)
>
> Index: ust/include/ust/tracepoint.h
> ===================================================================
> --- ust.orig/include/ust/tracepoint.h
> +++ ust/include/ust/tracepoint.h
> @@ -49,6 +49,18 @@ struct tracepoint {
> #define TP_PROTO(args...)	args
> #define TP_ARGS(args...)	args
>
> +/*
> + * Tracepoints should be added to the instrumented code using the
> + * "tracepoint()" macro.
> + */
> +#define tracepoint(name, args...)	__trace_##name(args)
> +
> +#define register_tracepoint(name, probe, data)			\
> +		__register_trace_##name(probe, data)
> +
> +#define unregister_tracepoint(name, probe, data)		\
> +		__unregister_trace_##name(probe, data)
> +
> #define CONFIG_TRACEPOINTS
> #ifdef CONFIG_TRACEPOINTS
>
> @@ -99,7 +111,7 @@ struct tracepoint {
>  */
> #define __DECLARE_TRACE(name, proto, args, data_proto, data_args)	\
> 	extern struct tracepoint __tracepoint_##name;			\
> -	static inline void trace_##name(proto)				\
> +	static inline void __trace_##name(proto)			\
> 	{								\
> 		__CHECK_TRACE(name, 0, TP_PROTO(data_proto),		\
> 			      TP_ARGS(data_args));			\
> @@ -110,14 +122,14 @@ struct tracepoint {
> 			      TP_ARGS(data_args));			\
> 	}								\
> 	static inline int						\
> -	register_trace_##name(void (*probe)(data_proto), void *data)	\
> +	__register_trace_##name(void (*probe)(data_proto), void *data)	\
> 	{								\
> 		return tracepoint_probe_register(#name, (void *)probe,	\
> 						 data);			\
> 									\
> 	}								\
> 	static inline int						\
> -	unregister_trace_##name(void (*probe)(data_proto), void *data)	\
> +	__unregister_trace_##name(void (*probe)(data_proto), void *data)\
> 	{								\
> 		return tracepoint_probe_unregister(#name, (void *)probe, \
> 						   data);		\
> @@ -145,11 +157,11 @@ extern void tracepoint_update_probe_rang
> 	{ }								\
> 	static inline void _trace_##name(proto)				\
> 	{ }								\
> -	static inline int register_trace_##name(void (*probe)(proto), void *data)	\
> +	static inline int __register_trace_##name(void (*probe)(proto), void *data)	\
> 	{								\
> 		return -ENOSYS;						\
> 	}								\
> -	static inline int unregister_trace_##name(void (*probe)(proto), void *data)	\
> +	static inline int __unregister_trace_##name(void (*probe)(proto), void *data)	\
> 	{								\
> 		return -ENOSYS;						\
> 	}
> Index: ust/include/ust/ust_trace.h
> ===================================================================
> --- ust.orig/include/ust/ust_trace.h
> +++ ust/include/ust/ust_trace.h
> @@ -70,11 +70,11 @@
> 	}								\
> 	static inline int register_event_##name(void *data)		\
> 	{								\
> -		return register_trace_##name(trace_printf_##name, data); \
> +		return register_tracepoint(name, trace_printf_##name, data); \
> 	}								\
> 	static inline int unregister_event_##name(void *data)		\
> 	{								\
> -		return unregister_trace_##name(trace_printf_##name, data); \
> +		return unregister_tracepoint(name, trace_printf_##name, data); \
> 	}								\
> 	struct trace_event __event_##name = {				\
> 		__tpstrtab_##name,					\
> @@ -88,7 +88,7 @@
> 	static void __attribute__((constructor)) init_##name()		\
> 	{								\
> 		void *dummy = NULL;					\
> -		register_trace_##name(trace_printf_##name, dummy);	\
> +		register_tracepoint(name, trace_printf_##name, dummy);	\
> 	}
>
>
> Index: ust/tests/hello/hello.c
> ===================================================================
> --- ust.orig/tests/hello/hello.c
> +++ ust/tests/hello/hello.c
> @@ -73,7 +73,7 @@ int main()
> 	for(i=0; i<50; i++) {
> 		trace_mark(bar, "str %s", "FOOBAZ");
> 		trace_mark(bar2, "number1 %d number2 %d", 53, 9800);
> -		trace_hello_tptest(i);
> +		tracepoint(hello_tptest, i);
> 		usleep(100000);
> 	}
>
> Index: ust/tests/hello/tp.c
> ===================================================================
> --- ust.orig/tests/hello/tp.c
> +++ ust/tests/hello/tp.c
> @@ -40,5 +40,5 @@ void tptest_probe(void *data, int anint)
> static void __attribute__((constructor)) init()
> {
> 	DBG("connecting tracepoint...\n");
> -	register_trace_hello_tptest(tptest_probe, &hello_struct);
> +	register_tracepoint(hello_tptest, tptest_probe, &hello_struct);
> }
> Index: ust/tests/register_test/register_test.c
> ===================================================================
> --- ust.orig/tests/register_test/register_test.c
> +++ ust/tests/register_test/register_test.c
> @@ -65,13 +65,13 @@ static void * register_thread_main(void
> 	}
>
> 	for (i=0; i<1000; i++) {
> -		while (!register_trace_hello_tptest(tptest_probe,
> +		while (!register_tracepoint(hello_tptest, tptest_probe,
> 						    &hello[j%HELLO_LENGTH])) {
> 			usleep(10);
> 			j++;
> 		}
> 		printf("Registered all\n");
> -		while (!unregister_trace_hello_tptest(tptest_probe,
> +		while (!unregister_tracepoint(hello_tptest, tptest_probe,
> 						      &hello[j%HELLO_LENGTH])) {
> 			usleep(10);
> 			j++;
> @@ -89,7 +89,7 @@ int main(int argc, char **argv)
>
> 	pthread_create(&register_thread, NULL, register_thread_main, NULL);
> 	for(i=0; i<1000000; i++) {
> -		trace_hello_tptest(i);
> +		tracepoint(hello_tptest, i);
> 	}
>
> 	return 0;
> Index: ust/tests/trace_event/trace_event_test.c
> ===================================================================
> --- ust.orig/tests/trace_event/trace_event_test.c
> +++ ust/tests/trace_event/trace_event_test.c
> @@ -25,7 +25,7 @@ int main(int argc, char * argv[])
> 	static unsigned long time, i;
> 	for (i=0; i<10; i++) {
> 		time=trace_clock_read64();
> -		trace_test(time, i);
> +		tracepoint(test, time, i);
> 	}
> 	return 0;
> }
> Index: ust/tests/tracepoint/benchmark/tracepoint_benchmark.c
> ===================================================================
> --- ust.orig/tests/tracepoint/benchmark/tracepoint_benchmark.c
> +++ ust/tests/tracepoint/benchmark/tracepoint_benchmark.c
> @@ -49,12 +49,12 @@ void tp_probe(void *data, unsigned int p
>
> static void __attribute__((constructor)) init()
> {
> -	register_trace_ust_event(tp_probe, NULL);
> +	register_tracepoint(ust_event, tp_probe, NULL);
> }
>
> void single_trace(unsigned int v)
> {
> -	trace_ust_event(v);
> +	tracepoint(ust_event, v);
> }
>
> void do_trace(void)
> Index: ust/tests/tracepoint/tracepoint_test.c
> ===================================================================
> --- ust.orig/tests/tracepoint/tracepoint_test.c
> +++ ust/tests/tracepoint/tracepoint_test.c
> @@ -90,18 +90,18 @@ void tp_probe(void *data, unsigned int p
>
> static void __attribute__((constructor)) init()
> {
> -	register_trace_ust_event(tp_probe, NULL);
> -	register_trace_ust_event(tp_probe2, NULL);
> -	register_trace_ust_event(tp_probe3, &msg_probe3);
> -	register_trace_ust_event2(tp_probe4, NULL);
> +	register_tracepoint(ust_event, tp_probe, NULL);
> +	register_tracepoint(ust_event, tp_probe2, NULL);
> +	register_tracepoint(ust_event, tp_probe3, &msg_probe3);
> +	register_tracepoint(ust_event2, tp_probe4, NULL);
> }
>
> int main(int argc, char **argv) {
> 	unsigned int v = 42;
> 	/* Tracepoint 1 : ust_event */
> -	trace_ust_event(v);
> +	tracepoint(ust_event, v);
> 	/* Tracepoint 2 : ust_event2 */
> -	trace_ust_event2(v);
> +	tracepoint(ust_event2, v);
>
> 	return 0;
> }
>
> -- 
> Mathieu Desnoyers
> Operating System Efficiency R&D Consultant
> EfficiOS Inc.
> http://www.efficios.com
>

Re: [UST PATCH] Tracepoints: add wrapper tracepoint() macro

[Mathieu Desnoyers]

* Nils Carlson (nils.carlson@ericsson.com) wrote:
> Hi,
>
> Again, looks good, but maybe we should prefix with ust_ ?

This is a very important question. The goal of this tracepoint
instrumentation is to replace the Dtrace instrumentation, which is
somewhat implementation-driven (fixed number of u32 values AFAIK) by
something more generic that allows a variable number of arguments.
Mapping from a generic "tracepoint()" macro to DTrace macros could then
be done in a compatibility header layer.

So although it makes sense to use a ust_ prefix for all the UST-specific
APIs, including e.g. probe registration to tracepoints, I'm really not
convinced that the prefix would be appropriate for the instrumentation
macros that will be called from the instrumented code.

tracepoint() seems less likely to clash with other namespaces than
"trace()", while still being generic enough.

I'm open to comments, and especially interested to know what the
SystemTAP team think about this issue.

> I like the macro approach.

Yes, this will give us much more flexibility than a static inline
function call in the end.

Thanks,

Mathieu

>
> /Nils
>
> On Mon, 11 Apr 2011, Mathieu Desnoyers wrote:
>
>> ** Instrumentation API change **
>>
>> Moving tracepoints from
>>
>> trace_name(args)
>> register_trace_name(...)
>> unregister_trace_name(...)
>>
>> to
>>
>> tracepoint(name, args)
>> register_tracepoint(name, ...)
>> unregister_tracepoint(name, ...)
>>
>> This will allow doing macro tricks at the "tracepoint()" macro expansion
>> site. This will be useful for integration with SystemTAP probes, which
>> needs to expand an inline assembly with constraints on the arguments.
>>
>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> CC: Nils Carlson <nils.carlson@ericsson.com>
>> CC: Steven Rostedt <srostedt@redhat.com>
>> CC: Josh Stone <jistone@redhat.com>
>> ---
>> include/ust/tracepoint.h                          |   22 +++++++++++++++++-----
>> include/ust/ust_trace.h                           |    6 +++---
>> tests/hello/hello.c                               |    2 +-
>> tests/hello/tp.c                                  |    2 +-
>> tests/register_test/register_test.c               |    6 +++---
>> tests/trace_event/trace_event_test.c              |    2 +-
>> tests/tracepoint/benchmark/tracepoint_benchmark.c |    4 ++--
>> tests/tracepoint/tracepoint_test.c                |   12 ++++++------
>> 8 files changed, 34 insertions(+), 22 deletions(-)
>>
>> Index: ust/include/ust/tracepoint.h
>> ===================================================================
>> --- ust.orig/include/ust/tracepoint.h
>> +++ ust/include/ust/tracepoint.h
>> @@ -49,6 +49,18 @@ struct tracepoint {
>> #define TP_PROTO(args...)	args
>> #define TP_ARGS(args...)	args
>>
>> +/*
>> + * Tracepoints should be added to the instrumented code using the
>> + * "tracepoint()" macro.
>> + */
>> +#define tracepoint(name, args...)	__trace_##name(args)
>> +
>> +#define register_tracepoint(name, probe, data)			\
>> +		__register_trace_##name(probe, data)
>> +
>> +#define unregister_tracepoint(name, probe, data)		\
>> +		__unregister_trace_##name(probe, data)
>> +
>> #define CONFIG_TRACEPOINTS
>> #ifdef CONFIG_TRACEPOINTS
>>
>> @@ -99,7 +111,7 @@ struct tracepoint {
>>  */
>> #define __DECLARE_TRACE(name, proto, args, data_proto, data_args)	\
>> 	extern struct tracepoint __tracepoint_##name;			\
>> -	static inline void trace_##name(proto)				\
>> +	static inline void __trace_##name(proto)			\
>> 	{								\
>> 		__CHECK_TRACE(name, 0, TP_PROTO(data_proto),		\
>> 			      TP_ARGS(data_args));			\
>> @@ -110,14 +122,14 @@ struct tracepoint {
>> 			      TP_ARGS(data_args));			\
>> 	}								\
>> 	static inline int						\
>> -	register_trace_##name(void (*probe)(data_proto), void *data)	\
>> +	__register_trace_##name(void (*probe)(data_proto), void *data)	\
>> 	{								\
>> 		return tracepoint_probe_register(#name, (void *)probe,	\
>> 						 data);			\
>> 									\
>> 	}								\
>> 	static inline int						\
>> -	unregister_trace_##name(void (*probe)(data_proto), void *data)	\
>> +	__unregister_trace_##name(void (*probe)(data_proto), void *data)\
>> 	{								\
>> 		return tracepoint_probe_unregister(#name, (void *)probe, \
>> 						   data);		\
>> @@ -145,11 +157,11 @@ extern void tracepoint_update_probe_rang
>> 	{ }								\
>> 	static inline void _trace_##name(proto)				\
>> 	{ }								\
>> -	static inline int register_trace_##name(void (*probe)(proto), void *data)	\
>> +	static inline int __register_trace_##name(void (*probe)(proto), void *data)	\
>> 	{								\
>> 		return -ENOSYS;						\
>> 	}								\
>> -	static inline int unregister_trace_##name(void (*probe)(proto), void *data)	\
>> +	static inline int __unregister_trace_##name(void (*probe)(proto), void *data)	\
>> 	{								\
>> 		return -ENOSYS;						\
>> 	}
>> Index: ust/include/ust/ust_trace.h
>> ===================================================================
>> --- ust.orig/include/ust/ust_trace.h
>> +++ ust/include/ust/ust_trace.h
>> @@ -70,11 +70,11 @@
>> 	}								\
>> 	static inline int register_event_##name(void *data)		\
>> 	{								\
>> -		return register_trace_##name(trace_printf_##name, data); \
>> +		return register_tracepoint(name, trace_printf_##name, data); \
>> 	}								\
>> 	static inline int unregister_event_##name(void *data)		\
>> 	{								\
>> -		return unregister_trace_##name(trace_printf_##name, data); \
>> +		return unregister_tracepoint(name, trace_printf_##name, data); \
>> 	}								\
>> 	struct trace_event __event_##name = {				\
>> 		__tpstrtab_##name,					\
>> @@ -88,7 +88,7 @@
>> 	static void __attribute__((constructor)) init_##name()		\
>> 	{								\
>> 		void *dummy = NULL;					\
>> -		register_trace_##name(trace_printf_##name, dummy);	\
>> +		register_tracepoint(name, trace_printf_##name, dummy);	\
>> 	}
>>
>>
>> Index: ust/tests/hello/hello.c
>> ===================================================================
>> --- ust.orig/tests/hello/hello.c
>> +++ ust/tests/hello/hello.c
>> @@ -73,7 +73,7 @@ int main()
>> 	for(i=0; i<50; i++) {
>> 		trace_mark(bar, "str %s", "FOOBAZ");
>> 		trace_mark(bar2, "number1 %d number2 %d", 53, 9800);
>> -		trace_hello_tptest(i);
>> +		tracepoint(hello_tptest, i);
>> 		usleep(100000);
>> 	}
>>
>> Index: ust/tests/hello/tp.c
>> ===================================================================
>> --- ust.orig/tests/hello/tp.c
>> +++ ust/tests/hello/tp.c
>> @@ -40,5 +40,5 @@ void tptest_probe(void *data, int anint)
>> static void __attribute__((constructor)) init()
>> {
>> 	DBG("connecting tracepoint...\n");
>> -	register_trace_hello_tptest(tptest_probe, &hello_struct);
>> +	register_tracepoint(hello_tptest, tptest_probe, &hello_struct);
>> }
>> Index: ust/tests/register_test/register_test.c
>> ===================================================================
>> --- ust.orig/tests/register_test/register_test.c
>> +++ ust/tests/register_test/register_test.c
>> @@ -65,13 +65,13 @@ static void * register_thread_main(void
>> 	}
>>
>> 	for (i=0; i<1000; i++) {
>> -		while (!register_trace_hello_tptest(tptest_probe,
>> +		while (!register_tracepoint(hello_tptest, tptest_probe,
>> 						    &hello[j%HELLO_LENGTH])) {
>> 			usleep(10);
>> 			j++;
>> 		}
>> 		printf("Registered all\n");
>> -		while (!unregister_trace_hello_tptest(tptest_probe,
>> +		while (!unregister_tracepoint(hello_tptest, tptest_probe,
>> 						      &hello[j%HELLO_LENGTH])) {
>> 			usleep(10);
>> 			j++;
>> @@ -89,7 +89,7 @@ int main(int argc, char **argv)
>>
>> 	pthread_create(&register_thread, NULL, register_thread_main, NULL);
>> 	for(i=0; i<1000000; i++) {
>> -		trace_hello_tptest(i);
>> +		tracepoint(hello_tptest, i);
>> 	}
>>
>> 	return 0;
>> Index: ust/tests/trace_event/trace_event_test.c
>> ===================================================================
>> --- ust.orig/tests/trace_event/trace_event_test.c
>> +++ ust/tests/trace_event/trace_event_test.c
>> @@ -25,7 +25,7 @@ int main(int argc, char * argv[])
>> 	static unsigned long time, i;
>> 	for (i=0; i<10; i++) {
>> 		time=trace_clock_read64();
>> -		trace_test(time, i);
>> +		tracepoint(test, time, i);
>> 	}
>> 	return 0;
>> }
>> Index: ust/tests/tracepoint/benchmark/tracepoint_benchmark.c
>> ===================================================================
>> --- ust.orig/tests/tracepoint/benchmark/tracepoint_benchmark.c
>> +++ ust/tests/tracepoint/benchmark/tracepoint_benchmark.c
>> @@ -49,12 +49,12 @@ void tp_probe(void *data, unsigned int p
>>
>> static void __attribute__((constructor)) init()
>> {
>> -	register_trace_ust_event(tp_probe, NULL);
>> +	register_tracepoint(ust_event, tp_probe, NULL);
>> }
>>
>> void single_trace(unsigned int v)
>> {
>> -	trace_ust_event(v);
>> +	tracepoint(ust_event, v);
>> }
>>
>> void do_trace(void)
>> Index: ust/tests/tracepoint/tracepoint_test.c
>> ===================================================================
>> --- ust.orig/tests/tracepoint/tracepoint_test.c
>> +++ ust/tests/tracepoint/tracepoint_test.c
>> @@ -90,18 +90,18 @@ void tp_probe(void *data, unsigned int p
>>
>> static void __attribute__((constructor)) init()
>> {
>> -	register_trace_ust_event(tp_probe, NULL);
>> -	register_trace_ust_event(tp_probe2, NULL);
>> -	register_trace_ust_event(tp_probe3, &msg_probe3);
>> -	register_trace_ust_event2(tp_probe4, NULL);
>> +	register_tracepoint(ust_event, tp_probe, NULL);
>> +	register_tracepoint(ust_event, tp_probe2, NULL);
>> +	register_tracepoint(ust_event, tp_probe3, &msg_probe3);
>> +	register_tracepoint(ust_event2, tp_probe4, NULL);
>> }
>>
>> int main(int argc, char **argv) {
>> 	unsigned int v = 42;
>> 	/* Tracepoint 1 : ust_event */
>> -	trace_ust_event(v);
>> +	tracepoint(ust_event, v);
>> 	/* Tracepoint 2 : ust_event2 */
>> -	trace_ust_event2(v);
>> +	tracepoint(ust_event2, v);
>>
>> 	return 0;
>> }
>>
>> -- 
>> Mathieu Desnoyers
>> Operating System Efficiency R&D Consultant
>> EfficiOS Inc.
>> http://www.efficios.com
>>

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [UST PATCH] Tracepoints: add wrapper tracepoint() macro

[Josh Stone]

On 04/11/2011 07:14 AM, Mathieu Desnoyers wrote:
> * Nils Carlson (nils.carlson@ericsson.com) wrote:
>> Hi,
>>
>> Again, looks good, but maybe we should prefix with ust_ ?
> 
> This is a very important question. The goal of this tracepoint
> instrumentation is to replace the Dtrace instrumentation, which is
> somewhat implementation-driven (fixed number of u32 values AFAIK) by
> something more generic that allows a variable number of arguments.
> Mapping from a generic "tracepoint()" macro to DTrace macros could then
> be done in a compatibility header layer.

I'm not certain about DTrace's implementation, but I doubt that they
limit the args so much.  In SystemTap, we permit 0-10 pointers and
integers up to 64-bit, with size and signed-ness maintained.

> So although it makes sense to use a ust_ prefix for all the UST-specific
> APIs, including e.g. probe registration to tracepoints, I'm really not
> convinced that the prefix would be appropriate for the instrumentation
> macros that will be called from the instrumented code.
> 
> tracepoint() seems less likely to clash with other namespaces than
> "trace()", while still being generic enough.
> 
> I'm open to comments, and especially interested to know what the
> SystemTAP team think about this issue.

I think in most projects where we have added SDT, they use wrapper
macros in their own namespace anyway, e.g. PYTHON_FUNCTION_ENTRY(),
perhaps so they can more easily be defined empty on other platforms.  So
IMO it really only matters that your definitions don't conflict, and the
rest will probably be abstracted.

Josh