[PATCH 00/18 v3] tracing: Use TRACE_DEFINE_ENUM() to show enum values

[PATCH 00/18 v3] tracing: Use TRACE_DEFINE_ENUM() to show enum values

[Steven Rostedt]

As there are many tracepoints that use __print_symbolic() to translate
numbers into ASCII strings, and several of these translate enums as
well, it causes a problem for user space tools that read the tracepoint
format files and have to translate the binary data to their associated
strings.

For example, with the tlb_flush tracepoint, we have this in the format
file:

print fmt: "pages:%ld reason:%s (%d)", REC->pages,
 __print_symbolic(REC->reason,
   { TLB_FLUSH_ON_TASK_SWITCH, "flush on task switch" },
   { TLB_REMOTE_SHOOTDOWN, "remote shootdown" },
   { TLB_LOCAL_SHOOTDOWN, "local shootdown" },
   { TLB_LOCAL_MM_SHOOTDOWN, "local mm shootdown" }), REC->reason

Now, userspace does not know what the value of TLB_REMOTE_SHOOTDOWN is.
To solve this, a new macro is created as a helper to allow tracepoints
to export enums they use to userspace. This macro is called,
TRACE_DEFINE_ENUM(), such that

 TRACE_DEFINE_ENUM(TLB_REMOTE_SHOOTDOWN);

will convert the "print fmt"s in the format files to its actual value
and no longer display the enum name.

On boot up (or module load), the enums saved via TRACE_DEFINE_ENUM()
will be searched for in the TP_printk()s of the tracepoints. Logic
knows enough to ignore quoted text.

And the output of the tlb_flush format is now:

print fmt: "pages:%ld reason:%s (%d)", REC->pages,
 __print_symbolic(REC->reason,
   { 0, "flush on task switch" },
   { 1, "remote shootdown" },
   { 2, "local shootdown" },
   { 3, "local mm shootdown" }), REC->reason

And userspace tools can easily parse that without special handling.

For debugging, if CONFIG_TRACE_ENUM_MAP_FILE is enabled, a file is added
in the tracing directory to show what enums were added, their values and
the TRACE_SYSTEM that added them:

 # cat /sys/kernel/debug/tracing/enum_map
TLB_LOCAL_MM_SHOOTDOWN 3 (tlb)
TLB_LOCAL_SHOOTDOWN 2 (tlb)
TLB_REMOTE_SHOOTDOWN 1 (tlb)
TLB_FLUSH_ON_TASK_SWITCH 0 (tlb)

-- Steve

Local SHA1: 6689c330e3a60defc4c89da17cf5d561649bc55b


Steven Rostedt (1):
      tracing/drm: Remove unused TRACE_SYSTEM_STRING define

Steven Rostedt (Red Hat) (17):
      tracing: Add TRACE_SYSTEM_VAR to intel-sst
      tracing: Add TRACE_SYSTEM_VAR to kvm-s390
      tracing: Add TRACE_SYSTEM_VAR to xhci-hcd
      tracing: Give system name a pointer
      tracing: Update trace-event-sample with TRACE_SYSTEM_VAR documentation
      tracing: Add TRACE_DEFINE_ENUM() macro to map enums to their values
      tracing: Allow for modules to convert their enums to values
      tracing/samples: Update the trace-event-sample.h with TRACE_DEFINE_ENUM()
      x86/tlb/trace: Export enums in used by tlb_flush tracepoint
      net/9p/tracing: Export enums in tracepoints to userspace
      f2fs: Export the enums in the tracepoints to userspace
      irq/tracing: Export enums in tracepoints to user space
      mm: tracing: Export enums in tracepoints to user space
      SUNRPC: Export enums in tracepoints to user space
      v4l: Export enums used by tracepoints to user space
      writeback: Export enums used by tracepoint to user space
      tracing: Add enum_map file to show enums that have been mapped

----
 arch/s390/kvm/trace-s390.h                 |   7 +
 drivers/gpu/drm/drm_trace.h                |   1 -
 drivers/gpu/drm/i915/i915_trace.h          |   1 -
 drivers/gpu/drm/radeon/radeon_trace.h      |   1 -
 drivers/usb/host/xhci-trace.h              |   7 +
 include/asm-generic/vmlinux.lds.h          |   5 +-
 include/linux/ftrace_event.h               |   4 +-
 include/linux/module.h                     |   2 +
 include/linux/tracepoint.h                 |   8 +
 include/trace/events/9p.h                  | 157 ++++++++-------
 include/trace/events/f2fs.h                |  30 +++
 include/trace/events/intel-sst.h           |   7 +
 include/trace/events/irq.h                 |  39 ++--
 include/trace/events/migrate.h             |  42 +++-
 include/trace/events/sunrpc.h              |  62 ++++--
 include/trace/events/tlb.h                 |  30 ++-
 include/trace/events/v4l2.h                |  75 ++++---
 include/trace/events/writeback.h           |  33 +++-
 include/trace/ftrace.h                     |  41 +++-
 kernel/module.c                            |   3 +
 kernel/trace/Kconfig                       |  28 +++
 kernel/trace/trace.c                       | 304 ++++++++++++++++++++++++++++-
 kernel/trace/trace.h                       |   2 +
 kernel/trace/trace_events.c                | 121 +++++++++++-
 samples/trace_events/trace-events-sample.h |  84 +++++++-
 25 files changed, 932 insertions(+), 162 deletions(-)

[PATCH 02/18 v3] tracing: Add TRACE_SYSTEM_VAR to intel-sst

[Steven Rostedt]

