[RFC][PATCH] tracing: Replace '-' with '_' in event system names

[RFC][PATCH] tracing: Replace '-' with '_' in event system names

[Steven Rostedt]

There's a change I want to make to the tracing infrastructure that may
require TRACE_SYSTEM be a valid variable name. As '-' can not be used
in a variable, and I found only three cases that it is a TRACE_SYSTEM
name, I want to ask those that are responsible if it is OK to change
them?

The three systems are:

 #define TRACE_SYSTEM kvm-s390
 #define TRACE_SYSTEM xhci-hcd
 #define TRACE_SYSTEM intel-sst


I want to replace the '-' with '_'. But this will have a user space
visible affect, which is why I'm inquiring with you. If this will break
any scripts or annoy any users that you know of, I'll need to make a
work around (which would not be hard to do). But if I don't need to do
that, I prefer to just use TRACE_SYSTEM as is.

I'll just do the work around if there is any user space tool that you
know of that will break with this change.

The effect is that the directories in /sys/kernel/debug/tracing/events/
will be different. That is,

 /sys/kernel/debug/tracing/events/kvm-s390/

will become

 /sys/kernel/debug/tracing/events/kvm_s390/

And the same for the other two. Is this a problem?

Thanks!

-- Steve

Tentatively-signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/s390/kvm/trace-s390.h       | 2 +-
 drivers/usb/host/xhci-trace.h    | 2 +-
 include/trace/events/intel-sst.h | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/s390/kvm/trace-s390.h b/arch/s390/kvm/trace-s390.h
index 653a7ec09ef5..bf02effdf535 100644
--- a/arch/s390/kvm/trace-s390.h
+++ b/arch/s390/kvm/trace-s390.h
@@ -4,7 +4,7 @@
 #include <linux/tracepoint.h>
 
 #undef TRACE_SYSTEM
-#define TRACE_SYSTEM kvm-s390
+#define TRACE_SYSTEM kvm_s390
 #define TRACE_INCLUDE_PATH .
 #undef TRACE_INCLUDE_FILE
 #define TRACE_INCLUDE_FILE trace-s390
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index dde3959b7a33..b610374b6f36 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -12,7 +12,7 @@
  */
 
 #undef TRACE_SYSTEM
-#define TRACE_SYSTEM xhci-hcd
+#define TRACE_SYSTEM xhci_hcd
 
 #if !defined(__XHCI_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
 #define __XHCI_TRACE_H
diff --git a/include/trace/events/intel-sst.h b/include/trace/events/intel-sst.h
index 76c72d3f1902..11bb4ca0fac7 100644
--- a/include/trace/events/intel-sst.h
+++ b/include/trace/events/intel-sst.h
@@ -1,5 +1,5 @@
 #undef TRACE_SYSTEM
-#define TRACE_SYSTEM intel-sst
+#define TRACE_SYSTEM intel_sst
 
 #if !defined(_TRACE_INTEL_SST_H) || defined(TRACE_HEADER_MULTI_READ)
 #define _TRACE_INTEL_SST_H

Re: [RFC][PATCH] tracing: Replace '-' with '_' in event system names

[Xenia Ragiadakou]

