LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [RFC][PATCH] tracing: Replace '-' with '_' in event system names
@ 2015-04-01  3:18 Steven Rostedt
  2015-04-01  9:20 ` Xenia Ragiadakou
  2015-04-01 10:36 ` Cornelia Huck
  0 siblings, 2 replies; 4+ messages in thread
From: Steven Rostedt @ 2015-04-01  3:18 UTC (permalink / raw)
  To: LKML
  Cc: Christian Borntraeger, Cornelia Huck, David Hildenbrand,
	Marcelo Tosatti, Xenia Ragiadakou, Sarah Sharp, Mark Brown

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC][PATCH] tracing: Replace '-' with '_' in event system names
  2015-04-01  3:18 [RFC][PATCH] tracing: Replace '-' with '_' in event system names Steven Rostedt
@ 2015-04-01  9:20 ` Xenia Ragiadakou
  2015-04-01 10:36 ` Cornelia Huck
  1 sibling, 0 replies; 4+ messages in thread
From: Xenia Ragiadakou @ 2015-04-01  9:20 UTC (permalink / raw)
  To: Steven Rostedt, LKML
  Cc: Christian Borntraeger, Cornelia Huck, David Hildenbrand,
	Marcelo Tosatti, Sarah Sharp, Mark Brown

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC][PATCH] tracing: Replace '-' with '_' in event system names
  2015-04-01  3:18 [RFC][PATCH] tracing: Replace '-' with '_' in event system names Steven Rostedt
  2015-04-01  9:20 ` Xenia Ragiadakou
@ 2015-04-01 10:36 ` Cornelia Huck
  2015-04-01 17:06   ` Steven Rostedt
  1 sibling, 1 reply; 4+ messages in thread
From: Cornelia Huck @ 2015-04-01 10:36 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: LKML, Christian Borntraeger, David Hildenbrand, Marcelo Tosatti,
	Xenia Ragiadakou, Sarah Sharp, Mark Brown

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.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC][PATCH] tracing: Replace '-' with '_' in event system names
  2015-04-01 10:36 ` Cornelia Huck
@ 2015-04-01 17:06   ` Steven Rostedt
  0 siblings, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2015-04-01 17:06 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: LKML, Christian Borntraeger, David Hildenbrand, Marcelo Tosatti,
	Xenia Ragiadakou, Sarah Sharp, Mark Brown

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
 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-04-01 17:06 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-01  3:18 [RFC][PATCH] tracing: Replace '-' with '_' in event system names Steven Rostedt
2015-04-01  9:20 ` Xenia Ragiadakou
2015-04-01 10:36 ` Cornelia Huck
2015-04-01 17:06   ` Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).