[-- Attachment #1: 0002-tracing-Add-TRACE_SYSTEM_VAR-to-intel-sst.patch --]
[-- Type: text/plain, Size: 1143 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

New code will require TRACE_SYSTEM to be a valid C variable name,
but some tracepoints have TRACE_SYSTEM with '-' and not '_', so
it can not be used. Instead, add a TRACE_SYSTEM_VAR that can
give the tracing infrastructure a unique name for the trace system.

Link: http://lkml.kernel.org/r/20150402142831.GT6023@sirena.org.uk

Acked-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/trace/events/intel-sst.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/trace/events/intel-sst.h b/include/trace/events/intel-sst.h
index 76c72d3f1902..edc24e6dea1b 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
 
-- 
2.1.4

[PATCH 03/18 v3] tracing: Add TRACE_SYSTEM_VAR to kvm-s390

[Steven Rostedt]

[-- Attachment #1: 0003-tracing-Add-TRACE_SYSTEM_VAR-to-kvm-s390.patch --]
[-- Type: text/plain, Size: 1312 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

New code will require TRACE_SYSTEM to be a valid C variable name,
but some tracepoints have TRACE_SYSTEM with '-' and not '_', so
it can not be used. Instead, add a TRACE_SYSTEM_VAR that can
give the tracing infrastructure a unique name for the trace system.

Link: http://lkml.kernel.org/r/20150402111500.5e52c1ed.cornelia.huck@de.ibm.com

Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: David Hildenbrand <dahi@linux.vnet.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/s390/kvm/trace-s390.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/s390/kvm/trace-s390.h b/arch/s390/kvm/trace-s390.h
index 653a7ec09ef5..3208d33a48cb 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 kvm_s390
+
+/*
  * Trace point for the creation of the kvm instance.
  */
 TRACE_EVENT(kvm_s390_create_vm,
-- 
2.1.4

[PATCH 04/18 v3] tracing: Add TRACE_SYSTEM_VAR to xhci-hcd

[Steven Rostedt]

[-- Attachment #1: 0004-tracing-Add-TRACE_SYSTEM_VAR-to-xhci-hcd.patch --]
[-- Type: text/plain, Size: 1103 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

New code will require TRACE_SYSTEM to be a valid C variable name,
but some tracepoints have TRACE_SYSTEM with '-' and not '_', so
it can not be used. Instead, add a TRACE_SYSTEM_VAR that can
give the tracing infrastructure a unique name for the trace system.

Cc: Xenia Ragiadakou <burzalodowa@gmail.com>
Cc: Sarah Sharp <sarah.a.sharp@linux.intel.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 drivers/usb/host/xhci-trace.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index dde3959b7a33..59c05653b2ea 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
 
-- 
2.1.4

[PATCH 05/18 v3] tracing: Give system name a pointer

[Steven Rostedt]

[-- Attachment #1: 0005-tracing-Give-system-name-a-pointer.patch --]
[-- Type: text/plain, Size: 2555 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

Normally the compiler will use the same pointer for a string throughout
the file. But there's no guarantee of that happening. Later changes will
require that all events have the same pointer to the system string.

Name the system string and have all events point to it.

Testing this, it did not increases the size of the text, except for the
notes section, which should not harm the real size any.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/linux/ftrace_event.h |  2 +-
 include/trace/ftrace.h       | 19 +++++++++++++++++--
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index c674ee8f7fca..62b8fac7ded5 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -202,7 +202,7 @@ enum trace_reg {
 struct ftrace_event_call;
 
 struct ftrace_event_class {
-	char			*system;
+	const char		*system;
 	void			*probe;
 #ifdef CONFIG_PERF_EVENTS
 	void			*perf_probe;
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 41bf65f04dd9..2f9b95b6d3fb 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -18,6 +18,21 @@
 
 #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 TRACE_SYSTEM_STRING[] =	\
+		__stringify(TRACE_SYSTEM)
+
+TRACE_MAKE_SYSTEM_STR();
+
 /*
  * DECLARE_EVENT_CLASS can be used to add a generic function
  * handlers for events. That is, if all events have the same
@@ -105,7 +120,6 @@
 
 #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
 
-
 /*
  * Stage 2 of the trace events.
  *
@@ -692,7 +706,7 @@ static inline void ftrace_test_probe_##call(void)			\
 _TRACE_PERF_PROTO(call, PARAMS(proto));					\
 static const char print_fmt_##call[] = print;				\
 static struct ftrace_event_class __used __refdata event_class_##call = { \
-	.system			= __stringify(TRACE_SYSTEM),		\
+	.system			= TRACE_SYSTEM_STRING,			\
 	.define_fields		= ftrace_define_fields_##call,		\
 	.fields			= LIST_HEAD_INIT(event_class_##call.fields),\
 	.raw_init		= trace_event_raw_init,			\
@@ -735,6 +749,7 @@ __attribute__((section("_ftrace_events"))) *__event_##call = &event_##call
 
 #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
 
+#undef TRACE_SYSTEM_VAR
 
 #ifdef CONFIG_PERF_EVENTS
 
-- 
2.1.4

[PATCH 06/18 v3] tracing: Update trace-event-sample with TRACE_SYSTEM_VAR documentation

[Steven Rostedt]

[-- Attachment #1: 0006-tracing-Update-trace-event-sample-with-TRACE_SYSTEM_.patch --]
[-- Type: text/plain, Size: 1644 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

Add documentation about TRACE_SYSTEM needing to be alpha-numeric or with
underscores, and that if it is not, then the use of TRACE_SYSTEM_VAR is
required to make something that is.

An example of this is shown in samples/trace_events/trace-events-sample.h

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 samples/trace_events/trace-events-sample.h | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/samples/trace_events/trace-events-sample.h b/samples/trace_events/trace-events-sample.h
index a2c8b02b6359..19405f18cc8a 100644
--- a/samples/trace_events/trace-events-sample.h
+++ b/samples/trace_events/trace-events-sample.h
@@ -22,7 +22,25 @@
  * protection, just like TRACE_INCLUDE_FILE.
  */
 #undef TRACE_SYSTEM
-#define TRACE_SYSTEM sample
+#define TRACE_SYSTEM sample-trace
+
+/*
+ * TRACE_SYSTEM is expected to be a C valid variable (alpha-numeric
+ * and underscore), although it may start with numbers. If for some
+ * reason it is not, you need to add the following lines:
+ */
+#undef TRACE_SYSTEM_VAR
+#define TRACE_SYSTEM_VAR sample_trace
+/*
+ * But the above is only needed if TRACE_SYSTEM is not alpha-numeric
+ * and underscored. By default, TRACE_SYSTEM_VAR will be equal to
+ * TRACE_SYSTEM. As TRACE_SYSTEM_VAR must be alpha-numeric, if
+ * TRACE_SYSTEM is not, then TRACE_SYSTEM_VAR must be defined with
+ * only alpha-numeric and underscores.
+ *
+ * The TRACE_SYSTEM_VAR is only used internally and not visible to
+ * user space.
+ */
 
 /*
  * Notice that this file is not protected like a normal header.
-- 
2.1.4

[PATCH 07/18 v3] tracing: Add TRACE_DEFINE_ENUM() macro to map enums to their values

[Steven Rostedt]

[-- Attachment #1: 0007-tracing-Add-TRACE_DEFINE_ENUM-macro-to-map-enums-to-.patch --]
[-- Type: text/plain, Size: 11572 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

Several tracepoints use the helper functions __print_symbolic() or
__print_flags() and pass in enums that do the mapping between the
binary data stored and the value to print. This works well for reading
the ASCII trace files, but when the data is read via userspace tools
such as perf and trace-cmd, the conversion of the binary value to a
human string format is lost if an enum is used, as userspace does not
have access to what the ENUM is.

For example, the tracepoint trace_tlb_flush() has:

 __print_symbolic(REC->reason,
    { TLB_FLUSH_ON_TASK_SWITCH, "flush on task switch" },
    { TLB_REMOTE_SHOOTDOWN, "remote shootdown" },
    { TLB_LOCAL_SHOOTDOWN, "local shootdown" },
    { TLB_LOCAL_MM_SHOOTDOWN, "local mm shootdown" })

Which maps the enum values to the strings they represent. But perf and
trace-cmd do no know what value TLB_LOCAL_MM_SHOOTDOWN is, and would
not be able to map it.

With TRACE_DEFINE_ENUM(), developers can place these in the event header
files and ftrace will convert the enums to their values:

By adding:

 TRACE_DEFINE_ENUM(TLB_FLUSH_ON_TASK_SWITCH);
 TRACE_DEFINE_ENUM(TLB_REMOTE_SHOOTDOWN);
 TRACE_DEFINE_ENUM(TLB_LOCAL_SHOOTDOWN);
 TRACE_DEFINE_ENUM(TLB_LOCAL_MM_SHOOTDOWN);

 $ cat /sys/kernel/debug/tracing/events/tlb/tlb_flush/format
[...]
 __print_symbolic(REC->reason,
    { 0, "flush on task switch" },
    { 1, "remote shootdown" },
    { 2, "local shootdown" },
    { 3, "local mm shootdown" })

The above is what userspace expects to see, and tools do not need to
be modified to parse them.

Cc: Guilherme Cox <cox@computer.org>
Cc: Tony Luck <tony.luck@gmail.com>
Cc: Xie XiuQi <xiexiuqi@huawei.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/asm-generic/vmlinux.lds.h |   5 +-
 include/linux/ftrace_event.h      |   2 +-
 include/linux/tracepoint.h        |   8 +++
 include/trace/ftrace.h            |  22 ++++++-
 kernel/trace/trace.c              |  26 ++++++++-
 kernel/trace/trace.h              |   2 +
 kernel/trace/trace_events.c       | 119 ++++++++++++++++++++++++++++++++++++++
 7 files changed, 178 insertions(+), 6 deletions(-)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index ac78910d7416..f8e8b34dc427 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -124,7 +124,10 @@
 #define FTRACE_EVENTS()	. = ALIGN(8);					\
 			VMLINUX_SYMBOL(__start_ftrace_events) = .;	\
 			*(_ftrace_events)				\
-			VMLINUX_SYMBOL(__stop_ftrace_events) = .;
+			VMLINUX_SYMBOL(__stop_ftrace_events) = .;	\
+			VMLINUX_SYMBOL(__start_ftrace_enum_maps) = .;	\
+			*(_ftrace_enum_map)				\
+			VMLINUX_SYMBOL(__stop_ftrace_enum_maps) = .;
 #else
 #define FTRACE_EVENTS()
 #endif
diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index 62b8fac7ded5..112cf49d9576 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -285,7 +285,7 @@ struct ftrace_event_call {
 		struct tracepoint	*tp;
 	};
 	struct trace_event	event;
-	const char		*print_fmt;
+	char			*print_fmt;
 	struct event_filter	*filter;
 	void			*mod;
 	void			*data;
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index c72851328ca9..a5f7f3ecafa3 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -36,6 +36,12 @@ struct tracepoint {
 	struct tracepoint_func __rcu *funcs;
 };
 
+struct trace_enum_map {
+	const char		*system;
+	const char		*enum_string;
+	unsigned long		enum_value;
+};
+
 extern int
 tracepoint_probe_register(struct tracepoint *tp, void *probe, void *data);
 extern int
@@ -87,6 +93,8 @@ extern void syscall_unregfunc(void);
 
 #define PARAMS(args...) args
 
+#define TRACE_DEFINE_ENUM(x)
+
 #endif /* _LINUX_TRACEPOINT_H */
 
 /*
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 2f9b95b6d3fb..37d4b10b111d 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -33,6 +33,19 @@
 
 TRACE_MAKE_SYSTEM_STR();
 
+#undef TRACE_DEFINE_ENUM
+#define TRACE_DEFINE_ENUM(a)				\
+	static struct trace_enum_map __used __initdata	\
+	__##TRACE_SYSTEM##_##a =			\
+	{						\
+		.system = TRACE_SYSTEM_STRING,		\
+		.enum_string = #a,			\
+		.enum_value = a				\
+	};						\
+	static struct trace_enum_map __used		\
+	__attribute__((section("_ftrace_enum_map")))	\
+	*TRACE_SYSTEM##_##a = &__##TRACE_SYSTEM##_##a
+
 /*
  * DECLARE_EVENT_CLASS can be used to add a generic function
  * handlers for events. That is, if all events have the same
@@ -136,6 +149,9 @@ TRACE_MAKE_SYSTEM_STR();
  * The size of an array is also encoded, in the higher 16 bits of <item>.
  */
 
+#undef TRACE_DEFINE_ENUM
+#define TRACE_DEFINE_ENUM(a)
+
 #undef __field
 #define __field(type, item)
 
@@ -553,7 +569,7 @@ static inline notrace int ftrace_get_offsets_##call(			\
  *	.trace			= ftrace_raw_output_<call>, <-- stage 2
  * };
  *
- * static const char print_fmt_<call>[] = <TP_printk>;
+ * static char print_fmt_<call>[] = <TP_printk>;
  *
  * static struct ftrace_event_class __used event_class_<template> = {
  *	.system			= "<system>",
@@ -704,7 +720,7 @@ static inline void ftrace_test_probe_##call(void)			\
 #undef DECLARE_EVENT_CLASS
 #define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print)	\
 _TRACE_PERF_PROTO(call, PARAMS(proto));					\
-static const char print_fmt_##call[] = print;				\
+static char print_fmt_##call[] = print;					\
 static struct ftrace_event_class __used __refdata event_class_##call = { \
 	.system			= TRACE_SYSTEM_STRING,			\
 	.define_fields		= ftrace_define_fields_##call,		\
@@ -733,7 +749,7 @@ __attribute__((section("_ftrace_events"))) *__event_##call = &event_##call
 #undef DEFINE_EVENT_PRINT
 #define DEFINE_EVENT_PRINT(template, call, proto, args, print)		\
 									\
-static const char print_fmt_##call[] = print;				\
+static char print_fmt_##call[] = print;					\
 									\
 static struct ftrace_event_call __used event_##call = {			\
 	.class			= &event_class_##template,		\
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 62c6506d663f..ebf49649534c 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -3908,6 +3908,20 @@ static const struct file_operations tracing_saved_cmdlines_size_fops = {
 	.write		= tracing_saved_cmdlines_size_write,
 };
 
+static void
+trace_insert_enum_map(struct trace_enum_map **start, struct trace_enum_map **stop)
+{
+	struct trace_enum_map **map;
+	int len = stop - start;
+
+	if (len <= 0)
+		return;
+
+	map = start;
+
+	trace_event_enum_update(map, len);
+}
+
 static ssize_t
 tracing_set_trace_read(struct file *filp, char __user *ubuf,
 		       size_t cnt, loff_t *ppos)
@@ -6542,6 +6556,14 @@ struct dentry *tracing_init_dentry(void)
 	return tr->dir;
 }
 
+extern struct trace_enum_map *__start_ftrace_enum_maps[];
+extern struct trace_enum_map *__stop_ftrace_enum_maps[];
+
+static void __init trace_enum_init(void)
+{
+	trace_insert_enum_map(__start_ftrace_enum_maps, __stop_ftrace_enum_maps);
+}
+
 static __init int tracer_init_debugfs(void)
 {
 	struct dentry *d_tracer;
@@ -6566,6 +6588,8 @@ static __init int tracer_init_debugfs(void)
 	trace_create_file("saved_cmdlines_size", 0644, d_tracer,
 			  NULL, &tracing_saved_cmdlines_size_fops);
 
+	trace_enum_init();
+
 #ifdef CONFIG_DYNAMIC_FTRACE
 	trace_create_file("dyn_ftrace_total_info", 0444, d_tracer,
 			&ftrace_update_tot_cnt, &tracing_dyn_info_fops);
@@ -6888,7 +6912,7 @@ void __init trace_init(void)
 			tracepoint_printk = 0;
 	}
 	tracer_alloc_buffers();
-	trace_event_init();	
+	trace_event_init();
 }
 
 __init static int clear_boot_tracer(void)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index dd8205a35760..b48d4b08f691 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1309,8 +1309,10 @@ static inline void init_ftrace_syscalls(void) { }
 
 #ifdef CONFIG_EVENT_TRACING
 void trace_event_init(void);
+void trace_event_enum_update(struct trace_enum_map **map, int len);
 #else
 static inline void __init trace_event_init(void) { }
+static inlin void trace_event_enum_update(struct trace_enum_map **map, int len) { }
 #endif
 
 extern struct trace_iterator *tracepoint_print_iter;
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index db54dda10ccc..fc58c50fbf01 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1704,6 +1704,125 @@ __register_event(struct ftrace_event_call *call, struct module *mod)
 	return 0;
 }
 
+static char *enum_replace(char *ptr, struct trace_enum_map *map, int len)
+{
+	int rlen;
+	int elen;
+
+	/* Find the length of the enum value as a string */
+	elen = snprintf(ptr, 0, "%ld", map->enum_value);
+	/* Make sure there's enough room to replace the string with the value */
+	if (len < elen)
+		return NULL;
+
+	snprintf(ptr, elen + 1, "%ld", map->enum_value);
+
+	/* Get the rest of the string of ptr */
+	rlen = strlen(ptr + len);
+	memmove(ptr + elen, ptr + len, rlen);
+	/* Make sure we end the new string */
+	ptr[elen + rlen] = 0;
+
+	return ptr + elen;
+}
+
+static void update_event_printk(struct ftrace_event_call *call,
+				struct trace_enum_map *map)
+{
+	char *ptr;
+	int quote = 0;
+	int len = strlen(map->enum_string);
+
+	for (ptr = call->print_fmt; *ptr; ptr++) {
+		if (*ptr == '\\') {
+			ptr++;
+			/* paranoid */
+			if (!*ptr)
+				break;
+			continue;
+		}
+		if (*ptr == '"') {
+			quote ^= 1;
+			continue;
+		}
+		if (quote)
+			continue;
+		if (isdigit(*ptr)) {
+			/* skip numbers */
+			do {
+				ptr++;
+				/* Check for alpha chars like ULL */
+			} while (isalnum(*ptr));
+			/*
+			 * A number must have some kind of delimiter after
+			 * it, and we can ignore that too.
+			 */
+			continue;
+		}
+		if (isalpha(*ptr) || *ptr == '_') {
+			if (strncmp(map->enum_string, ptr, len) == 0 &&
+			    !isalnum(ptr[len]) && ptr[len] != '_') {
+				ptr = enum_replace(ptr, map, len);
+				/* Hmm, enum string smaller than value */
+				if (WARN_ON_ONCE(!ptr))
+					return;
+				/*
+				 * No need to decrement here, as enum_replace()
+				 * returns the pointer to the character passed
+				 * the enum, and two enums can not be placed
+				 * back to back without something in between.
+				 * We can skip that something in between.
+				 */
+				continue;
+			}
+		skip_more:
+			do {
+				ptr++;
+			} while (isalnum(*ptr) || *ptr == '_');
+			/*
+			 * If what comes after this variable is a '.' or
+			 * '->' then we can continue to ignore that string.
+			 */
+			if (*ptr == '.' || (ptr[0] == '-' && ptr[1] == '>')) {
+				ptr += *ptr == '.' ? 1 : 2;
+				goto skip_more;
+			}
+			/*
+			 * Once again, we can skip the delimiter that came
+			 * after the string.
+			 */
+			continue;
+		}
+	}
+}
+
+void trace_event_enum_update(struct trace_enum_map **map, int len)
+{
+	struct ftrace_event_call *call, *p;
+	const char *last_system = NULL;
+	int last_i;
+	int i;
+
+	down_write(&trace_event_sem);
+	list_for_each_entry_safe(call, p, &ftrace_events, list) {
+		/* events are usually grouped together with systems */
+		if (!last_system || call->class->system != last_system) {
+			last_i = 0;
+			last_system = call->class->system;
+		}
+
+		for (i = last_i; i < len; i++) {
+			if (call->class->system == map[i]->system) {
+				/* Save the first system if need be */
+				if (!last_i)
+					last_i = i;
+				update_event_printk(call, map[i]);
+			}
+		}
+	}
+	up_write(&trace_event_sem);
+}
+
 static struct ftrace_event_file *
 trace_create_new_event(struct ftrace_event_call *call,
 		       struct trace_array *tr)
-- 
2.1.4

Re: [PATCH 07/18 v3] tracing: Add TRACE_DEFINE_ENUM() macro to map enums to their values

[Namhyung Kim]

Hi Steve,

On Thu, Apr 02, 2015 at 09:38:09PM -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> 
> Several tracepoints use the helper functions __print_symbolic() or
> __print_flags() and pass in enums that do the mapping between the
> binary data stored and the value to print. This works well for reading
> the ASCII trace files, but when the data is read via userspace tools
> such as perf and trace-cmd, the conversion of the binary value to a
> human string format is lost if an enum is used, as userspace does not
> have access to what the ENUM is.
> 
> For example, the tracepoint trace_tlb_flush() has:
> 
>  __print_symbolic(REC->reason,
>     { TLB_FLUSH_ON_TASK_SWITCH, "flush on task switch" },
>     { TLB_REMOTE_SHOOTDOWN, "remote shootdown" },
>     { TLB_LOCAL_SHOOTDOWN, "local shootdown" },
>     { TLB_LOCAL_MM_SHOOTDOWN, "local mm shootdown" })
> 
> Which maps the enum values to the strings they represent. But perf and
> trace-cmd do no know what value TLB_LOCAL_MM_SHOOTDOWN is, and would
> not be able to map it.
> 
> With TRACE_DEFINE_ENUM(), developers can place these in the event header
> files and ftrace will convert the enums to their values:
> 
> By adding:
> 
>  TRACE_DEFINE_ENUM(TLB_FLUSH_ON_TASK_SWITCH);
>  TRACE_DEFINE_ENUM(TLB_REMOTE_SHOOTDOWN);
>  TRACE_DEFINE_ENUM(TLB_LOCAL_SHOOTDOWN);
>  TRACE_DEFINE_ENUM(TLB_LOCAL_MM_SHOOTDOWN);
> 
>  $ cat /sys/kernel/debug/tracing/events/tlb/tlb_flush/format
> [...]
>  __print_symbolic(REC->reason,
>     { 0, "flush on task switch" },
>     { 1, "remote shootdown" },
>     { 2, "local shootdown" },
>     { 3, "local mm shootdown" })
> 
> The above is what userspace expects to see, and tools do not need to
> be modified to parse them.
> 
> Cc: Guilherme Cox <cox@computer.org>
> Cc: Tony Luck <tony.luck@gmail.com>
> Cc: Xie XiuQi <xiexiuqi@huawei.com>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---

[SNIP]
> +static void update_event_printk(struct ftrace_event_call *call,
> +				struct trace_enum_map *map)
> +{
> +	char *ptr;
> +	int quote = 0;
> +	int len = strlen(map->enum_string);
> +
> +	for (ptr = call->print_fmt; *ptr; ptr++) {
> +		if (*ptr == '\\') {
> +			ptr++;
> +			/* paranoid */
> +			if (!*ptr)
> +				break;
> +			continue;
> +		}
> +		if (*ptr == '"') {
> +			quote ^= 1;
> +			continue;
> +		}
> +		if (quote)
> +			continue;
> +		if (isdigit(*ptr)) {
> +			/* skip numbers */
> +			do {
> +				ptr++;
> +				/* Check for alpha chars like ULL */
> +			} while (isalnum(*ptr));
> +			/*
> +			 * A number must have some kind of delimiter after
> +			 * it, and we can ignore that too.
> +			 */
> +			continue;
> +		}
> +		if (isalpha(*ptr) || *ptr == '_') {
> +			if (strncmp(map->enum_string, ptr, len) == 0 &&
> +			    !isalnum(ptr[len]) && ptr[len] != '_') {
> +				ptr = enum_replace(ptr, map, len);
> +				/* Hmm, enum string smaller than value */
> +				if (WARN_ON_ONCE(!ptr))
> +					return;
> +				/*
> +				 * No need to decrement here, as enum_replace()
> +				 * returns the pointer to the character passed
> +				 * the enum, and two enums can not be placed
> +				 * back to back without something in between.
> +				 * We can skip that something in between.
> +				 */
> +				continue;

Maybe I'm becoming a bit paranoid, what I worried was like this:

  ENUM1\"ENUM2\"

In this case, it skips the backslash and makes quotation effective..


> +			}
> +		skip_more:
> +			do {
> +				ptr++;
> +			} while (isalnum(*ptr) || *ptr == '_');
> +			/*
> +			 * If what comes after this variable is a '.' or
> +			 * '->' then we can continue to ignore that string.
> +			 */
> +			if (*ptr == '.' || (ptr[0] == '-' && ptr[1] == '>')) {
> +				ptr += *ptr == '.' ? 1 : 2;
> +				goto skip_more;
> +			}
> +			/*
> +			 * Once again, we can skip the delimiter that came
> +			 * after the string.
> +			 */
> +			continue;
> +		}
> +	}
> +}
> +
> +void trace_event_enum_update(struct trace_enum_map **map, int len)
> +{
> +	struct ftrace_event_call *call, *p;
> +	const char *last_system = NULL;
> +	int last_i;
> +	int i;
> +
> +	down_write(&trace_event_sem);
> +	list_for_each_entry_safe(call, p, &ftrace_events, list) {
> +		/* events are usually grouped together with systems */
> +		if (!last_system || call->class->system != last_system) {

I think simply checking "call->class->system != last_system" would work.

Thanks,
Namhyung


> +			last_i = 0;
> +			last_system = call->class->system;
> +		}
> +
> +		for (i = last_i; i < len; i++) {
> +			if (call->class->system == map[i]->system) {
> +				/* Save the first system if need be */
> +				if (!last_i)
> +					last_i = i;
> +				update_event_printk(call, map[i]);
> +			}
> +		}
> +	}
> +	up_write(&trace_event_sem);
> +}
> +
>  static struct ftrace_event_file *
>  trace_create_new_event(struct ftrace_event_call *call,
>  		       struct trace_array *tr)
> -- 
> 2.1.4
> 
> 

Re: [PATCH 07/18 v3] tracing: Add TRACE_DEFINE_ENUM() macro to map enums to their values

[Steven Rostedt]

On Mon, 6 Apr 2015 13:54:33 +0900
Namhyung Kim <namhyung@kernel.org> wrote:


> > +		if (isalpha(*ptr) || *ptr == '_') {
> > +			if (strncmp(map->enum_string, ptr, len) == 0 &&
> > +			    !isalnum(ptr[len]) && ptr[len] != '_') {
> > +				ptr = enum_replace(ptr, map, len);
> > +				/* Hmm, enum string smaller than value */
> > +				if (WARN_ON_ONCE(!ptr))
> > +					return;
> > +				/*
> > +				 * No need to decrement here, as enum_replace()
> > +				 * returns the pointer to the character passed
> > +				 * the enum, and two enums can not be placed
> > +				 * back to back without something in between.
> > +				 * We can skip that something in between.
> > +				 */
> > +				continue;
> 
> Maybe I'm becoming a bit paranoid, what I worried was like this:
> 
>   ENUM1\"ENUM2\"
> 
> In this case, it skips the backslash and makes quotation effective..

The only time a backslash is OK is if it's in a quote, where we do not
process enums there anyway.

The above isn't valid C outside of quotes, so I'm still not worried.

> 
> 
> > +			}
> > +		skip_more:
> > +			do {
> > +				ptr++;
> > +			} while (isalnum(*ptr) || *ptr == '_');
> > +			/*
> > +			 * If what comes after this variable is a '.' or
> > +			 * '->' then we can continue to ignore that string.
> > +			 */
> > +			if (*ptr == '.' || (ptr[0] == '-' && ptr[1] == '>')) {
> > +				ptr += *ptr == '.' ? 1 : 2;
> > +				goto skip_more;
> > +			}
> > +			/*
> > +			 * Once again, we can skip the delimiter that came
> > +			 * after the string.
> > +			 */
> > +			continue;
> > +		}
> > +	}
> > +}
> > +
> > +void trace_event_enum_update(struct trace_enum_map **map, int len)
> > +{
> > +	struct ftrace_event_call *call, *p;
> > +	const char *last_system = NULL;
> > +	int last_i;
> > +	int i;
> > +
> > +	down_write(&trace_event_sem);
> > +	list_for_each_entry_safe(call, p, &ftrace_events, list) {
> > +		/* events are usually grouped together with systems */
> > +		if (!last_system || call->class->system != last_system) {
> 
> I think simply checking "call->class->system != last_system" would work.

I think you are correct, but I'm not sure I want to change it. Mainly
because it's more readable that way. The !last_system is basically the
"this is first time". Leaving it out may cause people to think it's
wrong.

But I may change my mind and remove it anyway ;-)

If there's other things wrong with this patch, I may update this too.

Thanks for reviewing.

-- Steve

Re: [PATCH 07/18 v3] tracing: Add TRACE_DEFINE_ENUM() macro to map enums to their values

[Namhyung Kim]

Hi Steve,

On Mon, Apr 06, 2015 at 07:52:37AM -0400, Steven Rostedt wrote:
> On Mon, 6 Apr 2015 13:54:33 +0900
> Namhyung Kim <namhyung@kernel.org> wrote:
> 
> 
> > > +		if (isalpha(*ptr) || *ptr == '_') {
> > > +			if (strncmp(map->enum_string, ptr, len) == 0 &&
> > > +			    !isalnum(ptr[len]) && ptr[len] != '_') {
> > > +				ptr = enum_replace(ptr, map, len);
> > > +				/* Hmm, enum string smaller than value */
> > > +				if (WARN_ON_ONCE(!ptr))
> > > +					return;
> > > +				/*
> > > +				 * No need to decrement here, as enum_replace()
> > > +				 * returns the pointer to the character passed
> > > +				 * the enum, and two enums can not be placed
> > > +				 * back to back without something in between.
> > > +				 * We can skip that something in between.
> > > +				 */
> > > +				continue;
> > 
> > Maybe I'm becoming a bit paranoid, what I worried was like this:
> > 
> >   ENUM1\"ENUM2\"
> > 
> > In this case, it skips the backslash and makes quotation effective..
> 
> The only time a backslash is OK is if it's in a quote, where we do not
> process enums there anyway.
> 
> The above isn't valid C outside of quotes, so I'm still not worried.

OK

> 
> > 
> > 
> > > +			}
> > > +		skip_more:
> > > +			do {
> > > +				ptr++;
> > > +			} while (isalnum(*ptr) || *ptr == '_');
> > > +			/*
> > > +			 * If what comes after this variable is a '.' or
> > > +			 * '->' then we can continue to ignore that string.
> > > +			 */
> > > +			if (*ptr == '.' || (ptr[0] == '-' && ptr[1] == '>')) {
> > > +				ptr += *ptr == '.' ? 1 : 2;
> > > +				goto skip_more;
> > > +			}
> > > +			/*
> > > +			 * Once again, we can skip the delimiter that came
> > > +			 * after the string.
> > > +			 */
> > > +			continue;
> > > +		}
> > > +	}
> > > +}
> > > +
> > > +void trace_event_enum_update(struct trace_enum_map **map, int len)
> > > +{
> > > +	struct ftrace_event_call *call, *p;
> > > +	const char *last_system = NULL;
> > > +	int last_i;
> > > +	int i;
> > > +
> > > +	down_write(&trace_event_sem);
> > > +	list_for_each_entry_safe(call, p, &ftrace_events, list) {
> > > +		/* events are usually grouped together with systems */
> > > +		if (!last_system || call->class->system != last_system) {
> > 
> > I think simply checking "call->class->system != last_system" would work.
> 
> I think you are correct, but I'm not sure I want to change it. Mainly
> because it's more readable that way. The !last_system is basically the
> "this is first time". Leaving it out may cause people to think it's
> wrong.
> 
> But I may change my mind and remove it anyway ;-)
> 
> If there's other things wrong with this patch, I may update this too.
> 
> Thanks for reviewing.

OK.  You can add my Acked-by if you like..

Thanks,
Namhyung

Re: [PATCH 07/18 v3] tracing: Add TRACE_DEFINE_ENUM() macro to map enums to their values

[Steven Rostedt]

On Tue, 7 Apr 2015 23:16:02 +0900
Namhyung Kim <namhyung@kernel.org> wrote:


> > Thanks for reviewing.
> 
> OK.  You can add my Acked-by if you like..

Cool, I will.

Thanks!

-- Steve

Re: [PATCH 07/18 v3] tracing: Add TRACE_DEFINE_ENUM() macro to map enums to their values

[Sasha Levin]

On 04/02/2015 09:38 PM, Steven Rostedt wrote:
> From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> 
> Several tracepoints use the helper functions __print_symbolic() or
> __print_flags() and pass in enums that do the mapping between the
> binary data stored and the value to print. This works well for reading
> the ASCII trace files, but when the data is read via userspace tools
> such as perf and trace-cmd, the conversion of the binary value to a
> human string format is lost if an enum is used, as userspace does not
> have access to what the ENUM is.
> 
> For example, the tracepoint trace_tlb_flush() has:
> 
>  __print_symbolic(REC->reason,
>     { TLB_FLUSH_ON_TASK_SWITCH, "flush on task switch" },
>     { TLB_REMOTE_SHOOTDOWN, "remote shootdown" },
>     { TLB_LOCAL_SHOOTDOWN, "local shootdown" },
>     { TLB_LOCAL_MM_SHOOTDOWN, "local mm shootdown" })
> 
> Which maps the enum values to the strings they represent. But perf and
> trace-cmd do no know what value TLB_LOCAL_MM_SHOOTDOWN is, and would
> not be able to map it.
> 
> With TRACE_DEFINE_ENUM(), developers can place these in the event header
> files and ftrace will convert the enums to their values:
> 
> By adding:
> 
>  TRACE_DEFINE_ENUM(TLB_FLUSH_ON_TASK_SWITCH);
>  TRACE_DEFINE_ENUM(TLB_REMOTE_SHOOTDOWN);
>  TRACE_DEFINE_ENUM(TLB_LOCAL_SHOOTDOWN);
>  TRACE_DEFINE_ENUM(TLB_LOCAL_MM_SHOOTDOWN);
> 
>  $ cat /sys/kernel/debug/tracing/events/tlb/tlb_flush/format
> [...]
>  __print_symbolic(REC->reason,
>     { 0, "flush on task switch" },
>     { 1, "remote shootdown" },
>     { 2, "local shootdown" },
>     { 3, "local mm shootdown" })
> 
> The above is what userspace expects to see, and tools do not need to
> be modified to parse them.
> 
> Cc: Guilherme Cox <cox@computer.org>
> Cc: Tony Luck <tony.luck@gmail.com>
> Cc: Xie XiuQi <xiexiuqi@huawei.com>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---

Hey Steven,

I'm seeing the following when booting:

[   10.678876] BUG: KASan: out of bounds access in trace_event_enum_update+0xb1d/0xb70 at addr ffffffffa6c4dc68
[   10.680222] Read of size 1 by task swapper/0/1
[   10.680222] Address belongs to variable print_fmt_9p_client_req+0x848/0x880
[   10.680222] CPU: 19 PID: 1 Comm: swapper/0 Not tainted 4.0.0-next-20150415-sasha-00064-g7858a62 #2147
[   10.680222]  ffffffffa6c4dc68 00000000915e69dc ffff88004e8efb18 ffffffffa1b4f7a6
[   10.680222]  0000000000000000 ffff88004e8efba8 ffff88004e8efb98 ffffffff976544b1
[   10.680222]  ffff88004e8efbd8 ffffffff990134a4 0000000000000282 ffffffffa444e381
[   10.680222] Call Trace:
[   10.680222] dump_stack (lib/dump_stack.c:52)
[   10.680222] kasan_report_error (mm/kasan/report.c:132 mm/kasan/report.c:193)
[   10.680222] ? snprintf (lib/vsprintf.c:2069)
[   10.680222] ? vsprintf (lib/vsprintf.c:2069)
[   10.680222] ? trace_event_enum_update (kernel/trace/trace_events.c:1807)
[   10.680222] __asan_report_load1_noabort (mm/kasan/report.c:230 mm/kasan/report.c:248)
[   10.680222] ? trace_event_enum_update (kernel/trace/trace_events.c:1807)
[   10.680222] trace_event_enum_update (kernel/trace/trace_events.c:1807)
[   10.680222] tracer_init_tracefs (kernel/trace/trace.c:4122 kernel/trace/trace.c:6750 kernel/trace/trace.c:6848)
[   10.680222] ? clear_boot_tracer (kernel/trace/trace.c:6825)
[   10.680222] ? clear_boot_tracer (kernel/trace/trace.c:6825)
[   10.680222] do_one_initcall (init/main.c:788)
[   10.680222] ? try_to_run_init_process (init/main.c:777)
[   10.680222] ? parse_args (kernel/params.c:131 kernel/params.c:216)
[   10.680222] ? trace_hardirqs_on (kernel/locking/lockdep.c:2630)
[   10.680222] kernel_init_freeable (init/main.c:853 init/main.c:861 init/main.c:880 init/main.c:1001)
[   10.680222] ? local_clock (kernel/sched/clock.c:392)
[   10.680222] ? start_kernel (init/main.c:973)
[   10.680222] ? finish_task_switch (kernel/sched/core.c:2249)
[   10.680222] ? finish_task_switch (include/linux/tick.h:186 kernel/sched/core.c:2263)
[   10.680222] ? finish_task_switch (kernel/sched/sched.h:1077 kernel/sched/core.c:2245)
[   10.680222] ? rest_init (init/main.c:928)
[   10.680222] kernel_init (init/main.c:933)
[   10.680222] ? rest_init (init/main.c:928)
[   10.680222] ret_from_fork (arch/x86/kernel/entry_64.S:631)
[   10.680222] ? rest_init (init/main.c:928)
[   10.680222] Memory state around the buggy address:
[   10.680222]  ffffffffa6c4db00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[   10.680222]  ffffffffa6c4db80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[   10.680222] >ffffffffa6c4dc00: 00 00 00 00 00 00 00 00 00 00 00 00 00 fa fa fa
[   10.680222]                                                           ^
[   10.680222]  ffffffffa6c4dc80: fa fa fa fa 00 00 00 00 fa fa fa fa 00 00 00 00
[   10.680222]  ffffffffa6c4dd00: fa fa fa fa 00 00 00 00 fa fa fa fa 00 00 00 00


Thanks,
Sasha

Re: [PATCH 07/18 v3] tracing: Add TRACE_DEFINE_ENUM() macro to map enums to their values

[Steven Rostedt]

On Wed, 15 Apr 2015 09:22:37 -0400
Sasha Levin <sasha.levin@oracle.com> wrote:

> Hey Steven,
> 
> I'm seeing the following when booting:
> 
> [   10.678876] BUG: KASan: out of bounds access in trace_event_enum_update+0xb1d/0xb70 at addr ffffffffa6c4dc68

Thanks for the report. Mind sending me over your config, and which git
commit was your HEAD. I can't seem to find 7858a62 from your output.

-- Steve

Re: [PATCH 07/18 v3] tracing: Add TRACE_DEFINE_ENUM() macro to map enums to their values

[Sasha Levin]

[-- Attachment #1: Type: text/plain, Size: 542 bytes --]

On 04/15/2015 10:05 AM, Steven Rostedt wrote:
> On Wed, 15 Apr 2015 09:22:37 -0400
> Sasha Levin <sasha.levin@oracle.com> wrote:
> 
>> Hey Steven,
>>
>> I'm seeing the following when booting:
>>
>> [   10.678876] BUG: KASan: out of bounds access in trace_event_enum_update+0xb1d/0xb70 at addr ffffffffa6c4dc68
> 
> Thanks for the report. Mind sending me over your config, and which git
> commit was your HEAD. I can't seem to find 7858a62 from your output.

It reproduces on the latest -next kernel. I've attached my config.


Thanks,
Sasha


[-- Attachment #2: config-sasha.gz --]
[-- Type: application/gzip, Size: 40344 bytes --]

Re: [PATCH 07/18 v3] tracing: Add TRACE_DEFINE_ENUM() macro to map enums to their values

[Steven Rostedt]

On Wed, 15 Apr 2015 20:58:31 -0400
Sasha Levin <sasha.levin@oracle.com> wrote:

> On 04/15/2015 10:05 AM, Steven Rostedt wrote:
> > On Wed, 15 Apr 2015 09:22:37 -0400
> > Sasha Levin <sasha.levin@oracle.com> wrote:
> > 
> >> Hey Steven,
> >>
> >> I'm seeing the following when booting:
> >>
> >> [   10.678876] BUG: KASan: out of bounds access in trace_event_enum_update+0xb1d/0xb70 at addr ffffffffa6c4dc68
> > 
> > Thanks for the report. Mind sending me over your config, and which git
> > commit was your HEAD. I can't seem to find 7858a62 from your output.
> 
> It reproduces on the latest -next kernel. I've attached my config.
> 

What version of gcc is required to run KASan, the highest version I
have to build kernels with is 4.9.0

I have no idea what event caused the issue :-/


-- Steve

Re: [PATCH 07/18 v3] tracing: Add TRACE_DEFINE_ENUM() macro to map enums to their values

[Sasha Levin]

On 04/16/2015 11:21 PM, Steven Rostedt wrote:
> On Wed, 15 Apr 2015 20:58:31 -0400
> Sasha Levin <sasha.levin@oracle.com> wrote:
> 
>> > On 04/15/2015 10:05 AM, Steven Rostedt wrote:
>>> > > On Wed, 15 Apr 2015 09:22:37 -0400
>>> > > Sasha Levin <sasha.levin@oracle.com> wrote:
>>> > > 
>>>> > >> Hey Steven,
>>>> > >>
>>>> > >> I'm seeing the following when booting:
>>>> > >>
>>>> > >> [   10.678876] BUG: KASan: out of bounds access in trace_event_enum_update+0xb1d/0xb70 at addr ffffffffa6c4dc68
>>> > > 
>>> > > Thanks for the report. Mind sending me over your config, and which git
>>> > > commit was your HEAD. I can't seem to find 7858a62 from your output.
>> > 
>> > It reproduces on the latest -next kernel. I've attached my config.
>> > 
> What version of gcc is required to run KASan, the highest version I
> have to build kernels with is 4.9.0
> 
> I have no idea what event caused the issue :-/

4.9.2+ is needed for KASan.


Thanks,
Sasha

[PATCH] kasan: Show gcc version requirements in Kconfig and Documentation

[Joe Perches]

The documentation shows a need for gcc > 4.9.2, but it's
really >=.  The Kconfig entries don't show require versions
so add them.  Correct a latter/later typo too.

Signed-off-by: Joe Perches <joe@perches.com>
---

(dropping Ingo from cc's)

On Thu, 2015-04-16 at 23:59 -0400, Sasha Levin wrote:
> On 04/16/2015 11:21 PM, Steven Rostedt wrote:
> > I have no idea what event caused the issue :-/
> 4.9.2+ is needed for KASan.

Perhaps the documentation and Kconfig entries could
describe that a bit better.

 Documentation/kasan.txt | 6 +++---
 lib/Kconfig.kasan       | 6 ++++--
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/Documentation/kasan.txt b/Documentation/kasan.txt
index 092fc10..3038b4b 100644
--- a/Documentation/kasan.txt
+++ b/Documentation/kasan.txt
@@ -9,7 +9,7 @@ a fast and comprehensive solution for finding use-after-free and out-of-bounds
 bugs.
 
 KASan uses compile-time instrumentation for checking every memory access,
-therefore you will need a certain version of GCC > 4.9.2
+therefore you will need a gcc version of 4.9.2 or later.
 
 Currently KASan is supported only for x86_64 architecture and requires that the
 kernel be built with the SLUB allocator.
@@ -23,8 +23,8 @@ To enable KASAN configure kernel with:
 
 and choose between CONFIG_KASAN_OUTLINE and CONFIG_KASAN_INLINE. Outline/inline
 is compiler instrumentation types. The former produces smaller binary the
-latter is 1.1 - 2 times faster. Inline instrumentation requires GCC 5.0 or
-latter.
+latter is 1.1 - 2 times faster. Inline instrumentation requires a gcc version
+of 5.0 or later.
 
 Currently KASAN works only with the SLUB memory allocator.
 For better bug detection and nicer report, enable CONFIG_STACKTRACE and put
diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index 4fecaedc..1e1c23e 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -10,8 +10,9 @@ config KASAN
 	help
 	  Enables kernel address sanitizer - runtime memory debugger,
 	  designed to find out-of-bounds accesses and use-after-free bugs.
-	  This is strictly debugging feature. It consumes about 1/8
-	  of available memory and brings about ~x3 performance slowdown.
+	  This is strictly a debugging feature and it requires a gcc version
+	  of 4.9.2 or later.  It consumes about 1/8 of available memory and
+	  brings about ~x3 performance slowdown.
 	  For better error detection enable CONFIG_STACKTRACE,
 	  and add slub_debug=U to boot cmdline.
 
@@ -40,6 +41,7 @@ config KASAN_INLINE
 	  memory accesses. This is faster than outline (in some workloads
 	  it gives about x2 boost over outline instrumentation), but
 	  make kernel's .text size much bigger.
+	  This requires a gcc version of 5.0 or later.
 
 endchoice
 

Re: [PATCH] kasan: Show gcc version requirements in Kconfig and Documentation

[Andrey Ryabinin]

On 04/17/2015 07:44 AM, Joe Perches wrote:
> The documentation shows a need for gcc > 4.9.2, but it's
> really >=.  The Kconfig entries don't show require versions
> so add them.  Correct a latter/later typo too.
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> 
> (dropping Ingo from cc's)
> 
> On Thu, 2015-04-16 at 23:59 -0400, Sasha Levin wrote:
>> On 04/16/2015 11:21 PM, Steven Rostedt wrote:
>>> I have no idea what event caused the issue :-/
>> 4.9.2+ is needed for KASan.
> 
> Perhaps the documentation and Kconfig entries could
> describe that a bit better.
> 

Could you also please mention that gcc 5+ is required to detect out of bounds accesses
to global and stack variables?

Re: [PATCH] kasan: Show gcc version requirements in Kconfig and Documentation

[Joe Perches]

On Fri, 2015-04-17 at 10:54 +0300, Andrey Ryabinin wrote:
> On 04/17/2015 07:44 AM, Joe Perches wrote:
> > The documentation shows a need for gcc > 4.9.2, but it's
> > really >=.  The Kconfig entries don't show require versions
> > so add them.  Correct a latter/later typo too.
[]
> > On Thu, 2015-04-16 at 23:59 -0400, Sasha Levin wrote:
> >> On 04/16/2015 11:21 PM, Steven Rostedt wrote:
> >>> I have no idea what event caused the issue :-/
> >> 4.9.2+ is needed for KASan.
> > 
> > Perhaps the documentation and Kconfig entries could
> > describe that a bit better.
> > 
> 
> Could you also please mention that gcc 5+ is required to detect out of bounds accesses
> to global and stack variables?

You know better than I where and how to describe that.

[PATCH v2] kasan: Show gcc version requirements in Kconfig and Documentation

[Andrey Ryabinin]

From: Joe Perches <joe@perches.com>

The documentation shows a need for gcc > 4.9.2, but it's
really >=.  The Kconfig entries don't show require versions
so add them.  Correct a latter/later typo too.
Also mention that gcc 5 required to catch out of bounds accesses
to global and stack variables.

Signed-off-by: Joe Perches <joe@perches.com>
Signed-off-by: Andrey Ryabinin <a.ryabinin@samsung.com>
---
 Documentation/kasan.txt | 8 +++++---
 lib/Kconfig.kasan       | 8 ++++++--
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/Documentation/kasan.txt b/Documentation/kasan.txt
index 092fc10..4692241 100644
--- a/Documentation/kasan.txt
+++ b/Documentation/kasan.txt
@@ -9,7 +9,9 @@ a fast and comprehensive solution for finding use-after-free and out-of-bounds
 bugs.
 
 KASan uses compile-time instrumentation for checking every memory access,
-therefore you will need a certain version of GCC > 4.9.2
+therefore you will need a gcc version of 4.9.2 or later. KASan could detect out
+of bounds accesses to stack or global variables, but only if gcc 5.0 or later was
+used to built the kernel.
 
 Currently KASan is supported only for x86_64 architecture and requires that the
 kernel be built with the SLUB allocator.
@@ -23,8 +25,8 @@ To enable KASAN configure kernel with:
 
 and choose between CONFIG_KASAN_OUTLINE and CONFIG_KASAN_INLINE. Outline/inline
 is compiler instrumentation types. The former produces smaller binary the
-latter is 1.1 - 2 times faster. Inline instrumentation requires GCC 5.0 or
-latter.
+latter is 1.1 - 2 times faster. Inline instrumentation requires a gcc version
+of 5.0 or later.
 
 Currently KASAN works only with the SLUB memory allocator.
 For better bug detection and nicer report, enable CONFIG_STACKTRACE and put
diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index 4fecaedc..777eda7 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -10,8 +10,11 @@ config KASAN
 	help
 	  Enables kernel address sanitizer - runtime memory debugger,
 	  designed to find out-of-bounds accesses and use-after-free bugs.
-	  This is strictly debugging feature. It consumes about 1/8
-	  of available memory and brings about ~x3 performance slowdown.
+	  This is strictly a debugging feature and it requires a gcc version
+	  of 4.9.2 or later. Detection of out of bounds accesses to stack or
+	  global variables requires gcc 5.0 or later.
+	  This feature consumes about 1/8 of available memory and brings about
+	  ~x3 performance slowdown.
 	  For better error detection enable CONFIG_STACKTRACE,
 	  and add slub_debug=U to boot cmdline.
 
@@ -40,6 +43,7 @@ config KASAN_INLINE
 	  memory accesses. This is faster than outline (in some workloads
 	  it gives about x2 boost over outline instrumentation), but
 	  make kernel's .text size much bigger.
+	  This requires a gcc version of 5.0 or later.
 
 endchoice
 
-- 
2.3.5

Re: [PATCH v2] kasan: Show gcc version requirements in Kconfig and Documentation

[Jonathan Corbet]

On Fri, 17 Apr 2015 19:10:57 +0300
Andrey Ryabinin <a.ryabinin@samsung.com> wrote:

> From: Joe Perches <joe@perches.com>
> 
> The documentation shows a need for gcc > 4.9.2, but it's
> really >=.  The Kconfig entries don't show require versions
> so add them.  Correct a latter/later typo too.
> Also mention that gcc 5 required to catch out of bounds accesses
> to global and stack variables.

Applied to the docs tree, thanks.

jon

Re: [PATCH v2] kasan: Show gcc version requirements in Kconfig and Documentation

[Andrey Ryabinin]

2015-05-07 18:51 GMT+03:00 Jonathan Corbet <corbet@lwn.net>:
> On Fri, 17 Apr 2015 19:10:57 +0300
> Andrey Ryabinin <a.ryabinin@samsung.com> wrote:
>
>> From: Joe Perches <joe@perches.com>
>>
>> The documentation shows a need for gcc > 4.9.2, but it's
>> really >=.  The Kconfig entries don't show require versions
>> so add them.  Correct a latter/later typo too.
>> Also mention that gcc 5 required to catch out of bounds accesses
>> to global and stack variables.
>
> Applied to the docs tree, thanks.
>

This patch already merged in -rc3 via Andrew's tree.

> jon

Re: [PATCH 07/18 v3] tracing: Add TRACE_DEFINE_ENUM() macro to map enums to their values

[Andrey Ryabinin]

On 04/17/2015 06:59 AM, Sasha Levin wrote:
> On 04/16/2015 11:21 PM, Steven Rostedt wrote:
>> On Wed, 15 Apr 2015 20:58:31 -0400
>> Sasha Levin <sasha.levin@oracle.com> wrote:
>>
>>>> On 04/15/2015 10:05 AM, Steven Rostedt wrote:
>>>>>> On Wed, 15 Apr 2015 09:22:37 -0400
>>>>>> Sasha Levin <sasha.levin@oracle.com> wrote:
>>>>>>
>>>>>>>> Hey Steven,
>>>>>>>>
>>>>>>>> I'm seeing the following when booting:
>>>>>>>>
>>>>>>>> [   10.678876] BUG: KASan: out of bounds access in trace_event_enum_update+0xb1d/0xb70 at addr ffffffffa6c4dc68
>>>>>>
>>>>>> Thanks for the report. Mind sending me over your config, and which git
>>>>>> commit was your HEAD. I can't seem to find 7858a62 from your output.
>>>>
>>>> It reproduces on the latest -next kernel. I've attached my config.
>>>>
>> What version of gcc is required to run KASan, the highest version I
>> have to build kernels with is 4.9.0
>>
>> I have no idea what event caused the issue :-/
> 
> 4.9.2+ is needed for KASan.
> 

Actually, detection of out of bounds accesses to stack and global variables works
only with gcc 5+
And this is out-of-bounds access to global variable print_fmt_9p_client_req.

Line numbers in report is ambiguous, so I did following change to reduce compiler's optimization
and get more clear line numbers.

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 7da1dfe..40c31b6 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1726,7 +1726,7 @@ static char *enum_replace(char *ptr, struct trace_enum_map *map, int len)
        return ptr + elen;
 }

-static void update_event_printk(struct ftrace_event_call *call,
+__attribute__((optimize("O0"))) static noinline void update_event_printk(struct ftrace_event_call *call,
                                struct trace_enum_map *map)
 {
        char *ptr



This ended up in this splat (which has more sane line numbers):

[    0.123688] ==================================================================
[    0.123688] BUG: KASan: out of bounds access in update_event_printk+0x6fe/0x720 at addr ffffffff821654a8
[    0.123688] Read of size 1 by task swapper/0/1
[    0.123688] Address belongs to variable print_fmt_9p_client_req+0x848/0x880
[    0.124000] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.0.0+ #269
[    0.124000] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.7.5.1-0-g8936dbb-20141113_115728-nilsson.home.kraxel.org 04/01/2014
[    0.124000]  ffffffff82165720 ffff8801f4d17be8 ffffffff81cc0d35 000000000000003f
[    0.124000]  ffff8801f4d17c80 ffff8801f4d17c68 ffffffff8139cafc ffff8801f4d17c28
[    0.124000]  ffffffff8139c39d 0000000000000292 ffffffff81397c5c ffff8801f4d17c38
[    0.124000] Call Trace:
[    0.124000] dump_stack (lib/dump_stack.c:52)
[    0.124000] kasan_report_error (mm/kasan/report.c:132 mm/kasan/report.c:193)
[    0.124000] ? kasan_slab_alloc (mm/kasan/kasan.c:325)
[    0.124000] ? kmem_cache_alloc (mm/slub.c:2522)
[    0.124000] __asan_report_load1_noabort (mm/kasan/report.c:248)
[    0.124000] ? update_event_printk (kernel/trace/trace_events.c:1736 (discriminator 1))
[    0.124000] update_event_printk (kernel/trace/trace_events.c:1736 (discriminator 1))
[    0.124000] trace_event_enum_update (kernel/trace/trace_events.c:1819)
[    0.124000] ? tracefs_create_file (fs/tracefs/inode.c:413)
[    0.124000] tracer_init_tracefs (kernel/trace/trace.c:4122 kernel/trace/trace.c:6750 kernel/trace/trace.c:6848)
[    0.124000] ? set_tracepoint_printk (kernel/trace/trace.c:6825)
[    0.124000] do_one_initcall (init/main.c:788)
[    0.124000] ? try_to_run_init_process (init/main.c:777)
[    0.124000] ? parse_args (kernel/params.c:218)
[    0.124000] kernel_init_freeable (init/main.c:853 init/main.c:861 init/main.c:880 init/main.c:1001)
[    0.124000] ? __switch_to (arch/x86/include/asm/paravirt.h:25 arch/x86/kernel/process_64.c:410)
[    0.124000] ? rest_init (init/main.c:928)
[    0.124000] kernel_init (init/main.c:933)
[    0.124000] ? rest_init (init/main.c:928)
[    0.124000] ret_from_fork (arch/x86/kernel/entry_64.S:631)
[    0.124000] ? rest_init (init/main.c:928)
[    0.124000] Memory state around the buggy address:
[    0.124000]  ffffffff82165380: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[    0.124000]  ffffffff82165400: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[    0.124000] >ffffffff82165480: 00 00 00 00 00 fa fa fa fa fa fa fa 00 00 00 00
[    0.124000]                                   ^
[    0.124000]  ffffffff82165500: fa fa fa fa 00 00 00 00 fa fa fa fa 00 00 00 00
[    0.124000]  ffffffff82165580: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00 00
[    0.124000] ==================================================================

And kasan is not needed to debug this further.
Just stick WARN_ON(ptr > call->print_fmt + strlen(call->print_fmt)) into the 'for' loop in update_event_printk().



> 
> Thanks,
> Sasha
> 

Re: [PATCH 07/18 v3] tracing: Add TRACE_DEFINE_ENUM() macro to map enums to their values

[Steven Rostedt]

On Fri, 17 Apr 2015 10:44:42 +0300
Andrey Ryabinin <a.ryabinin@samsung.com> wrote:

 
> And kasan is not needed to debug this further.
> Just stick WARN_ON(ptr > call->print_fmt + strlen(call->print_fmt)) into the 'for' loop in update_event_printk().

Thanks, although I think I found the bug with just inspection. I put in
WARN_ON(*ptr) at various locations to see if it triggers.

-- Steve

Re: [PATCH 07/18 v3] tracing: Add TRACE_DEFINE_ENUM() macro to map enums to their values

[Steven Rostedt]

On Fri, 17 Apr 2015 09:29:02 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Fri, 17 Apr 2015 10:44:42 +0300
> Andrey Ryabinin <a.ryabinin@samsung.com> wrote:
> 
>  
> > And kasan is not needed to debug this further.
> > Just stick WARN_ON(ptr > call->print_fmt + strlen(call->print_fmt)) into the 'for' loop in update_event_printk().
> 
> Thanks, although I think I found the bug with just inspection. I put in
> WARN_ON(*ptr) at various locations to see if it triggers.

Yep, that's the issue.

Can you guys test this patch please:

-- Steve

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 39bcfc3f071d..97d454a4dbfb 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1753,6 +1753,8 @@ static void update_event_printk(struct ftrace_event_call *call,
 				ptr++;
 				/* Check for alpha chars like ULL */
 			} while (isalnum(*ptr));
+			if (!*ptr)
+				break;
 			/*
 			 * A number must have some kind of delimiter after
 			 * it, and we can ignore that too.
@@ -1779,12 +1781,16 @@ static void update_event_printk(struct ftrace_event_call *call,
 			do {
 				ptr++;
 			} while (isalnum(*ptr) || *ptr == '_');
+			if (!*ptr)
+				break;
 			/*
 			 * If what comes after this variable is a '.' or
 			 * '->' then we can continue to ignore that string.
 			 */
 			if (*ptr == '.' || (ptr[0] == '-' && ptr[1] == '>')) {
 				ptr += *ptr == '.' ? 1 : 2;
+				if (!*ptr)
+					break;
 				goto skip_more;
 			}
 			/*

Re: [PATCH 07/18 v3] tracing: Add TRACE_DEFINE_ENUM() macro to map enums to their values

[Andrey Ryabinin]

On 04/17/2015 04:51 PM, Steven Rostedt wrote:
> On Fri, 17 Apr 2015 09:29:02 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
>> On Fri, 17 Apr 2015 10:44:42 +0300
>> Andrey Ryabinin <a.ryabinin@samsung.com> wrote:
>>
>>  
>>> And kasan is not needed to debug this further.
>>> Just stick WARN_ON(ptr > call->print_fmt + strlen(call->print_fmt)) into the 'for' loop in update_event_printk().
>>
>> Thanks, although I think I found the bug with just inspection. I put in
>> WARN_ON(*ptr) at various locations to see if it triggers.
> 
> Yep, that's the issue.
> 
> Can you guys test this patch please:
> 

Tested-by: Andrey Ryabinin <a.ryabinin@samsung.com>

> -- Steve
> 
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 39bcfc3f071d..97d454a4dbfb 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -1753,6 +1753,8 @@ static void update_event_printk(struct ftrace_event_call *call,
>  				ptr++;
>  				/* Check for alpha chars like ULL */
>  			} while (isalnum(*ptr));
> +			if (!*ptr)
> +				break;
>  			/*
>  			 * A number must have some kind of delimiter after
>  			 * it, and we can ignore that too.
> @@ -1779,12 +1781,16 @@ static void update_event_printk(struct ftrace_event_call *call,
>  			do {
>  				ptr++;
>  			} while (isalnum(*ptr) || *ptr == '_');
> +			if (!*ptr)
> +				break;
>  			/*
>  			 * If what comes after this variable is a '.' or
>  			 * '->' then we can continue to ignore that string.
>  			 */
>  			if (*ptr == '.' || (ptr[0] == '-' && ptr[1] == '>')) {
>  				ptr += *ptr == '.' ? 1 : 2;
> +				if (!*ptr)
> +					break;
>  				goto skip_more;
>  			}
>  			/*
> 

[PATCH 08/18 v3] tracing: Allow for modules to convert their enums to values

[Steven Rostedt]

[-- Attachment #1: 0008-tracing-Allow-for-modules-to-convert-their-enums-to-.patch --]
[-- Type: text/plain, Size: 4148 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

Update the infrastructure such that modules that declare TRACE_DEFINE_ENUM()
will have those enums converted into their values in the tracepoint
print fmt strings.

Link: http://lkml.kernel.org/r/87vbhjp74q.fsf@rustcorp.com.au

Acked-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/linux/module.h      |  2 ++
 kernel/module.c             |  3 +++
 kernel/trace/trace.c        | 49 +++++++++++++++++++++++++++++++++++++++++----
 kernel/trace/trace_events.c |  2 +-
 4 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index 42999fe2dbd0..53dc41dd5c62 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -338,6 +338,8 @@ struct module {
 #ifdef CONFIG_EVENT_TRACING
 	struct ftrace_event_call **trace_events;
 	unsigned int num_trace_events;
+	struct trace_enum_map **trace_enums;
+	unsigned int num_trace_enums;
 #endif
 #ifdef CONFIG_FTRACE_MCOUNT_RECORD
 	unsigned int num_ftrace_callsites;
diff --git a/kernel/module.c b/kernel/module.c
index b3d634ed06c9..d8f8ab271c2b 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2753,6 +2753,9 @@ static int find_module_sections(struct module *mod, struct load_info *info)
 	mod->trace_events = section_objs(info, "_ftrace_events",
 					 sizeof(*mod->trace_events),
 					 &mod->num_trace_events);
+	mod->trace_enums = section_objs(info, "_ftrace_enum_map",
+					sizeof(*mod->trace_enums),
+					&mod->num_trace_enums);
 #endif
 #ifdef CONFIG_TRACING
 	mod->trace_bprintk_fmt_start = section_objs(info, "__trace_printk_fmt",
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index ebf49649534c..28e6654e640d 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -3908,11 +3908,9 @@ static const struct file_operations tracing_saved_cmdlines_size_fops = {
 	.write		= tracing_saved_cmdlines_size_write,
 };
 
-static void
-trace_insert_enum_map(struct trace_enum_map **start, struct trace_enum_map **stop)
+static void trace_insert_enum_map(struct trace_enum_map **start, int len)
 {
 	struct trace_enum_map **map;
-	int len = stop - start;
 
 	if (len <= 0)
 		return;
@@ -6561,9 +6559,48 @@ extern struct trace_enum_map *__stop_ftrace_enum_maps[];
 
 static void __init trace_enum_init(void)
 {
-	trace_insert_enum_map(__start_ftrace_enum_maps, __stop_ftrace_enum_maps);
+	int len;
+
+	len = __stop_ftrace_enum_maps - __start_ftrace_enum_maps;
+	trace_insert_enum_map(__start_ftrace_enum_maps, len);
+}
+
+#ifdef CONFIG_MODULES
+static void trace_module_add_enums(struct module *mod)
+{
+	if (!mod->num_trace_enums)
+		return;
+
+	/*
+	 * Modules with bad taint do not have events created, do
+	 * not bother with enums either.
+	 */
+	if (trace_module_has_bad_taint(mod))
+		return;
+
+	trace_insert_enum_map(mod->trace_enums, mod->num_trace_enums);
+}
+
+static int trace_module_notify(struct notifier_block *self,
+			       unsigned long val, void *data)
+{
+	struct module *mod = data;
+
+	switch (val) {
+	case MODULE_STATE_COMING:
+		trace_module_add_enums(mod);
+		break;
+	}
+
+	return 0;
 }
 
+static struct notifier_block trace_module_nb = {
+	.notifier_call = trace_module_notify,
+	.priority = 0,
+};
+#endif
+
 static __init int tracer_init_debugfs(void)
 {
 	struct dentry *d_tracer;
@@ -6590,6 +6627,10 @@ static __init int tracer_init_debugfs(void)
 
 	trace_enum_init();
 
+#ifdef CONFIG_MODULES
+	register_module_notifier(&trace_module_nb);
+#endif
+
 #ifdef CONFIG_DYNAMIC_FTRACE
 	trace_create_file("dyn_ftrace_total_info", 0444, d_tracer,
 			&ftrace_update_tot_cnt, &tracing_dyn_info_fops);
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index fc58c50fbf01..a576bbe75577 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -2034,7 +2034,7 @@ static int trace_module_notify(struct notifier_block *self,
 
 static struct notifier_block trace_module_nb = {
 	.notifier_call = trace_module_notify,
-	.priority = 0,
+	.priority = 1, /* higher than trace.c module notify */
 };
 #endif /* CONFIG_MODULES */
 
-- 
2.1.4

[PATCH 09/18 v3] tracing/samples: Update the trace-event-sample.h with TRACE_DEFINE_ENUM()

[Steven Rostedt]

[-- Attachment #1: 0009-tracing-samples-Update-the-trace-event-sample.h-with.patch --]
[-- Type: text/plain, Size: 3443 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

Document the use of TRACE_DEFINE_ENUM() by adding enums to the
trace-event-sample.h and using this macro to convert them in the format
files.

Also update the comments and sho the use of __print_symbolic() and
__print_flags() as well as adding comments abount __print_array().

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 samples/trace_events/trace-events-sample.h | 64 +++++++++++++++++++++++++++++-
 1 file changed, 63 insertions(+), 1 deletion(-)

diff --git a/samples/trace_events/trace-events-sample.h b/samples/trace_events/trace-events-sample.h
index 19405f18cc8a..8965d1bb8811 100644
--- a/samples/trace_events/trace-events-sample.h
+++ b/samples/trace_events/trace-events-sample.h
@@ -198,8 +198,30 @@ static inline int __length_of(const int *list)
 		;
 	return i;
 }
+
+enum {
+	TRACE_SAMPLE_FOO = 2,
+	TRACE_SAMPLE_BAR = 4,
+	TRACE_SAMPLE_ZOO = 8,
+};
 #endif
 
+/*
+ * If enums are used in the TP_printk(), their names will be shown in
+ * format files and not their values. This can cause problems with user
+ * space programs that parse the format files to know how to translate
+ * the raw binary trace output into human readable text.
+ *
+ * To help out user space programs, any enum that is used in the TP_printk()
+ * should be defined by TRACE_DEFINE_ENUM() macro. All that is needed to
+ * be done is to add this macro with the enum within it in the trace
+ * header file, and it will be converted in the output.
+ */
+
+TRACE_DEFINE_ENUM(TRACE_SAMPLE_FOO);
+TRACE_DEFINE_ENUM(TRACE_SAMPLE_BAR);
+TRACE_DEFINE_ENUM(TRACE_SAMPLE_ZOO);
+
 TRACE_EVENT(foo_bar,
 
 	TP_PROTO(const char *foo, int bar, const int *lst,
@@ -224,7 +246,47 @@ TRACE_EVENT(foo_bar,
 		__assign_bitmask(cpus, cpumask_bits(mask), num_possible_cpus());
 	),
 
-	TP_printk("foo %s %d %s %s (%s)", __entry->foo, __entry->bar,
+	TP_printk("foo %s %d %s %s %s %s (%s)", __entry->foo, __entry->bar,
+
+/*
+ * Notice here the use of some helper functions. This includes:
+ *
+ *  __print_symbolic( variable, { value, "string" }, ... ),
+ *
+ *    The variable is tested against each value of the { } pair. If
+ *    the variable matches one of the values, then it will print the
+ *    string in that pair. If non are matched, it returns a string
+ *    version of the number (if __entry->bar == 7 then "7" is returned).
+ */
+		  __print_symbolic(__entry->bar,
+				   { 0, "zero" },
+				   { TRACE_SAMPLE_FOO, "TWO" },
+				   { TRACE_SAMPLE_BAR, "FOUR" },
+				   { TRACE_SAMPLE_ZOO, "EIGHT" },
+				   { 10, "TEN" }
+			  ),
+
+/*
+ *  __print_flags( variable, "delim", { value, "flag" }, ... ),
+ *
+ *    This is similar to __print_symbolic, except that it tests the bits
+ *    of the value. If ((FLAG & variable) == FLAG) then the string is
+ *    printed. If more than one flag matches, then each one that does is
+ *    also printed with delim in between them.
+ *    If not all bits are accounted for, then the not found bits will be
+ *    added in hex format: 0x506 will show BIT2|BIT4|0x500
+ */
+		  __print_flags(__entry->bar, "|",
+				{ 1, "BIT1" },
+				{ 2, "BIT2" },
+				{ 4, "BIT3" },
+				{ 8, "BIT4" }
+			  ),
+/*
+ *  __print_array( array, len, element_size )
+ *
+ *    This prints out the array that is defined by __array in a nice format.
+ */
 		  __print_array(__get_dynamic_array(list),
 				__get_dynamic_array_len(list),
 				sizeof(int)),
-- 
2.1.4

[PATCH 10/18 v3] x86/tlb/trace: Export enums in used by tlb_flush tracepoint

[Steven Rostedt]

[-- Attachment #1: 0010-x86-tlb-trace-Export-enums-in-used-by-tlb_flush-trac.patch --]
[-- Type: text/plain, Size: 1645 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

Have the enums used in __print_symbolic() by the trace_tlb_flush()
tracepoint exported to userpace such that they can be parsed by
userspace tools.

Cc: Dave Hansen <dave@sr71.net>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/trace/events/tlb.h | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/include/trace/events/tlb.h b/include/trace/events/tlb.h
index 0e7635765153..4250f364a6ca 100644
--- a/include/trace/events/tlb.h
+++ b/include/trace/events/tlb.h
@@ -7,11 +7,31 @@
 #include <linux/mm_types.h>
 #include <linux/tracepoint.h>
 
-#define TLB_FLUSH_REASON	\
-	{ TLB_FLUSH_ON_TASK_SWITCH,	"flush on task switch" },	\
-	{ TLB_REMOTE_SHOOTDOWN,		"remote shootdown" },		\
-	{ TLB_LOCAL_SHOOTDOWN,		"local shootdown" },		\
-	{ TLB_LOCAL_MM_SHOOTDOWN,	"local mm shootdown" }
+#define TLB_FLUSH_REASON						\
+	EM(  TLB_FLUSH_ON_TASK_SWITCH,	"flush on task switch" )	\
+	EM(  TLB_REMOTE_SHOOTDOWN,	"remote shootdown" )		\
+	EM(  TLB_LOCAL_SHOOTDOWN,	"local shootdown" )		\
+	EMe( TLB_LOCAL_MM_SHOOTDOWN,	"local mm shootdown" )
+
+/*
+ * First define the enums in TLB_FLUSH_REASON to be exported to userspace
+ * via TRACE_DEFINE_ENUM().
+ */
+#undef EM
+#undef EMe
+#define EM(a,b)		TRACE_DEFINE_ENUM(a);
+#define EMe(a,b)	TRACE_DEFINE_ENUM(a);
+
+TLB_FLUSH_REASON
+
+/*
+ * Now redefine the EM() and EMe() macros to map the enums to the strings
+ * that will be printed in the output.
+ */
+#undef EM
+#undef EMe
+#define EM(a,b)		{ a, b },
+#define EMe(a,b)	{ a, b }
 
 TRACE_EVENT_CONDITION(tlb_flush,
 
-- 
2.1.4

[PATCH 11/18 v3] net/9p/tracing: Export enums in tracepoints to userspace

[Steven Rostedt]

[-- Attachment #1: 0011-net-9p-tracing-Export-enums-in-tracepoints-to-usersp.patch --]
[-- Type: text/plain, Size: 6698 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

The tracepoints in the 9p code use a lot of enums for the __print_symbolic()
function. These enums are shown in the tracepoint format files, and user
space tools such as trace-cmd does not have the information to parse it.
Add helper macros to export the enums with TRACE_DEFINE_ENUM().

Cc: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Cc: Eric Van Hensbergen <ericvh@gmail.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/trace/events/9p.h | 157 ++++++++++++++++++++++++++--------------------
 1 file changed, 88 insertions(+), 69 deletions(-)

diff --git a/include/trace/events/9p.h b/include/trace/events/9p.h
index a0666362c111..633ee9ee9778 100644
--- a/include/trace/events/9p.h
+++ b/include/trace/events/9p.h
@@ -6,76 +6,95 @@
 
 #include <linux/tracepoint.h>
 
+#define P9_MSG_T							\
+		EM( P9_TLERROR,		"P9_TLERROR" )			\
+		EM( P9_RLERROR,		"P9_RLERROR" )			\
+		EM( P9_TSTATFS,		"P9_TSTATFS" )			\
+		EM( P9_RSTATFS,		"P9_RSTATFS" )			\
+		EM( P9_TLOPEN,		"P9_TLOPEN" )			\
+		EM( P9_RLOPEN,		"P9_RLOPEN" )			\
+		EM( P9_TLCREATE,	"P9_TLCREATE" )			\
+		EM( P9_RLCREATE,	"P9_RLCREATE" )			\
+		EM( P9_TSYMLINK,	"P9_TSYMLINK" )			\
+		EM( P9_RSYMLINK,	"P9_RSYMLINK" )			\
+		EM( P9_TMKNOD,		"P9_TMKNOD" )			\
+		EM( P9_RMKNOD,		"P9_RMKNOD" )			\
+		EM( P9_TRENAME,		"P9_TRENAME" )			\
+		EM( P9_RRENAME,		"P9_RRENAME" )			\
+		EM( P9_TREADLINK,	"P9_TREADLINK" )		\
+		EM( P9_RREADLINK,	"P9_RREADLINK" )		\
+		EM( P9_TGETATTR,	"P9_TGETATTR" )			\
+		EM( P9_RGETATTR,	"P9_RGETATTR" )			\
+		EM( P9_TSETATTR,	"P9_TSETATTR" )			\
+		EM( P9_RSETATTR,	"P9_RSETATTR" )			\
+		EM( P9_TXATTRWALK,	"P9_TXATTRWALK" )		\
+		EM( P9_RXATTRWALK,	"P9_RXATTRWALK" )		\
+		EM( P9_TXATTRCREATE,	"P9_TXATTRCREATE" )		\
+		EM( P9_RXATTRCREATE,	"P9_RXATTRCREATE" )		\
+		EM( P9_TREADDIR,	"P9_TREADDIR" )			\
+		EM( P9_RREADDIR,	"P9_RREADDIR" )			\
+		EM( P9_TFSYNC,		"P9_TFSYNC" )			\
+		EM( P9_RFSYNC,		"P9_RFSYNC" )			\
+		EM( P9_TLOCK,		"P9_TLOCK" )			\
+		EM( P9_RLOCK,		"P9_RLOCK" )			\
+		EM( P9_TGETLOCK,	"P9_TGETLOCK" )			\
+		EM( P9_RGETLOCK,	"P9_RGETLOCK" )			\
+		EM( P9_TLINK,		"P9_TLINK" )			\
+		EM( P9_RLINK,		"P9_RLINK" )			\
+		EM( P9_TMKDIR,		"P9_TMKDIR" )			\
+		EM( P9_RMKDIR,		"P9_RMKDIR" )			\
+		EM( P9_TRENAMEAT,	"P9_TRENAMEAT" )		\
+		EM( P9_RRENAMEAT,	"P9_RRENAMEAT" )		\
+		EM( P9_TUNLINKAT,	"P9_TUNLINKAT" )		\
+		EM( P9_RUNLINKAT,	"P9_RUNLINKAT" )		\
+		EM( P9_TVERSION,	"P9_TVERSION" )			\
+		EM( P9_RVERSION,	"P9_RVERSION" )			\
+		EM( P9_TAUTH,		"P9_TAUTH" )			\
+		EM( P9_RAUTH,		"P9_RAUTH" )			\
+		EM( P9_TATTACH,		"P9_TATTACH" )			\
+		EM( P9_RATTACH,		"P9_RATTACH" )			\
+		EM( P9_TERROR,		"P9_TERROR" )			\
+		EM( P9_RERROR,		"P9_RERROR" )			\
+		EM( P9_TFLUSH,		"P9_TFLUSH" )			\
+		EM( P9_RFLUSH,		"P9_RFLUSH" )			\
+		EM( P9_TWALK,		"P9_TWALK" )			\
+		EM( P9_RWALK,		"P9_RWALK" )			\
+		EM( P9_TOPEN,		"P9_TOPEN" )			\
+		EM( P9_ROPEN,		"P9_ROPEN" )			\
+		EM( P9_TCREATE,		"P9_TCREATE" )			\
+		EM( P9_RCREATE,		"P9_RCREATE" )			\
+		EM( P9_TREAD,		"P9_TREAD" )			\
+		EM( P9_RREAD,		"P9_RREAD" )			\
+		EM( P9_TWRITE,		"P9_TWRITE" )			\
+		EM( P9_RWRITE,		"P9_RWRITE" )			\
+		EM( P9_TCLUNK,		"P9_TCLUNK" )			\
+		EM( P9_RCLUNK,		"P9_RCLUNK" )			\
+		EM( P9_TREMOVE,		"P9_TREMOVE" )			\
+		EM( P9_RREMOVE,		"P9_RREMOVE" )			\
+		EM( P9_TSTAT,		"P9_TSTAT" )			\
+		EM( P9_RSTAT,		"P9_RSTAT" )			\
+		EM( P9_TWSTAT,		"P9_TWSTAT" )			\
+		EMe(P9_RWSTAT,		"P9_RWSTAT" )
+
+/* Define EM() to export the enums to userspace via TRACE_DEFINE_ENUM() */
+#undef EM
+#undef EMe
+#define EM(a, b)	TRACE_DEFINE_ENUM(a);
+#define EMe(a, b)	TRACE_DEFINE_ENUM(a);
+
+P9_MSG_T
+
+/*
+ * Now redefine the EM() and EMe() macros to map the enums to the strings
+ * that will be printed in the output.
+ */
+#undef EM
+#undef EMe
+#define EM(a, b)	{ a, b },
+#define EMe(a, b)	{ a, b }
+
 #define show_9p_op(type)						\
-	__print_symbolic(type,						\
-			 { P9_TLERROR,		"P9_TLERROR" },		\
-			 { P9_RLERROR,		"P9_RLERROR" },		\
-			 { P9_TSTATFS,		"P9_TSTATFS" },		\
-			 { P9_RSTATFS,		"P9_RSTATFS" },		\
-			 { P9_TLOPEN,		"P9_TLOPEN" },		\
-			 { P9_RLOPEN,		"P9_RLOPEN" },		\
-			 { P9_TLCREATE,		"P9_TLCREATE" },	\
-			 { P9_RLCREATE,		"P9_RLCREATE" },	\
-			 { P9_TSYMLINK,		"P9_TSYMLINK" },	\
-			 { P9_RSYMLINK,		"P9_RSYMLINK" },	\
-			 { P9_TMKNOD,		"P9_TMKNOD" },		\
-			 { P9_RMKNOD,		"P9_RMKNOD" },		\
-			 { P9_TRENAME,		"P9_TRENAME" },		\
-			 { P9_RRENAME,		"P9_RRENAME" },		\
-			 { P9_TREADLINK,	"P9_TREADLINK" },	\
-			 { P9_RREADLINK,	"P9_RREADLINK" },	\
-			 { P9_TGETATTR,		"P9_TGETATTR" },	\
-			 { P9_RGETATTR,		"P9_RGETATTR" },	\
-			 { P9_TSETATTR,		"P9_TSETATTR" },	\
-			 { P9_RSETATTR,		"P9_RSETATTR" },	\
-			 { P9_TXATTRWALK,	"P9_TXATTRWALK" },	\
-			 { P9_RXATTRWALK,	"P9_RXATTRWALK" },	\
-			 { P9_TXATTRCREATE,	"P9_TXATTRCREATE" },	\
-			 { P9_RXATTRCREATE,	"P9_RXATTRCREATE" },	\
-			 { P9_TREADDIR,		"P9_TREADDIR" },	\
-			 { P9_RREADDIR,		"P9_RREADDIR" },	\
-			 { P9_TFSYNC,		"P9_TFSYNC" },		\
-			 { P9_RFSYNC,		"P9_RFSYNC" },		\
-			 { P9_TLOCK,		"P9_TLOCK" },		\
-			 { P9_RLOCK,		"P9_RLOCK" },		\
-			 { P9_TGETLOCK,		"P9_TGETLOCK" },	\
-			 { P9_RGETLOCK,		"P9_RGETLOCK" },	\
-			 { P9_TLINK,		"P9_TLINK" },		\
-			 { P9_RLINK,		"P9_RLINK" },		\
-			 { P9_TMKDIR,		"P9_TMKDIR" },		\
-			 { P9_RMKDIR,		"P9_RMKDIR" },		\
-			 { P9_TRENAMEAT,	"P9_TRENAMEAT" },	\
-			 { P9_RRENAMEAT,	"P9_RRENAMEAT" },	\
-			 { P9_TUNLINKAT,	"P9_TUNLINKAT" },	\
-			 { P9_RUNLINKAT,	"P9_RUNLINKAT" },	\
-			 { P9_TVERSION,		"P9_TVERSION" },	\
-			 { P9_RVERSION,		"P9_RVERSION" },	\
-			 { P9_TAUTH,		"P9_TAUTH" },		\
-			 { P9_RAUTH,		"P9_RAUTH" },		\
-			 { P9_TATTACH,		"P9_TATTACH" },		\
-			 { P9_RATTACH,		"P9_RATTACH" },		\
-			 { P9_TERROR,		"P9_TERROR" },		\
-			 { P9_RERROR,		"P9_RERROR" },		\
-			 { P9_TFLUSH,		"P9_TFLUSH" },		\
-			 { P9_RFLUSH,		"P9_RFLUSH" },		\
-			 { P9_TWALK,		"P9_TWALK" },		\
-			 { P9_RWALK,		"P9_RWALK" },		\
-			 { P9_TOPEN,		"P9_TOPEN" },		\
-			 { P9_ROPEN,		"P9_ROPEN" },		\
-			 { P9_TCREATE,		"P9_TCREATE" },		\
-			 { P9_RCREATE,		"P9_RCREATE" },		\
-			 { P9_TREAD,		"P9_TREAD" },		\
-			 { P9_RREAD,		"P9_RREAD" },		\
-			 { P9_TWRITE,		"P9_TWRITE" },		\
-			 { P9_RWRITE,		"P9_RWRITE" },		\
-			 { P9_TCLUNK,		"P9_TCLUNK" },		\
-			 { P9_RCLUNK,		"P9_RCLUNK" },		\
-			 { P9_TREMOVE,		"P9_TREMOVE" },		\
-			 { P9_RREMOVE,		"P9_RREMOVE" },		\
-			 { P9_TSTAT,		"P9_TSTAT" },		\
-			 { P9_RSTAT,		"P9_RSTAT" },		\
-			 { P9_TWSTAT,		"P9_TWSTAT" },		\
-			 { P9_RWSTAT,		"P9_RWSTAT" })
+	__print_symbolic(type, P9_MSG_T)
 
 TRACE_EVENT(9p_client_req,
 	    TP_PROTO(struct p9_client *clnt, int8_t type, int tag),
-- 
2.1.4

[PATCH 12/18 v3] f2fs: Export the enums in the tracepoints to userspace

[Steven Rostedt]

[-- Attachment #1: 0012-f2fs-Export-the-enums-in-the-tracepoints-to-userspac.patch --]
[-- Type: text/plain, Size: 2099 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

The tracepoints that use __print_symbolic() use enums as the value
to convert to strings. Unfortunately, the format files for these
tracepoints show the enum name and not their value. This causes some
userspace tools not to know how to convert __print_symbolic() to
their strings.

Add TRACE_DEFINE_ENUM() macros to export the enums used to userspace
to let those tools know what those enum values are.

Cc: Namjae Jeon <namjae.jeon@samsung.com>
Cc: Pankaj Kumar <pankaj.km@samsung.com>
Cc: Jaegeuk Kim <jaegeuk@kernel.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/trace/events/f2fs.h | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
index 5422dbfaf97d..36f4536b6149 100644
--- a/include/trace/events/f2fs.h
+++ b/include/trace/events/f2fs.h
@@ -9,6 +9,36 @@
 #define show_dev(entry)		MAJOR(entry->dev), MINOR(entry->dev)
 #define show_dev_ino(entry)	show_dev(entry), (unsigned long)entry->ino
 
+TRACE_DEFINE_ENUM(NODE);
+TRACE_DEFINE_ENUM(DATA);
+TRACE_DEFINE_ENUM(META);
+TRACE_DEFINE_ENUM(META_FLUSH);
+TRACE_DEFINE_ENUM(CURSEG_HOT_DATA);
+TRACE_DEFINE_ENUM(CURSEG_WARM_DATA);
+TRACE_DEFINE_ENUM(CURSEG_COLD_DATA);
+TRACE_DEFINE_ENUM(CURSEG_HOT_NODE);
+TRACE_DEFINE_ENUM(CURSEG_WARM_NODE);
+TRACE_DEFINE_ENUM(CURSEG_COLD_NODE);
+TRACE_DEFINE_ENUM(NO_CHECK_TYPE);
+TRACE_DEFINE_ENUM(GC_GREEDY);
+TRACE_DEFINE_ENUM(GC_CB);
+TRACE_DEFINE_ENUM(FG_GC);
+TRACE_DEFINE_ENUM(BG_GC);
+TRACE_DEFINE_ENUM(LFS);
+TRACE_DEFINE_ENUM(SSR);
+TRACE_DEFINE_ENUM(__REQ_RAHEAD);
+TRACE_DEFINE_ENUM(__REQ_WRITE);
+TRACE_DEFINE_ENUM(__REQ_SYNC);
+TRACE_DEFINE_ENUM(__REQ_NOIDLE);
+TRACE_DEFINE_ENUM(__REQ_FLUSH);
+TRACE_DEFINE_ENUM(__REQ_FUA);
+TRACE_DEFINE_ENUM(__REQ_PRIO);
+TRACE_DEFINE_ENUM(__REQ_META);
+TRACE_DEFINE_ENUM(CP_UMOUNT);
+TRACE_DEFINE_ENUM(CP_FASTBOOT);
+TRACE_DEFINE_ENUM(CP_SYNC);
+TRACE_DEFINE_ENUM(CP_DISCARD);
+
 #define show_block_type(type)						\
 	__print_symbolic(type,						\
 		{ NODE,		"NODE" },				\
-- 
2.1.4

[PATCH 13/18 v3] irq/tracing: Export enums in tracepoints to user space

[Steven Rostedt]

[-- Attachment #1: 0013-irq-tracing-Export-enums-in-tracepoints-to-user-spac.patch --]
[-- Type: text/plain, Size: 2074 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

The enums used by the softirq mapping is what is shown in the output
of the __print_symbolic() and not their values, that are needed
to map them to their strings. Export them to userspace with the
TRACE_DEFINE_ENUM() macro so that user space tools can map the enums
with their values.

Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/trace/events/irq.h | 39 +++++++++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/include/trace/events/irq.h b/include/trace/events/irq.h
index 3608bebd3d9c..ff8f6c091a15 100644
--- a/include/trace/events/irq.h
+++ b/include/trace/events/irq.h
@@ -9,19 +9,34 @@
 struct irqaction;
 struct softirq_action;
 
-#define softirq_name(sirq) { sirq##_SOFTIRQ, #sirq }
+#define SOFTIRQ_NAME_LIST				\
+			 softirq_name(HI)		\
+			 softirq_name(TIMER)		\
+			 softirq_name(NET_TX)		\
+			 softirq_name(NET_RX)		\
+			 softirq_name(BLOCK)		\
+			 softirq_name(BLOCK_IOPOLL)	\
+			 softirq_name(TASKLET)		\
+			 softirq_name(SCHED)		\
+			 softirq_name(HRTIMER)		\
+			 softirq_name_end(RCU)
+
+#undef softirq_name
+#undef softirq_name_end
+
+#define softirq_name(sirq) TRACE_DEFINE_ENUM(sirq##_SOFTIRQ);
+#define softirq_name_end(sirq)  TRACE_DEFINE_ENUM(sirq##_SOFTIRQ);
+
+SOFTIRQ_NAME_LIST
+
+#undef softirq_name
+#undef softirq_name_end
+
+#define softirq_name(sirq) { sirq##_SOFTIRQ, #sirq },
+#define softirq_name_end(sirq) { sirq##_SOFTIRQ, #sirq }
+
 #define show_softirq_name(val)				\
-	__print_symbolic(val,				\
-			 softirq_name(HI),		\
-			 softirq_name(TIMER),		\
-			 softirq_name(NET_TX),		\
-			 softirq_name(NET_RX),		\
-			 softirq_name(BLOCK),		\
-			 softirq_name(BLOCK_IOPOLL),	\
-			 softirq_name(TASKLET),		\
-			 softirq_name(SCHED),		\
-			 softirq_name(HRTIMER),		\
-			 softirq_name(RCU))
+	__print_symbolic(val, SOFTIRQ_NAME_LIST)
 
 /**
  * irq_handler_entry - called immediately before the irq action handler
-- 
2.1.4

[PATCH 14/18 v3] mm: tracing: Export enums in tracepoints to user space

[Steven Rostedt]

[-- Attachment #1: 0014-mm-tracing-Export-enums-in-tracepoints-to-user-space.patch --]
[-- Type: text/plain, Size: 2260 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

The enums used in tracepoints with __print_symbolic() have their
names shown in the tracepoint format files and not their values.
This makes it difficult for user space tools to convert the binary
data to the strings as user space does not know what those enums
are about.

By having them use TRACE_DEFINE_ENUM(), the names of the enums will
be mapped to the values and shown to user space.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/trace/events/migrate.h | 42 ++++++++++++++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 10 deletions(-)

diff --git a/include/trace/events/migrate.h b/include/trace/events/migrate.h
index dd2b5467d905..539b25a76111 100644
--- a/include/trace/events/migrate.h
+++ b/include/trace/events/migrate.h
@@ -7,18 +7,40 @@
 #include <linux/tracepoint.h>
 
 #define MIGRATE_MODE						\
-	{MIGRATE_ASYNC,		"MIGRATE_ASYNC"},		\
-	{MIGRATE_SYNC_LIGHT,	"MIGRATE_SYNC_LIGHT"},		\
-	{MIGRATE_SYNC,		"MIGRATE_SYNC"}		
+	EM( MIGRATE_ASYNC,	"MIGRATE_ASYNC")		\
+	EM( MIGRATE_SYNC_LIGHT,	"MIGRATE_SYNC_LIGHT")		\
+	EMe(MIGRATE_SYNC,	"MIGRATE_SYNC")
+
 
 #define MIGRATE_REASON						\
-	{MR_COMPACTION,		"compaction"},			\
-	{MR_MEMORY_FAILURE,	"memory_failure"},		\
-	{MR_MEMORY_HOTPLUG,	"memory_hotplug"},		\
-	{MR_SYSCALL,		"syscall_or_cpuset"},		\
-	{MR_MEMPOLICY_MBIND,	"mempolicy_mbind"},		\
-	{MR_NUMA_MISPLACED,	"numa_misplaced"},		\
-	{MR_CMA,		"cma"}
+	EM( MR_COMPACTION,	"compaction")			\
+	EM( MR_MEMORY_FAILURE,	"memory_failure")		\
+	EM( MR_MEMORY_HOTPLUG,	"memory_hotplug")		\
+	EM( MR_SYSCALL,		"syscall_or_cpuset")		\
+	EM( MR_MEMPOLICY_MBIND,	"mempolicy_mbind")		\
+	EM( MR_NUMA_MISPLACED,	"numa_misplaced")		\
+	EMe(MR_CMA,		"cma")
+
+/*
+ * First define the enums in the above macros to be exported to userspace
+ * via TRACE_DEFINE_ENUM().
+ */
+#undef EM
+#undef EMe
+#define EM(a, b)	TRACE_DEFINE_ENUM(a);
+#define EMe(a, b)	TRACE_DEFINE_ENUM(a);
+
+MIGRATE_MODE
+MIGRATE_REASON
+
+/*
+ * Now redefine the EM() and EMe() macros to map the enums to the strings
+ * that will be printed in the output.
+ */
+#undef EM
+#undef EMe
+#define EM(a, b)	{a, b},
+#define EMe(a, b)	{a, b}
 
 TRACE_EVENT(mm_migrate_pages,
 
-- 
2.1.4

[PATCH 15/18 v3] SUNRPC: Export enums in tracepoints to user space

[Steven Rostedt]

[-- Attachment #1: 0015-SUNRPC-Export-enums-in-tracepoints-to-user-space.patch --]
[-- Type: text/plain, Size: 2834 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

The enums used in the tracepoints for __print_symbolic() have their
names shown in the tracepoint format files. User space tools do not know
how to convert those names into their values to be able to convert the
binary data.

Use TRACE_DEFINE_ENUM() to export the enum names to their values for
userspace to do the parsing correctly.

Cc: Trond Myklebust <Trond.Myklebust@netapp.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/trace/events/sunrpc.h | 62 ++++++++++++++++++++++++++++++-------------
 1 file changed, 44 insertions(+), 18 deletions(-)

diff --git a/include/trace/events/sunrpc.h b/include/trace/events/sunrpc.h
index b9c1dc6c825a..fd1a02cb3c82 100644
--- a/include/trace/events/sunrpc.h
+++ b/include/trace/events/sunrpc.h
@@ -179,27 +179,53 @@ DEFINE_EVENT(rpc_task_queued, rpc_task_wakeup,
 
 );
 
+/*
+ * First define the enums in the below macros to be exported to userspace
+ * via TRACE_DEFINE_ENUM().
+ */
+#undef EM
+#undef EMe
+#define EM(a, b)	TRACE_DEFINE_ENUM(a);
+#define EMe(a, b)	TRACE_DEFINE_ENUM(a);
+
+#define RPC_SHOW_SOCKET				\
+	EM( SS_FREE, "FREE" )			\
+	EM( SS_UNCONNECTED, "UNCONNECTED" )	\
+	EM( SS_CONNECTING, "CONNECTING," )	\
+	EM( SS_CONNECTED, "CONNECTED," )	\
+	EMe(SS_DISCONNECTING, "DISCONNECTING" )
+
 #define rpc_show_socket_state(state) \
-	__print_symbolic(state, \
-		{ SS_FREE, "FREE" }, \
-		{ SS_UNCONNECTED, "UNCONNECTED" }, \
-		{ SS_CONNECTING, "CONNECTING," }, \
-		{ SS_CONNECTED, "CONNECTED," }, \
-		{ SS_DISCONNECTING, "DISCONNECTING" })
+	__print_symbolic(state, RPC_SHOW_SOCKET)
+
+RPC_SHOW_SOCKET
+
+#define RPC_SHOW_SOCK				\
+	EM( TCP_ESTABLISHED, "ESTABLISHED" )	\
+	EM( TCP_SYN_SENT, "SYN_SENT" )		\
+	EM( TCP_SYN_RECV, "SYN_RECV" )		\
+	EM( TCP_FIN_WAIT1, "FIN_WAIT1" )	\
+	EM( TCP_FIN_WAIT2, "FIN_WAIT2" )	\
+	EM( TCP_TIME_WAIT, "TIME_WAIT" )	\
+	EM( TCP_CLOSE, "CLOSE" )		\
+	EM( TCP_CLOSE_WAIT, "CLOSE_WAIT" )	\
+	EM( TCP_LAST_ACK, "LAST_ACK" )		\
+	EM( TCP_LISTEN, "LISTEN" )		\
+	EMe( TCP_CLOSING, "CLOSING" )
 
 #define rpc_show_sock_state(state) \
-	__print_symbolic(state, \
-		{ TCP_ESTABLISHED, "ESTABLISHED" }, \
-		{ TCP_SYN_SENT, "SYN_SENT" }, \
-		{ TCP_SYN_RECV, "SYN_RECV" }, \
-		{ TCP_FIN_WAIT1, "FIN_WAIT1" }, \
-		{ TCP_FIN_WAIT2, "FIN_WAIT2" }, \
-		{ TCP_TIME_WAIT, "TIME_WAIT" }, \
-		{ TCP_CLOSE, "CLOSE" }, \
-		{ TCP_CLOSE_WAIT, "CLOSE_WAIT" }, \
-		{ TCP_LAST_ACK, "LAST_ACK" }, \
-		{ TCP_LISTEN, "LISTEN" }, \
-		{ TCP_CLOSING, "CLOSING" })
+	__print_symbolic(state, RPC_SHOW_SOCK)
+
+RPC_SHOW_SOCK
+
+/*
+ * Now redefine the EM() and EMe() macros to map the enums to the strings
+ * that will be printed in the output.
+ */
+#undef EM
+#undef EMe
+#define EM(a, b)	{a, b},
+#define EMe(a, b)	{a, b}
 
 DECLARE_EVENT_CLASS(xs_socket_event,
 
-- 
2.1.4

Re: [PATCH 15/18 v3] SUNRPC: Export enums in tracepoints to user space

[Steven Rostedt]

Trond,

I had your old email that I Cc'd this patch to. I updated this in my
git repo for your current email (if this is your current email).

-- Steve


On Thu, 02 Apr 2015 21:38:17 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> 
> The enums used in the tracepoints for __print_symbolic() have their
> names shown in the tracepoint format files. User space tools do not know
> how to convert those names into their values to be able to convert the
> binary data.
> 
> Use TRACE_DEFINE_ENUM() to export the enum names to their values for
> userspace to do the parsing correctly.
> 
> Cc: Trond Myklebust <Trond.Myklebust@netapp.com>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
>  include/trace/events/sunrpc.h | 62 ++++++++++++++++++++++++++++++-------------
>  1 file changed, 44 insertions(+), 18 deletions(-)
> 
> diff --git a/include/trace/events/sunrpc.h b/include/trace/events/sunrpc.h
> index b9c1dc6c825a..fd1a02cb3c82 100644
> --- a/include/trace/events/sunrpc.h
> +++ b/include/trace/events/sunrpc.h
> @@ -179,27 +179,53 @@ DEFINE_EVENT(rpc_task_queued, rpc_task_wakeup,
>  
>  );
>  
> +/*
> + * First define the enums in the below macros to be exported to userspace
> + * via TRACE_DEFINE_ENUM().
> + */
> +#undef EM
> +#undef EMe
> +#define EM(a, b)	TRACE_DEFINE_ENUM(a);
> +#define EMe(a, b)	TRACE_DEFINE_ENUM(a);
> +
> +#define RPC_SHOW_SOCKET				\
> +	EM( SS_FREE, "FREE" )			\
> +	EM( SS_UNCONNECTED, "UNCONNECTED" )	\
> +	EM( SS_CONNECTING, "CONNECTING," )	\
> +	EM( SS_CONNECTED, "CONNECTED," )	\
> +	EMe(SS_DISCONNECTING, "DISCONNECTING" )
> +
>  #define rpc_show_socket_state(state) \
> -	__print_symbolic(state, \
> -		{ SS_FREE, "FREE" }, \
> -		{ SS_UNCONNECTED, "UNCONNECTED" }, \
> -		{ SS_CONNECTING, "CONNECTING," }, \
> -		{ SS_CONNECTED, "CONNECTED," }, \
> -		{ SS_DISCONNECTING, "DISCONNECTING" })
> +	__print_symbolic(state, RPC_SHOW_SOCKET)
> +
> +RPC_SHOW_SOCKET
> +
> +#define RPC_SHOW_SOCK				\
> +	EM( TCP_ESTABLISHED, "ESTABLISHED" )	\
> +	EM( TCP_SYN_SENT, "SYN_SENT" )		\
> +	EM( TCP_SYN_RECV, "SYN_RECV" )		\
> +	EM( TCP_FIN_WAIT1, "FIN_WAIT1" )	\
> +	EM( TCP_FIN_WAIT2, "FIN_WAIT2" )	\
> +	EM( TCP_TIME_WAIT, "TIME_WAIT" )	\
> +	EM( TCP_CLOSE, "CLOSE" )		\
> +	EM( TCP_CLOSE_WAIT, "CLOSE_WAIT" )	\
> +	EM( TCP_LAST_ACK, "LAST_ACK" )		\
> +	EM( TCP_LISTEN, "LISTEN" )		\
> +	EMe( TCP_CLOSING, "CLOSING" )
>  
>  #define rpc_show_sock_state(state) \
> -	__print_symbolic(state, \
> -		{ TCP_ESTABLISHED, "ESTABLISHED" }, \
> -		{ TCP_SYN_SENT, "SYN_SENT" }, \
> -		{ TCP_SYN_RECV, "SYN_RECV" }, \
> -		{ TCP_FIN_WAIT1, "FIN_WAIT1" }, \
> -		{ TCP_FIN_WAIT2, "FIN_WAIT2" }, \
> -		{ TCP_TIME_WAIT, "TIME_WAIT" }, \
> -		{ TCP_CLOSE, "CLOSE" }, \
> -		{ TCP_CLOSE_WAIT, "CLOSE_WAIT" }, \
> -		{ TCP_LAST_ACK, "LAST_ACK" }, \
> -		{ TCP_LISTEN, "LISTEN" }, \
> -		{ TCP_CLOSING, "CLOSING" })
> +	__print_symbolic(state, RPC_SHOW_SOCK)
> +
> +RPC_SHOW_SOCK
> +
> +/*
> + * Now redefine the EM() and EMe() macros to map the enums to the strings
> + * that will be printed in the output.
> + */
> +#undef EM
> +#undef EMe
> +#define EM(a, b)	{a, b},
> +#define EMe(a, b)	{a, b}
>  
>  DECLARE_EVENT_CLASS(xs_socket_event,
>  

Re: [PATCH 15/18 v3] SUNRPC: Export enums in tracepoints to user space

[Trond Myklebust]

Hi Steve,

On Tue, Apr 7, 2015 at 12:40 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
>
> Trond,
>
> I had your old email that I Cc'd this patch to. I updated this in my
> git repo for your current email (if this is your current email).

Thanks! Yes the trond.myklebust@primarydata.com address is the correct one.

> -- Steve
>
>
> On Thu, 02 Apr 2015 21:38:17 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
>
>> From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
>>
>> The enums used in the tracepoints for __print_symbolic() have their
>> names shown in the tracepoint format files. User space tools do not know
>> how to convert those names into their values to be able to convert the
>> binary data.
>>
>> Use TRACE_DEFINE_ENUM() to export the enum names to their values for
>> userspace to do the parsing correctly.
>>
>> Cc: Trond Myklebust <Trond.Myklebust@netapp.com>
>> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

Acked-by: Trond Myklebust <trond.myklebust@primarydata.com>

>> ---
>>  include/trace/events/sunrpc.h | 62 ++++++++++++++++++++++++++++++-------------
>>  1 file changed, 44 insertions(+), 18 deletions(-)
>>
>> diff --git a/include/trace/events/sunrpc.h b/include/trace/events/sunrpc.h
>> index b9c1dc6c825a..fd1a02cb3c82 100644
>> --- a/include/trace/events/sunrpc.h
>> +++ b/include/trace/events/sunrpc.h
>> @@ -179,27 +179,53 @@ DEFINE_EVENT(rpc_task_queued, rpc_task_wakeup,
>>
>>  );
>>
>> +/*
>> + * First define the enums in the below macros to be exported to userspace
>> + * via TRACE_DEFINE_ENUM().
>> + */
>> +#undef EM
>> +#undef EMe
>> +#define EM(a, b)     TRACE_DEFINE_ENUM(a);
>> +#define EMe(a, b)    TRACE_DEFINE_ENUM(a);
>> +
>> +#define RPC_SHOW_SOCKET                              \
>> +     EM( SS_FREE, "FREE" )                   \
>> +     EM( SS_UNCONNECTED, "UNCONNECTED" )     \
>> +     EM( SS_CONNECTING, "CONNECTING," )      \
>> +     EM( SS_CONNECTED, "CONNECTED," )        \
>> +     EMe(SS_DISCONNECTING, "DISCONNECTING" )
>> +
>>  #define rpc_show_socket_state(state) \
>> -     __print_symbolic(state, \
>> -             { SS_FREE, "FREE" }, \
>> -             { SS_UNCONNECTED, "UNCONNECTED" }, \
>> -             { SS_CONNECTING, "CONNECTING," }, \
>> -             { SS_CONNECTED, "CONNECTED," }, \
>> -             { SS_DISCONNECTING, "DISCONNECTING" })
>> +     __print_symbolic(state, RPC_SHOW_SOCKET)
>> +
>> +RPC_SHOW_SOCKET
>> +
>> +#define RPC_SHOW_SOCK                                \
>> +     EM( TCP_ESTABLISHED, "ESTABLISHED" )    \
>> +     EM( TCP_SYN_SENT, "SYN_SENT" )          \
>> +     EM( TCP_SYN_RECV, "SYN_RECV" )          \
>> +     EM( TCP_FIN_WAIT1, "FIN_WAIT1" )        \
>> +     EM( TCP_FIN_WAIT2, "FIN_WAIT2" )        \
>> +     EM( TCP_TIME_WAIT, "TIME_WAIT" )        \
>> +     EM( TCP_CLOSE, "CLOSE" )                \
>> +     EM( TCP_CLOSE_WAIT, "CLOSE_WAIT" )      \
>> +     EM( TCP_LAST_ACK, "LAST_ACK" )          \
>> +     EM( TCP_LISTEN, "LISTEN" )              \
>> +     EMe( TCP_CLOSING, "CLOSING" )
>>
>>  #define rpc_show_sock_state(state) \
>> -     __print_symbolic(state, \
>> -             { TCP_ESTABLISHED, "ESTABLISHED" }, \
>> -             { TCP_SYN_SENT, "SYN_SENT" }, \
>> -             { TCP_SYN_RECV, "SYN_RECV" }, \
>> -             { TCP_FIN_WAIT1, "FIN_WAIT1" }, \
>> -             { TCP_FIN_WAIT2, "FIN_WAIT2" }, \
>> -             { TCP_TIME_WAIT, "TIME_WAIT" }, \
>> -             { TCP_CLOSE, "CLOSE" }, \
>> -             { TCP_CLOSE_WAIT, "CLOSE_WAIT" }, \
>> -             { TCP_LAST_ACK, "LAST_ACK" }, \
>> -             { TCP_LISTEN, "LISTEN" }, \
>> -             { TCP_CLOSING, "CLOSING" })
>> +     __print_symbolic(state, RPC_SHOW_SOCK)
>> +
>> +RPC_SHOW_SOCK
>> +
>> +/*
>> + * Now redefine the EM() and EMe() macros to map the enums to the strings
>> + * that will be printed in the output.
>> + */
>> +#undef EM
>> +#undef EMe
>> +#define EM(a, b)     {a, b},
>> +#define EMe(a, b)    {a, b}
>>
>>  DECLARE_EVENT_CLASS(xs_socket_event,
>>
>

[PATCH 16/18 v3] v4l: Export enums used by tracepoints to user space

[Steven Rostedt]

[-- Attachment #1: 0016-v4l-Export-enums-used-by-tracepoints-to-user-space.patch --]
[-- Type: text/plain, Size: 4177 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

Enums used by tracepoints for __print_symbolic() are shown in the
tracepoint format files with just their names and not their values.
This makes it difficult for user space tools to know how to convert the
binary data into their string representations.

By adding the use of TRACE_DEFINE_ENUM(), the enum names will be mapped
to their values and shown in the tracing file system to let tools
convert the data as necessary.

Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/trace/events/v4l2.h | 75 ++++++++++++++++++++++++++++++---------------
 1 file changed, 50 insertions(+), 25 deletions(-)

diff --git a/include/trace/events/v4l2.h b/include/trace/events/v4l2.h
index b9bb1f204693..20112170ff11 100644
--- a/include/trace/events/v4l2.h
+++ b/include/trace/events/v4l2.h
@@ -6,33 +6,58 @@
 
 #include <linux/tracepoint.h>
 
-#define show_type(type)							       \
-	__print_symbolic(type,						       \
-		{ V4L2_BUF_TYPE_VIDEO_CAPTURE,	      "VIDEO_CAPTURE" },       \
-		{ V4L2_BUF_TYPE_VIDEO_OUTPUT,	      "VIDEO_OUTPUT" },	       \
-		{ V4L2_BUF_TYPE_VIDEO_OVERLAY,	      "VIDEO_OVERLAY" },       \
-		{ V4L2_BUF_TYPE_VBI_CAPTURE,	      "VBI_CAPTURE" },	       \
-		{ V4L2_BUF_TYPE_VBI_OUTPUT,	      "VBI_OUTPUT" },	       \
-		{ V4L2_BUF_TYPE_SLICED_VBI_CAPTURE,   "SLICED_VBI_CAPTURE" },  \
-		{ V4L2_BUF_TYPE_SLICED_VBI_OUTPUT,    "SLICED_VBI_OUTPUT" },   \
-		{ V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY, "VIDEO_OUTPUT_OVERLAY" },\
-		{ V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, "VIDEO_CAPTURE_MPLANE" },\
-		{ V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,  "VIDEO_OUTPUT_MPLANE" }, \
-		{ V4L2_BUF_TYPE_SDR_CAPTURE,          "SDR_CAPTURE" },         \
-		{ V4L2_BUF_TYPE_PRIVATE,	      "PRIVATE" })
+/* Enums require being exported to userspace, for user tool parsing */
+#undef EM
+#undef EMe
+#define EM(a, b)	TRACE_DEFINE_ENUM(a);
+#define EMe(a, b)	TRACE_DEFINE_ENUM(a);
+
+#define show_type(type)							\
+	__print_symbolic(type, SHOW_TYPE)
+
+#define SHOW_TYPE							\
+	EM( V4L2_BUF_TYPE_VIDEO_CAPTURE,	"VIDEO_CAPTURE" )	\
+	EM( V4L2_BUF_TYPE_VIDEO_OUTPUT,		"VIDEO_OUTPUT" )	\
+	EM( V4L2_BUF_TYPE_VIDEO_OVERLAY,	"VIDEO_OVERLAY" )	\
+	EM( V4L2_BUF_TYPE_VBI_CAPTURE,		"VBI_CAPTURE" )		\
+	EM( V4L2_BUF_TYPE_VBI_OUTPUT,		"VBI_OUTPUT" )		\
+	EM( V4L2_BUF_TYPE_SLICED_VBI_CAPTURE,   "SLICED_VBI_CAPTURE" )	\
+	EM( V4L2_BUF_TYPE_SLICED_VBI_OUTPUT,    "SLICED_VBI_OUTPUT" )	\
+	EM( V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY, "VIDEO_OUTPUT_OVERLAY" ) \
+	EM( V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, "VIDEO_CAPTURE_MPLANE" ) \
+	EM( V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,  "VIDEO_OUTPUT_MPLANE" )	\
+	EM( V4L2_BUF_TYPE_SDR_CAPTURE,          "SDR_CAPTURE" )		\
+	EMe(V4L2_BUF_TYPE_PRIVATE,		"PRIVATE" )
+
+SHOW_TYPE
 
 #define show_field(field)						\
-	__print_symbolic(field,						\
-		{ V4L2_FIELD_ANY,		"ANY" },		\
-		{ V4L2_FIELD_NONE,		"NONE" },		\
-		{ V4L2_FIELD_TOP,		"TOP" },		\
-		{ V4L2_FIELD_BOTTOM,		"BOTTOM" },		\
-		{ V4L2_FIELD_INTERLACED,	"INTERLACED" },		\
-		{ V4L2_FIELD_SEQ_TB,		"SEQ_TB" },		\
-		{ V4L2_FIELD_SEQ_BT,		"SEQ_BT" },		\
-		{ V4L2_FIELD_ALTERNATE,		"ALTERNATE" },		\
-		{ V4L2_FIELD_INTERLACED_TB,	"INTERLACED_TB" },      \
-		{ V4L2_FIELD_INTERLACED_BT,	"INTERLACED_BT" })
+	__print_symbolic(field, SHOW_FIELD)
+
+#define SHOW_FIELD							\
+	EM( V4L2_FIELD_ANY,		"ANY" )				\
+	EM( V4L2_FIELD_NONE,		"NONE" )			\
+	EM( V4L2_FIELD_TOP,		"TOP" )				\
+	EM( V4L2_FIELD_BOTTOM,		"BOTTOM" )			\
+	EM( V4L2_FIELD_INTERLACED,	"INTERLACED" )			\
+	EM( V4L2_FIELD_SEQ_TB,		"SEQ_TB" )			\
+	EM( V4L2_FIELD_SEQ_BT,		"SEQ_BT" )			\
+	EM( V4L2_FIELD_ALTERNATE,	"ALTERNATE" )			\
+	EM( V4L2_FIELD_INTERLACED_TB,	"INTERLACED_TB" )		\
+	EMe( V4L2_FIELD_INTERLACED_BT,	"INTERLACED_BT" )
+
+SHOW_FIELD
+
+/*
+ * Now redefine the EM() and EMe() macros to map the enums to the strings
+ * that will be printed in the output.
+ */
+#undef EM
+#undef EMe
+#define EM(a, b)	{a, b},
+#define EMe(a, b)	{a, b}
+
+/* V4L2_TC_TYPE_* are macros, not defines, they do not need processing */
 
 #define show_timecode_type(type)					\
 	__print_symbolic(type,						\
-- 
2.1.4

[PATCH 17/18 v3] writeback: Export enums used by tracepoint to user space

[Steven Rostedt]

[-- Attachment #1: 0017-writeback-Export-enums-used-by-tracepoint-to-user-sp.patch --]
[-- Type: text/plain, Size: 2149 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

The enums used in tracepoints for __print_symbolic() do not have their
values shown in the tracepoint format files and this makes it difficult
for user space tools to convert the binary values to the strings they
are to represent.

Add TRACE_DEFINE_ENUM(x) macros to export the enum names to their values
to make the tracing output from user space tools more robust.

Cc: Dave Chinner <dchinner@redhat.com>
Cc: Jens Axboe <jaxboe@fusionio.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/trace/events/writeback.h | 33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h
index 5a14ead59696..880dd7437172 100644
--- a/include/trace/events/writeback.h
+++ b/include/trace/events/writeback.h
@@ -23,15 +23,32 @@
 		{I_REFERENCED,		"I_REFERENCED"}		\
 	)
 
+/* enums need to be exported to user space */
+#undef EM
+#undef EMe
+#define EM(a,b) 	TRACE_DEFINE_ENUM(a);
+#define EMe(a,b)	TRACE_DEFINE_ENUM(a);
+
 #define WB_WORK_REASON							\
-		{WB_REASON_BACKGROUND,		"background"},		\
-		{WB_REASON_TRY_TO_FREE_PAGES,	"try_to_free_pages"},	\
-		{WB_REASON_SYNC,		"sync"},		\
-		{WB_REASON_PERIODIC,		"periodic"},		\
-		{WB_REASON_LAPTOP_TIMER,	"laptop_timer"},	\
-		{WB_REASON_FREE_MORE_MEM,	"free_more_memory"},	\
-		{WB_REASON_FS_FREE_SPACE,	"fs_free_space"},	\
-		{WB_REASON_FORKER_THREAD,	"forker_thread"}
+	EM( WB_REASON_BACKGROUND,		"background")		\
+	EM( WB_REASON_TRY_TO_FREE_PAGES,	"try_to_free_pages")	\
+	EM( WB_REASON_SYNC,			"sync")			\
+	EM( WB_REASON_PERIODIC,			"periodic")		\
+	EM( WB_REASON_LAPTOP_TIMER,		"laptop_timer")		\
+	EM( WB_REASON_FREE_MORE_MEM,		"free_more_memory")	\
+	EM( WB_REASON_FS_FREE_SPACE,		"fs_free_space")	\
+	EMe(WB_REASON_FORKER_THREAD,		"forker_thread")
+
+WB_WORK_REASON
+
+/*
+ * Now redefine the EM() and EMe() macros to map the enums to the strings
+ * that will be printed in the output.
+ */
+#undef EM
+#undef EMe
+#define EM(a,b)		{ a, b },
+#define EMe(a,b)	{ a, b }
 
 struct wb_writeback_work;
 
-- 
2.1.4

[PATCH 18/18 v3] tracing: Add enum_map file to show enums that have been mapped

[Steven Rostedt]

[-- Attachment #1: 0018-tracing-Add-enum_map-file-to-show-enums-that-have-be.patch --]
[-- Type: text/plain, Size: 9834 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

Add a enum_map file in the tracing directory to see what enums have been
saved to convert in the print fmt files.

As this requires the enum mapping to be persistent in memory, it is only
created if the new config option CONFIG_TRACE_ENUM_MAP_FILE is enabled.
This is for debugging and will increase the persistent memory footprint
of the kernel.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/Kconfig |  28 ++++++
 kernel/trace/trace.c | 245 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 269 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index a5da09c899dd..fedbdd7d5d1e 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -599,6 +599,34 @@ config RING_BUFFER_STARTUP_TEST
 
 	 If unsure, say N
 
+config TRACE_ENUM_MAP_FILE
+       bool "Show enum mappings for trace events"
+       depends on TRACING
+       help
+        The "print fmt" of the trace events will show the enum names instead
+	of their values. This can cause problems for user space tools that
+	use this string to parse the raw data as user space does not know
+	how to convert the string to its value.
+
+	To fix this, there's a special macro in the kernel that can be used
+	to convert the enum into its value. If this macro is used, then the
+	print fmt strings will have the enums converted to their values.
+
+	If something does not get converted properly, this option can be
+	used to show what enums the kernel tried to convert.
+
+	This option is for debugging the enum conversions. A file is created
+	in the tracing directory called "enum_map" that will show the enum
+	names matched with their values and what trace event system they
+	belong too.
+
+	Normally, the mapping of the strings to values will be freed after
+	boot up or module load. With this option, they will not be freed, as
+	they are needed for the "enum_map" file. Enabling this option will
+	increase the memory footprint of the running kernel.
+
+	If unsure, say N
+
 endif # FTRACE
 
 endif # TRACING_SUPPORT
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 28e6654e640d..39e69568302e 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -123,6 +123,42 @@ enum ftrace_dump_mode ftrace_dump_on_oops;
 /* When set, tracing will stop when a WARN*() is hit */
 int __disable_trace_on_warning;
 
+#ifdef CONFIG_TRACE_ENUM_MAP_FILE
+/* Map of enums to their values, for "enum_map" file */
+struct trace_enum_map_head {
+	struct module			*mod;
+	unsigned long			length;
+};
+
+union trace_enum_map_item;
+
+struct trace_enum_map_tail {
+	/*
+	 * "end" is first and points to NULL as it must be different
+	 * than "mod" or "enum_string"
+	 */
+	union trace_enum_map_item	*next;
+	const char			*end;	/* points to NULL */
+};
+
+static DEFINE_MUTEX(trace_enum_mutex);
+
+/*
+ * The trace_enum_maps are saved in an array with two extra elements,
+ * one at the beginning, and one at the end. The beginning item contains
+ * the count of the saved maps (head.length), and the module they
+ * belong to if not built in (head.mod). The ending item contains a
+ * pointer to the next array of saved enum_map items.
+ */
+union trace_enum_map_item {
+	struct trace_enum_map		map;
+	struct trace_enum_map_head	head;
+	struct trace_enum_map_tail	tail;
+};
+
+static union trace_enum_map_item *trace_enum_maps;
+#endif /* CONFIG_TRACE_ENUM_MAP_FILE */
+
 static int tracing_set_tracer(struct trace_array *tr, const char *buf);
 
 #define MAX_TRACER_SIZE		100
@@ -3908,7 +3944,169 @@ static const struct file_operations tracing_saved_cmdlines_size_fops = {
 	.write		= tracing_saved_cmdlines_size_write,
 };
 
-static void trace_insert_enum_map(struct trace_enum_map **start, int len)
+#ifdef CONFIG_TRACE_ENUM_MAP_FILE
+static union trace_enum_map_item *
+update_enum_map(union trace_enum_map_item *ptr)
+{
+	if (!ptr->map.enum_string) {
+		if (ptr->tail.next) {
+			ptr = ptr->tail.next;
+			/* Set ptr to the next real item (skip head) */
+			ptr++;
+		} else
+			return NULL;
+	}
+	return ptr;
+}
+
+static void *enum_map_next(struct seq_file *m, void *v, loff_t *pos)
+{
+	union trace_enum_map_item *ptr = v;
+
+	/*
+	 * Paranoid! If ptr points to end, we don't want to increment past it.
+	 * This really should never happen.
+	 */
+	ptr = update_enum_map(ptr);
+	if (WARN_ON_ONCE(!ptr))
+		return NULL;
+
+	ptr++;
+
+	(*pos)++;
+
+	ptr = update_enum_map(ptr);
+
+	return ptr;
+}
+
+static void *enum_map_start(struct seq_file *m, loff_t *pos)
+{
+	union trace_enum_map_item *v;
+	loff_t l = 0;
+
+	mutex_lock(&trace_enum_mutex);
+
+	v = trace_enum_maps;
+	if (v)
+		v++;
+
+	while (v && l < *pos) {
+		v = enum_map_next(m, v, &l);
+	}
+
+	return v;
+}
+
+static void enum_map_stop(struct seq_file *m, void *v)
+{
+	mutex_unlock(&trace_enum_mutex);
+}
+
+static int enum_map_show(struct seq_file *m, void *v)
+{
+	union trace_enum_map_item *ptr = v;
+
+	seq_printf(m, "%s %ld (%s)\n",
+		   ptr->map.enum_string, ptr->map.enum_value,
+		   ptr->map.system);
+
+	return 0;
+}
+
+static const struct seq_operations tracing_enum_map_seq_ops = {
+	.start		= enum_map_start,
+	.next		= enum_map_next,
+	.stop		= enum_map_stop,
+	.show		= enum_map_show,
+};
+
+static int tracing_enum_map_open(struct inode *inode, struct file *filp)
+{
+	if (tracing_disabled)
+		return -ENODEV;
+
+	return seq_open(filp, &tracing_enum_map_seq_ops);
+}
+
+static const struct file_operations tracing_enum_map_fops = {
+	.open		= tracing_enum_map_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= seq_release,
+};
+
+static inline union trace_enum_map_item *
+trace_enum_jmp_to_tail(union trace_enum_map_item *ptr)
+{
+	/* Return tail of array given the head */
+	return ptr + ptr->head.length + 1;
+}
+
+static void
+trace_insert_enum_map_file(struct module *mod, struct trace_enum_map **start,
+			   int len)
+{
+	struct trace_enum_map **stop;
+	struct trace_enum_map **map;
+	union trace_enum_map_item *map_array;
+	union trace_enum_map_item *ptr;
+
+	stop = start + len;
+
+	/*
+	 * The trace_enum_maps contains the map plus a head and tail item,
+	 * where the head holds the module and length of array, and the
+	 * tail holds a pointer to the next list.
+	 */
+	map_array = kmalloc(sizeof(*map_array) * (len + 2), GFP_KERNEL);
+	if (!map_array) {
+		pr_warning("Unable to allocate trace enum mapping\n");
+		return;
+	}
+
+	mutex_lock(&trace_enum_mutex);
+
+	if (!trace_enum_maps)
+		trace_enum_maps = map_array;
+	else {
+		ptr = trace_enum_maps;
+		for (;;) {
+			ptr = trace_enum_jmp_to_tail(ptr);
+			if (!ptr->tail.next)
+				break;
+			ptr = ptr->tail.next;
+
+		}
+		ptr->tail.next = map_array;
+	}
+	map_array->head.mod = mod;
+	map_array->head.length = len;
+	map_array++;
+
+	for (map = start; (unsigned long)map < (unsigned long)stop; map++) {
+		map_array->map = **map;
+		map_array++;
+	}
+	memset(map_array, 0, sizeof(*map_array));
+
+	mutex_unlock(&trace_enum_mutex);
+}
+
+static void trace_create_enum_file(struct dentry *d_tracer)
+{
+	trace_create_file("enum_map", 0444, d_tracer,
+			  NULL, &tracing_enum_map_fops);
+}
+
+#else /* CONFIG_TRACE_ENUM_MAP_FILE */
+static inline void trace_create_enum_file(struct dentry *d_tracer) { }
+static inline void trace_insert_enum_map_file(struct module *mod,
+			      struct trace_enum_map **start, int len) { }
+#endif /* !CONFIG_TRACE_ENUM_MAP_FILE */
+
+static void trace_insert_enum_map(struct module *mod,
+				  struct trace_enum_map **start, int len)
 {
 	struct trace_enum_map **map;
 
@@ -3918,6 +4116,8 @@ static void trace_insert_enum_map(struct trace_enum_map **start, int len)
 	map = start;
 
 	trace_event_enum_update(map, len);
+
+	trace_insert_enum_map_file(mod, start, len);
 }
 
 static ssize_t
@@ -6562,7 +6762,7 @@ static void __init trace_enum_init(void)
 	int len;
 
 	len = __stop_ftrace_enum_maps - __start_ftrace_enum_maps;
-	trace_insert_enum_map(__start_ftrace_enum_maps, len);
+	trace_insert_enum_map(NULL, __start_ftrace_enum_maps, len);
 }
 
 #ifdef CONFIG_MODULES
@@ -6578,9 +6778,41 @@ static void trace_module_add_enums(struct module *mod)
 	if (trace_module_has_bad_taint(mod))
 		return;
 
-	trace_insert_enum_map(mod->trace_enums, mod->num_trace_enums);
+	trace_insert_enum_map(mod, mod->trace_enums, mod->num_trace_enums);
 }
 
+#ifdef CONFIG_TRACE_ENUM_MAP_FILE
+static void trace_module_remove_enums(struct module *mod)
+{
+	union trace_enum_map_item *map;
+	union trace_enum_map_item **last = &trace_enum_maps;
+
+	if (!mod->num_trace_enums)
+		return;
+
+	mutex_lock(&trace_enum_mutex);
+
+	map = trace_enum_maps;
+
+	while (map) {
+		if (map->head.mod == mod)
+			break;
+		map = trace_enum_jmp_to_tail(map);
+		last = &map->tail.next;
+		map = map->tail.next;
+	}
+	if (!map)
+		goto out;
+
+	*last = trace_enum_jmp_to_tail(map)->tail.next;
+	kfree(map);
+ out:
+	mutex_unlock(&trace_enum_mutex);
+}
+#else
+static inline void trace_module_remove_enums(struct module *mod) { }
+#endif /* CONFIG_TRACE_ENUM_MAP_FILE */
+
 static int trace_module_notify(struct notifier_block *self,
 			       unsigned long val, void *data)
 {
@@ -6590,6 +6822,9 @@ static int trace_module_notify(struct notifier_block *self,
 	case MODULE_STATE_COMING:
 		trace_module_add_enums(mod);
 		break;
+	case MODULE_STATE_GOING:
+		trace_module_remove_enums(mod);
+		break;
 	}
 
 	return 0;
@@ -6599,7 +6834,7 @@ static struct notifier_block trace_module_nb = {
 	.notifier_call = trace_module_notify,
 	.priority = 0,
 };
-#endif
+#endif /* CONFIG_MODULES */
 
 static __init int tracer_init_debugfs(void)
 {
@@ -6627,6 +6862,8 @@ static __init int tracer_init_debugfs(void)
 
 	trace_enum_init();
 
+	trace_create_enum_file(d_tracer);
+
 #ifdef CONFIG_MODULES
 	register_module_notifier(&trace_module_nb);
 #endif
-- 
2.1.4

Re: [PATCH 01/18 v3] tracing/drm: Remove unused TRACE_SYSTEM_STRING define

[Masami Hiramatsu]

(2015/04/03 10:38), Steven Rostedt wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
> 
> The tracing infrastructure is adding a macro TRACE_SYSTEM_STRING, and
> hit the following build failure:
> 
>    In file included from include/trace/define_trace.h:90:0,
>                     from drivers/gpu/drm/.//radeon/radeon_trace.h:209,
>                     from drivers/gpu/drm/.//radeon/radeon_trace_points.c:9:
>>> >> include/trace/ftrace.h:28:0: warning: "TRACE_SYSTEM_STRING" redefined
>     #define TRACE_SYSTEM_STRING __app(TRACE_SYSTEM_VAR,__trace_system_name)
> 
> Seems that the DRM folks have added their own use to the
> TRACE_SYSTEM_STRING, with:
> 
>  #define TRACE_SYSTEM_STRING __stringify(TRACE_SYSTEM)
> 
> Although, I can not find its use anywhere. I could simply use another
> name, but if this macro is not being used, it should be removed.
> 
> Link: http://lkml.kernel.org/r/20150402123736.01eda052@gandalf.local.home
> 
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Reported-by: kbuild test robot <fengguang.wu@intel.com>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

Thanks,

> ---
>  drivers/gpu/drm/drm_trace.h           | 1 -
>  drivers/gpu/drm/i915/i915_trace.h     | 1 -
>  drivers/gpu/drm/radeon/radeon_trace.h | 1 -
>  3 files changed, 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_trace.h b/drivers/gpu/drm/drm_trace.h
> index 27cc95f36381..ce3c42813fbb 100644
> --- a/drivers/gpu/drm/drm_trace.h
> +++ b/drivers/gpu/drm/drm_trace.h
> @@ -7,7 +7,6 @@
>  
>  #undef TRACE_SYSTEM
>  #define TRACE_SYSTEM drm
> -#define TRACE_SYSTEM_STRING __stringify(TRACE_SYSTEM)
>  #define TRACE_INCLUDE_FILE drm_trace
>  
>  TRACE_EVENT(drm_vblank_event,
> diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
> index 6058a01b4443..d776621c8521 100644
> --- a/drivers/gpu/drm/i915/i915_trace.h
> +++ b/drivers/gpu/drm/i915/i915_trace.h
> @@ -12,7 +12,6 @@
>  
>  #undef TRACE_SYSTEM
>  #define TRACE_SYSTEM i915
> -#define TRACE_SYSTEM_STRING __stringify(TRACE_SYSTEM)
>  #define TRACE_INCLUDE_FILE i915_trace
>  
>  /* pipe updates */
> diff --git a/drivers/gpu/drm/radeon/radeon_trace.h b/drivers/gpu/drm/radeon/radeon_trace.h
> index ce075cb08cb2..fdce4062901f 100644
> --- a/drivers/gpu/drm/radeon/radeon_trace.h
> +++ b/drivers/gpu/drm/radeon/radeon_trace.h
> @@ -9,7 +9,6 @@
>  
>  #undef TRACE_SYSTEM
>  #define TRACE_SYSTEM radeon
> -#define TRACE_SYSTEM_STRING __stringify(TRACE_SYSTEM)
>  #define TRACE_INCLUDE_FILE radeon_trace
>  
>  TRACE_EVENT(radeon_bo_create,
> -- 2.1.4
> 


-- 
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [PATCH 00/18 v3] tracing: Use TRACE_DEFINE_ENUM() to show enum values

[Masami Hiramatsu]

(2015/04/03 10:38), Steven Rostedt wrote:
> As there are many tracepoints that use __print_symbolic() to translate
> numbers into ASCII strings, and several of these translate enums as
> well, it causes a problem for user space tools that read the tracepoint
> format files and have to translate the binary data to their associated
> strings.
> 
> For example, with the tlb_flush tracepoint, we have this in the format
> file:
> 
> print fmt: "pages:%ld reason:%s (%d)", REC->pages,
>  __print_symbolic(REC->reason,
>    { TLB_FLUSH_ON_TASK_SWITCH, "flush on task switch" },
>    { TLB_REMOTE_SHOOTDOWN, "remote shootdown" },
>    { TLB_LOCAL_SHOOTDOWN, "local shootdown" },
>    { TLB_LOCAL_MM_SHOOTDOWN, "local mm shootdown" }), REC->reason
> 
> Now, userspace does not know what the value of TLB_REMOTE_SHOOTDOWN is.
> To solve this, a new macro is created as a helper to allow tracepoints
> to export enums they use to userspace. This macro is called,
> TRACE_DEFINE_ENUM(), such that
> 
>  TRACE_DEFINE_ENUM(TLB_REMOTE_SHOOTDOWN);
> 
> will convert the "print fmt"s in the format files to its actual value
> and no longer display the enum name.
> 
> On boot up (or module load), the enums saved via TRACE_DEFINE_ENUM()
> will be searched for in the TP_printk()s of the tracepoints. Logic
> knows enough to ignore quoted text.
> 
> And the output of the tlb_flush format is now:
> 
> print fmt: "pages:%ld reason:%s (%d)", REC->pages,
>  __print_symbolic(REC->reason,
>    { 0, "flush on task switch" },
>    { 1, "remote shootdown" },
>    { 2, "local shootdown" },
>    { 3, "local mm shootdown" }), REC->reason
> 
> And userspace tools can easily parse that without special handling.
> 
> For debugging, if CONFIG_TRACE_ENUM_MAP_FILE is enabled, a file is added
> in the tracing directory to show what enums were added, their values and
> the TRACE_SYSTEM that added them:
> 
>  # cat /sys/kernel/debug/tracing/enum_map
> TLB_LOCAL_MM_SHOOTDOWN 3 (tlb)
> TLB_LOCAL_SHOOTDOWN 2 (tlb)
> TLB_REMOTE_SHOOTDOWN 1 (tlb)
> TLB_FLUSH_ON_TASK_SWITCH 0 (tlb)

I've reviewed all patches in this series and tested it.

Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Tested-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

for all the patches in this series.

However, I've also found that the __print_symbolic()s in

events/scsi/scsi_dispatch_cmd_*/format
events/xfs/xfs_map_blocks_alloc/format
events/xfs/xfs_get_blocks_found/format
events/xfs/xfs_get_blocks_alloc/format
events/power/pm_qos_*/format

are not fixed yet.

Thank you,

> 
> -- Steve
> 
> Local SHA1: 6689c330e3a60defc4c89da17cf5d561649bc55b
> 
> 
> Steven Rostedt (1):
>       tracing/drm: Remove unused TRACE_SYSTEM_STRING define
> 
> Steven Rostedt (Red Hat) (17):
>       tracing: Add TRACE_SYSTEM_VAR to intel-sst
>       tracing: Add TRACE_SYSTEM_VAR to kvm-s390
>       tracing: Add TRACE_SYSTEM_VAR to xhci-hcd
>       tracing: Give system name a pointer
>       tracing: Update trace-event-sample with TRACE_SYSTEM_VAR documentation
>       tracing: Add TRACE_DEFINE_ENUM() macro to map enums to their values
>       tracing: Allow for modules to convert their enums to values
>       tracing/samples: Update the trace-event-sample.h with TRACE_DEFINE_ENUM()
>       x86/tlb/trace: Export enums in used by tlb_flush tracepoint
>       net/9p/tracing: Export enums in tracepoints to userspace
>       f2fs: Export the enums in the tracepoints to userspace
>       irq/tracing: Export enums in tracepoints to user space
>       mm: tracing: Export enums in tracepoints to user space
>       SUNRPC: Export enums in tracepoints to user space
>       v4l: Export enums used by tracepoints to user space
>       writeback: Export enums used by tracepoint to user space
>       tracing: Add enum_map file to show enums that have been mapped
> 
> ----
>  arch/s390/kvm/trace-s390.h                 |   7 +
>  drivers/gpu/drm/drm_trace.h                |   1 -
>  drivers/gpu/drm/i915/i915_trace.h          |   1 -
>  drivers/gpu/drm/radeon/radeon_trace.h      |   1 -
>  drivers/usb/host/xhci-trace.h              |   7 +
>  include/asm-generic/vmlinux.lds.h          |   5 +-
>  include/linux/ftrace_event.h               |   4 +-
>  include/linux/module.h                     |   2 +
>  include/linux/tracepoint.h                 |   8 +
>  include/trace/events/9p.h                  | 157 ++++++++-------
>  include/trace/events/f2fs.h                |  30 +++
>  include/trace/events/intel-sst.h           |   7 +
>  include/trace/events/irq.h                 |  39 ++--
>  include/trace/events/migrate.h             |  42 +++-
>  include/trace/events/sunrpc.h              |  62 ++++--
>  include/trace/events/tlb.h                 |  30 ++-
>  include/trace/events/v4l2.h                |  75 ++++---
>  include/trace/events/writeback.h           |  33 +++-
>  include/trace/ftrace.h                     |  41 +++-
>  kernel/module.c                            |   3 +
>  kernel/trace/Kconfig                       |  28 +++
>  kernel/trace/trace.c                       | 304 ++++++++++++++++++++++++++++-
>  kernel/trace/trace.h                       |   2 +
>  kernel/trace/trace_events.c                | 121 +++++++++++-
>  samples/trace_events/trace-events-sample.h |  84 +++++++-
>  25 files changed, 932 insertions(+), 162 deletions(-)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
> 


-- 
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [PATCH 00/18 v3] tracing: Use TRACE_DEFINE_ENUM() to show enum values

[Steven Rostedt]

On Tue, 07 Apr 2015 20:26:29 +0900
Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:

> I've reviewed all patches in this series and tested it.
> 
> Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Tested-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> 
> for all the patches in this series.

Thanks for spending the time on this. I'll be hopefully posting this to
linux-next when it finally passes all my tests (I've been finding use
cases that need to be fixed for this to work). The code itself is still
sound.

> 
> However, I've also found that the __print_symbolic()s in
> 
> events/scsi/scsi_dispatch_cmd_*/format
> events/xfs/xfs_map_blocks_alloc/format
> events/xfs/xfs_get_blocks_found/format
> events/xfs/xfs_get_blocks_alloc/format
> events/power/pm_qos_*/format

Ah, missed some of those, as scsi and xfs use mostly defines, not enums.
I need to boot a config that has all tracepoints enabled and check them
out.

Thanks,

-- Steve

> 
> are not fixed yet.
> 
> Thank you,
> 
> > 
> > -- Steve