On 01/04/2015 06:18 πμ, Steven Rostedt wrote:
> There's a change I want to make to the tracing infrastructure that may
> require TRACE_SYSTEM be a valid variable name. As '-' can not be used
> in a variable, and I found only three cases that it is a TRACE_SYSTEM
> name, I want to ask those that are responsible if it is OK to change
> them?
>
> The three systems are:
>
>   #define TRACE_SYSTEM kvm-s390
>   #define TRACE_SYSTEM xhci-hcd
>   #define TRACE_SYSTEM intel-sst
>
>
> I want to replace the '-' with '_'. But this will have a user space
> visible affect, which is why I'm inquiring with you. If this will break
> any scripts or annoy any users that you know of, I'll need to make a
> work around (which would not be hard to do). But if I don't need to do
> that, I prefer to just use TRACE_SYSTEM as is.
>
> I'll just do the work around if there is any user space tool that you
> know of that will break with this change.
>
> The effect is that the directories in /sys/kernel/debug/tracing/events/
> will be different. That is,
>
>   /sys/kernel/debug/tracing/events/kvm-s390/
>
> will become
>
>   /sys/kernel/debug/tracing/events/kvm_s390/
>
> And the same for the other two. Is this a problem?
>
> Thanks!
>
> -- Steve
>
> Tentatively-signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
>   arch/s390/kvm/trace-s390.h       | 2 +-
>   drivers/usb/host/xhci-trace.h    | 2 +-
>   include/trace/events/intel-sst.h | 2 +-
>   3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/s390/kvm/trace-s390.h b/arch/s390/kvm/trace-s390.h
> index 653a7ec09ef5..bf02effdf535 100644
> --- a/arch/s390/kvm/trace-s390.h
> +++ b/arch/s390/kvm/trace-s390.h
> @@ -4,7 +4,7 @@
>   #include <linux/tracepoint.h>
>
>   #undef TRACE_SYSTEM
> -#define TRACE_SYSTEM kvm-s390
> +#define TRACE_SYSTEM kvm_s390
>   #define TRACE_INCLUDE_PATH .
>   #undef TRACE_INCLUDE_FILE
>   #define TRACE_INCLUDE_FILE trace-s390
> diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
> index dde3959b7a33..b610374b6f36 100644
> --- a/drivers/usb/host/xhci-trace.h
> +++ b/drivers/usb/host/xhci-trace.h
> @@ -12,7 +12,7 @@
>    */
>
>   #undef TRACE_SYSTEM
> -#define TRACE_SYSTEM xhci-hcd
> +#define TRACE_SYSTEM xhci_hcd
>
>   #if !defined(__XHCI_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
>   #define __XHCI_TRACE_H
> diff --git a/include/trace/events/intel-sst.h b/include/trace/events/intel-sst.h
> index 76c72d3f1902..11bb4ca0fac7 100644
> --- a/include/trace/events/intel-sst.h
> +++ b/include/trace/events/intel-sst.h
> @@ -1,5 +1,5 @@
>   #undef TRACE_SYSTEM
> -#define TRACE_SYSTEM intel-sst
> +#define TRACE_SYSTEM intel_sst
>
>   #if !defined(_TRACE_INTEL_SST_H) || defined(TRACE_HEADER_MULTI_READ)
>   #define _TRACE_INTEL_SST_H
>


The change affects the trace-cmd plugin for xhci, but I can fix that, no 
problem.

regards,
Xenia

Re: [RFC][PATCH] tracing: Replace '-' with '_' in event system names

[Cornelia Huck]

On Tue, 31 Mar 2015 23:18:02 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> There's a change I want to make to the tracing infrastructure that may
> require TRACE_SYSTEM be a valid variable name. As '-' can not be used
> in a variable, and I found only three cases that it is a TRACE_SYSTEM
> name, I want to ask those that are responsible if it is OK to change
> them?
> 
> The three systems are:
> 
>  #define TRACE_SYSTEM kvm-s390
>  #define TRACE_SYSTEM xhci-hcd
>  #define TRACE_SYSTEM intel-sst
> 
> 
> I want to replace the '-' with '_'. But this will have a user space
> visible affect, which is why I'm inquiring with you. If this will break
> any scripts or annoy any users that you know of, I'll need to make a
> work around (which would not be hard to do). But if I don't need to do
> that, I prefer to just use TRACE_SYSTEM as is.
> 
> I'll just do the work around if there is any user space tool that you
> know of that will break with this change.
> 
> The effect is that the directories in /sys/kernel/debug/tracing/events/
> will be different. That is,
> 
>  /sys/kernel/debug/tracing/events/kvm-s390/
> 
> will become
> 
>  /sys/kernel/debug/tracing/events/kvm_s390/
> 
> And the same for the other two. Is this a problem?

The only direct dependency for kvm-s390 I'm aware of is in
perf/tests/parse-events.c, added because parsing an event with '-' in
the name was broken.

Otherwise, I'm not aware of things that should break, excluding local
scripts and so on of course.

Re: [RFC][PATCH] tracing: Replace '-' with '_' in event system names

[Steven Rostedt]

On Wed, 1 Apr 2015 12:36:57 +0200
Cornelia Huck <cornelia.huck@de.ibm.com> wrote:


> The only direct dependency for kvm-s390 I'm aware of is in
> perf/tests/parse-events.c, added because parsing an event with '-' in
> the name was broken.
> 
> Otherwise, I'm not aware of things that should break, excluding local
> scripts and so on of course.

I don't like the fact that this changes what userspace sees. This is
the work around that I'm leaning towards:

-- Steve

diff --git a/arch/s390/kvm/trace-s390.h b/arch/s390/kvm/trace-s390.h
index 653a7ec09ef5..707a3e08639f 100644
--- a/arch/s390/kvm/trace-s390.h
+++ b/arch/s390/kvm/trace-s390.h
@@ -10,6 +10,13 @@
 #define TRACE_INCLUDE_FILE trace-s390
 
 /*
+ * The TRACE_SYSTEM_VAR defaults to TRACE_SYSTEM, but must be a
+ * legitimate C variable. It is not exported to user space.
+ */
+#undef TRACE_SYSTEM_VAR
+#define TRACE_SYSTEM_VAR trace_s390
+
+/*
  * Trace point for the creation of the kvm instance.
  */
 TRACE_EVENT(kvm_s390_create_vm,
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index dde3959b7a33..f44be8293217 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -14,6 +14,13 @@
 #undef TRACE_SYSTEM
 #define TRACE_SYSTEM xhci-hcd
 
+/*
+ * The TRACE_SYSTEM_VAR defaults to TRACE_SYSTEM, but must be a
+ * legitimate C variable. It is not exported to user space.
+ */
+#undef TRACE_SYSTEM_VAR
+#define TRACE_SYSTEM_VAR xhci_hcd
+
 #if !defined(__XHCI_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
 #define __XHCI_TRACE_H
 
diff --git a/include/trace/events/intel-sst.h b/include/trace/events/intel-sst.h
index 76c72d3f1902..a4705d7e549c 100644
--- a/include/trace/events/intel-sst.h
+++ b/include/trace/events/intel-sst.h
@@ -1,6 +1,13 @@
 #undef TRACE_SYSTEM
 #define TRACE_SYSTEM intel-sst
 
+/*
+ * The TRACE_SYSTEM_VAR defaults to TRACE_SYSTEM, but must be a
+ * legitimate C variable. It is not exported to user space.
+ */
+#undef TRACE_SYSTEM_VAR
+#define TRACE_SYSTEM_VAR intel_sst
+
 #if !defined(_TRACE_INTEL_SST_H) || defined(TRACE_HEADER_MULTI_READ)
 #define _TRACE_INTEL_SST_H
 
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 708e2caf8a36..e6c8ed5f6a0c 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -18,11 +18,17 @@
 
 #include <linux/ftrace_event.h>
 
+#ifndef TRACE_SYSTEM_VAR
+#define TRACE_SYSTEM_VAR TRACE_SYSTEM
+#endif
+
 #define __app__(x, y) str__##x##y
 #define __app(x, y) __app__(x, y)
 
+#define TRACE_SYSTEM_STRING __app(TRACE_SYSTEM_VAR,__trace_system_name)
+
 #define TRACE_MAKE_SYSTEM_STR()					      \
-	static const char __app(TRACE_SYSTEM,__trace_system_name)[] = \
+	static const char TRACE_SYSTEM_STRING[] = \
 		__stringify(TRACE_SYSTEM)
 
 TRACE_MAKE_SYSTEM_STR();
@@ -32,7 +38,7 @@ TRACE_MAKE_SYSTEM_STR();
 	static struct trace_enum_map __used __initdata			\
 	__##TRACE_SYSTEM##_##a =					\
 	{								\
-		.system = __app(TRACE_SYSTEM,__trace_system_name),	\
+		.system = TRACE_SYSTEM_STRING,				\
 		.enum_string = #a,					\
 		.enum_value = a						\
 	};								\
@@ -127,7 +133,6 @@ TRACE_MAKE_SYSTEM_STR();
 
 #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
 
-
 /*
  * Stage 2 of the trace events.
  *
@@ -717,7 +722,7 @@ static inline void ftrace_test_probe_##call(void)			\
 _TRACE_PERF_PROTO(call, PARAMS(proto));					\
 static char print_fmt_##call[] = print;					\
 static struct ftrace_event_class __used __refdata event_class_##call = { \
-	.system			= __app(TRACE_SYSTEM,__trace_system_name), \
+	.system			= TRACE_SYSTEM_STRING,			\
 	.define_fields		= ftrace_define_fields_##call,		\
 	.fields			= LIST_HEAD_INIT(event_class_##call.fields),\
 	.raw_init		= trace_event_raw_init,			\
@@ -760,6 +765,7 @@ __attribute__((section("_ftrace_events"))) *__event_##call = &event_##call
 
 #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
 
+#undef TRACE_SYSTEM_VAR
 
 #ifdef CONFIG_PERF_EVENTS