tracing: Use TRACE_DEFINE_ENUM() to show enum values

From: "Steven Rostedt (Red Hat)" <[email protected]>

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 export the value of the enum via the file:

tracing/enum_map

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/enum_map
TLB_FLUSH_ON_TASK_SWITCH 0
TLB_REMOTE_SHOOTDOWN 1
TLB_LOCAL_SHOOTDOWN 2
TLB_LOCAL_MM_SHOOTDOWN 3

The above can be easily parsed by userspace and it can be able to convert the enums to their values and properly parse the enums within the __print_symbolic() and __print_flags() helper functions.

Cc Guilherme Cox <[email protected]>

Cc Tony Luck <[email protected]>

Cc Xie XiuQi <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---

kernel/trace/trace.c +3 -0

index 62c6506d663f..53b449c522a7 100644
@@ -123,6 +123,9 @@ enum ftrace_dump_mode ftrace_dump_on_oops;
 /* When set, tracing will stop when a WARN*() is hit */
 int __disable_trace_on_warning;
 
+/* Map of enums to their values, for "enum_map" file */
+static struct trace_enum_map *trace_enum_maps;
+
 static int tracing_set_tracer(struct trace_array *tr, const char *buf);
 
 #define MAX_TRACER_SIZE		100

kernel/trace/trace.c +98 -1

+
+	ptr++;
+
+	(*pos)++;
+
+	if (!ptr->enum_string)
+		return NULL;
+
+	return ptr;
+}
+
+static void *enum_map_start(struct seq_file *m, loff_t *pos)
+{
+	struct trace_enum_map *v;
+	loff_t l = 0;
+
+	v = trace_enum_maps;
+	while (v && l < *pos) {
+		v = enum_map_next(m, v, &l);
+	}
+
+	return v;
+}
+
+static void enum_map_stop(struct seq_file *m, void *v)
+{
+}
+
+static int enum_map_show(struct seq_file *m, void *v)
+{
+	struct trace_enum_map *ptr = v;
+
+	seq_printf(m, "%s %ld\n", ptr->enum_string, ptr->enum_value);
+	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 void
+trace_insert_enum_map(struct trace_enum_map **start, struct trace_enum_map **stop)
+{
+	struct trace_enum_map **map;
+	struct trace_enum_map *map_array;
+	int len = stop - start;
+
+	if (len <= 0)
+		return;
+
+	map_array = kmalloc(sizeof(*map_array) * (len + 1), GFP_KERNEL);
+	if (!map_array) {
+		pr_warning("Unable to allocate trace enum mapping\n");
+		return;
+	}
+
+	trace_enum_maps = map_array;
+
+	for (map = start; (unsigned long)map < (unsigned long)stop; map++) {
+		*map_array = **map;
+		map_array++;
+	}
+	memset(map_array, 0, sizeof(*map_array));
+}
+
+
 static ssize_t
 tracing_set_trace_read(struct file *filp, char __user *ubuf,
 		       size_t cnt, loff_t *ppos)

@@ -6542,6 +6635,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 +6667,11 @@ 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();
+
+	trace_create_file("enum_map", 0444, d_tracer,
+			  NULL, &tracing_enum_map_fops);
+
 #ifdef CONFIG_DYNAMIC_FTRACE
 	trace_create_file("dyn_ftrace_total_info", 0444, d_tracer,
 			&ftrace_update_tot_cnt, &tracing_dyn_info_fops);

@@ -6888,7 +6994,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)

Signature · 8 lines

2.1.4


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

From: "Steven Rostedt (Red Hat)" <[email protected]>

Update the infrastructure such that modules that declare TRACE_DEFINE_ENUM() will have those enums shown in the enum_map files in the tracing directory.

Cc Rusty Russell <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 include/linux/module.h |   2 +
 kernel/module.c        |   3 +
 kernel/trace/trace.c   | 184 ++++++++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 173 insertions(+), 16 deletions(-)

include/linux/module.h +2 -0

index 42999fe2dbd0..53dc41dd5c62 100644
@@ -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;

kernel/module.c +3 -0

index b3d634ed06c9..d8f8ab271c2b 100644
@@ -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",

kernel/trace/trace.c +10 -1

index 53b449c522a7..97031878eb3d 100644
@@ -124,7 +124,38 @@ enum ftrace_dump_mode ftrace_dump_on_oops;
 int __disable_trace_on_warning;
 
 /* Map of enums to their values, for "enum_map" file */
-static struct trace_enum_map *trace_enum_maps;
+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

kernel/trace/trace.c +12 -0

+ * 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;
 
 static int tracing_set_tracer(struct trace_array *tr, const char *buf);
 

kernel/trace/trace.c +25 -10

+	if (!ptr)
 		return NULL;
 
 	ptr++;
 
 	(*pos)++;
 
-	if (!ptr->enum_string)
-		return NULL;
+	ptr = update_enum_map(ptr);
 
 	return ptr;
 }
 
 static void *enum_map_start(struct seq_file *m, loff_t *pos)
 {
-	struct trace_enum_map *v;
+	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);
 	}

@@ -3943,13 +3992,15 @@ static void *enum_map_start(struct seq_file *m, loff_t *pos)
 
 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)
 {
-	struct trace_enum_map *ptr = v;
+	union trace_enum_map_item *ptr = v;
+
+	seq_printf(m, "%s %ld\n", ptr->map.enum_string, ptr->map.enum_value);
 
-	seq_printf(m, "%s %ld\n", ptr->enum_string, ptr->enum_value);
 	return 0;
 }
 

@@ -3975,29 +4026,56 @@ static const struct file_operations tracing_enum_map_fops = {
 	.release	= seq_release,
 };
 
-static void
-trace_insert_enum_map(struct trace_enum_map **start, struct trace_enum_map **stop)
+static void trace_insert_enum_map(struct module *mod,
+				  struct trace_enum_map **start, int len)
 {
+	struct trace_enum_map **stop;
 	struct trace_enum_map **map;
-	struct trace_enum_map *map_array;
-	int len = stop - start;
+	union trace_enum_map_item *map_array;
+	union trace_enum_map_item *ptr;
 
 	if (len <= 0)
 		return;
 
-	map_array = kmalloc(sizeof(*map_array) * (len + 1), GFP_KERNEL);
+	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;
 	}

kernel/trace/trace.c +88 -2

+			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_array->map = **map;
 		map_array++;
 	}
 	memset(map_array, 0, sizeof(*map_array));
+
+	mutex_unlock(&trace_enum_mutex);
 }
 
 

@@ -6640,9 +6718,79 @@ 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(NULL, __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, mod->trace_enums, mod->num_trace_enums);
+}
+
+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 += map->head.length + 1;
+		last = &map->tail.next;
+		map = map->tail.next;
+	}
+	if (!map)
+		goto out;
+
+	*last = map[map->head.length + 1].tail.next;
+	kfree(map);
+ out:
+	mutex_unlock(&trace_enum_mutex);
+}
+
+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;
+	case MODULE_STATE_GOING:
+		trace_module_remove_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;

@@ -6672,6 +6820,10 @@ static __init int tracer_init_debugfs(void)
 	trace_create_file("enum_map", 0444, d_tracer,
 			  NULL, &tracing_enum_map_fops);
 
+#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);

Signature · 8 lines

2.1.4


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Quoted 1 line

+ * The trace_enum_maps are saved in an array whith two extra elements,

s/whith/with/

Quoted 20 lines

+ * 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.
+ */

kernel/trace/trace.c +7 -0

+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;
 
 static int tracing_set_tracer(struct trace_array *tr, const char *buf);

@@ -3911,29 +3942,47 @@ static const struct file_operations tracing_saved_cmdlines_size_fops = {
 	.write		= tracing_saved_cmdlines_size_write,
 };
 

kernel/trace/trace.c static const struct file_operations tracing_saved_cmdlines_size_fops = { · line 3951

Quoted 7 lines

kernel/trace/trace.c +7 -0

+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;
+			ptr++;

I guess this additional increment is to skip head item. Maybe it deserves a comment. :)

Quoted 13 lines

kernel/trace/trace.c +7 -1

+		} else
+			return NULL;
+	}
+	return ptr;
+}
+
 static void *enum_map_next(struct seq_file *m, void *v, loff_t *pos)
 {
-	struct trace_enum_map *ptr = v;
+	union trace_enum_map_item *ptr = v;
 

Review static const struct file_operations tracing_saved_cmdlines_size_fops = { · line 3962

Quoted 2 lines

(quoted diff) +1 -1

-	if (!ptr->enum_string)
+	ptr = update_enum_map(ptr);

Again, is this necessary?

Quoted 30 lines

kernel/trace/trace.c +8 -2

+	if (!ptr)
 		return NULL;
 
 	ptr++;
 
 	(*pos)++;
 
-	if (!ptr->enum_string)
-		return NULL;
+	ptr = update_enum_map(ptr);
 
 	return ptr;
 }
 
 
 
 
+	/*
+	 * 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;
 	}
 

kernel/trace/trace.c static const struct file_operations tracing_enum_map_fops = { · line 4060

Quoted 9 lines

kernel/trace/trace.c +8 -1

-	trace_enum_maps = map_array;
+	mutex_lock(&trace_enum_mutex);
+
+	if (!trace_enum_maps)
+		trace_enum_maps = map_array;
+	else {
+		ptr = trace_enum_maps;
+		for (;;) {
+			ptr = ptr + ptr->head.length + 1;

Looks like access to tail item is used frequently. How about adding a helper function like enum_maps_tail_item()?

From: "Steven Rostedt (Red Hat)" <[email protected]>

Update the infrastructure such that modules that declare TRACE_DEFINE_ENUM() will have those enums shown in the enum_map files in the tracing directory.

Cc Rusty Russell <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 include/linux/module.h |   2 +
 kernel/module.c        |   3 +
 kernel/trace/trace.c   | 184 ++++++++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 173 insertions(+), 16 deletions(-)

include/linux/module.h +2 -0

index 42999fe2dbd0..53dc41dd5c62 100644
@@ -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;

kernel/module.c +3 -0

index b3d634ed06c9..d8f8ab271c2b 100644
@@ -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",

kernel/trace/trace.c +168 -16

index 53b449c522a7..97031878eb3d 100644
@@ -124,7 +124,38 @@ enum ftrace_dump_mode ftrace_dump_on_oops;
 int __disable_trace_on_warning;
 
 /* Map of enums to their values, for "enum_map" file */
-static struct trace_enum_map *trace_enum_maps;
+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"
+	 */
+	const char			*end;	/* points to NULL */
+	union trace_enum_map_item	*next;
+};
+
+static DEFINE_MUTEX(trace_enum_mutex);
+
+/*
+ * The trace_enum_maps are saved in an array whith 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;
 
 static int tracing_set_tracer(struct trace_array *tr, const char *buf);
 

@@ -3911,29 +3942,47 @@ static const struct file_operations tracing_saved_cmdlines_size_fops = {
 	.write		= tracing_saved_cmdlines_size_write,
 };
 
+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;
+			ptr++;
+		} else
+			return NULL;
+	}
+	return ptr;
+}
+
 static void *enum_map_next(struct seq_file *m, void *v, loff_t *pos)
 {
-	struct trace_enum_map *ptr = v;
+	union trace_enum_map_item *ptr = v;
 
-	if (!ptr->enum_string)
+	ptr = update_enum_map(ptr);
+	if (!ptr)
 		return NULL;
 
 	ptr++;
 
 	(*pos)++;
 
-	if (!ptr->enum_string)
-		return NULL;
+	ptr = update_enum_map(ptr);
 
 	return ptr;
 }
 
 static void *enum_map_start(struct seq_file *m, loff_t *pos)
 {
-	struct trace_enum_map *v;
+	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);
 	}

@@ -3943,13 +3992,15 @@ static void *enum_map_start(struct seq_file *m, loff_t *pos)
 
 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)
 {
-	struct trace_enum_map *ptr = v;
+	union trace_enum_map_item *ptr = v;
+
+	seq_printf(m, "%s %ld\n", ptr->map.enum_string, ptr->map.enum_value);
 
-	seq_printf(m, "%s %ld\n", ptr->enum_string, ptr->enum_value);
 	return 0;
 }
 

@@ -3975,29 +4026,56 @@ static const struct file_operations tracing_enum_map_fops = {
 	.release	= seq_release,
 };
 
-static void
-trace_insert_enum_map(struct trace_enum_map **start, struct trace_enum_map **stop)
+static void trace_insert_enum_map(struct module *mod,
+				  struct trace_enum_map **start, int len)
 {
+	struct trace_enum_map **stop;
 	struct trace_enum_map **map;
-	struct trace_enum_map *map_array;
-	int len = stop - start;
+	union trace_enum_map_item *map_array;
+	union trace_enum_map_item *ptr;
 
 	if (len <= 0)
 		return;
 
-	map_array = kmalloc(sizeof(*map_array) * (len + 1), GFP_KERNEL);
+	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;
 	}
 
-	trace_enum_maps = map_array;
+	mutex_lock(&trace_enum_mutex);
+
+	if (!trace_enum_maps)
+		trace_enum_maps = map_array;
+	else {
+		ptr = trace_enum_maps;
+		for (;;) {
+			ptr = ptr + ptr->head.length + 1;
+			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_array->map = **map;
 		map_array++;
 	}
 	memset(map_array, 0, sizeof(*map_array));
+
+	mutex_unlock(&trace_enum_mutex);
 }
 
 

@@ -6640,9 +6718,79 @@ 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(NULL, __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, mod->trace_enums, mod->num_trace_enums);
+}
+
+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 += map->head.length + 1;
+		last = &map->tail.next;
+		map = map->tail.next;
+	}
+	if (!map)
+		goto out;
+
+	*last = map[map->head.length + 1].tail.next;
+	kfree(map);
+ out:
+	mutex_unlock(&trace_enum_mutex);
+}
+
+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;
+	case MODULE_STATE_GOING:
+		trace_module_remove_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;

@@ -6672,6 +6820,10 @@ static __init int tracer_init_debugfs(void)
 	trace_create_file("enum_map", 0444, d_tracer,
 			  NULL, &tracing_enum_map_fops);
 
+#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);

Signature · 1 line

2.1.4

From: "Steven Rostedt (Red Hat)" <[email protected]>

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 <[email protected]>

Cc Pankaj Kumar <[email protected]>

Cc Jaegeuk Kim <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 include/trace/events/f2fs.h | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

include/trace/events/f2fs.h +21 -0

index 5422dbfaf97d..a0074dd8d140 100644
@@ -9,6 +9,27 @@
 #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_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(__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);
+
 #define show_block_type(type)						\
 	__print_symbolic(type,						\
 		{ NODE,		"NODE" },				\

Signature · 8 lines

2.1.4


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Quoted 3 lines

Cc Namjae Jeon <[email protected]>

Cc Pankaj Kumar <[email protected]>

Cc Jaegeuk Kim <[email protected]>

AFAIK Jaegeuk's samsung email is not valid anymore..

  $ grep -a Jaegeuk MAINTAINERS
  M:  Jaegeuk Kim <[email protected]>

From: "Steven Rostedt (Red Hat)" <[email protected]>

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 <[email protected]>

Cc Ingo Molnar <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

include/trace/events/v4l2.h +5 -0

+
+#define SHOW_FIELD							\
+	EM( V4L2_FIELD_ANY,		"ANY" )				\
+	EM( V4L2_FIELD_NONE,		"NONE" )			\
+	EM( V4L2_FIELD_TOP,		"TOP" )				\

include/trace/events/v4l2.h line 35

Quoted 13 lines · Steven Rostedt

#define show_field(field) \

include/trace/events/v4l2.h +1 -11

-	__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

Hi Steve,

It is missing a right bracket ')' here.

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 export the TLB_REMOTE_SHOOTDOWN enum to use space.

How that is done is with a new file in the debugfs tracing directory.

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

Now userspace, when seeing TLB_REMOTE_SHOOTDOWN in the format, knows that represents "1", and can do the conversions properly.

Local SHA1: 6115e48a506e738f0f24771b4c87e4ba90d3d1c2

Steven Rostedt (Red Hat) (10):
tracing: Add TRACE_DEFINE_ENUM() macro to map enums to their values
tracing: Allow for modules to export their trace enums as well 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

----

 include/asm-generic/vmlinux.lds.h |   5 +-
 include/linux/module.h            |   2 +
 include/linux/tracepoint.h        |   7 +
 include/trace/events/9p.h         | 157 +++++++++++++----------
 include/trace/events/f2fs.h       |  21 +++
 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            |  11 ++
 kernel/module.c                   |   3 +
 kernel/trace/trace.c              | 260 +++++++++++++++++++++++++++++++++++++-
 14 files changed, 598 insertions(+), 149 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Quoted 9 lines · 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:

Quoted 6 lines

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

Hmm, would user-space application really need to know the symbol name of enums? If not, the event format files would better export the number(value) instead of the enum name, like below.

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

I'm still not sure how we can code it :( (It seems that some trick we need when showing the print fmt.)

Quoted 13 lines

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 export the TLB_REMOTE_SHOOTDOWN enum to use space.

How that is done is with a new file in the debugfs tracing directory.

Quoted 5 lines

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

BTW, if we can show the enum_map, can we also show the "symbolic" map instead of using the __print_symbolic() ? :)

Quoted 23 lines · On Mon, 30 Mar 2015 12:38:15 +0900 Masami Hiramatsu…

Quoted 9 lines · 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:

Quoted 6 lines

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

Hmm, would user-space application really need to know the symbol name of enums? If not, the event format files would better export the number(value) instead of the enum name, like below.

print fmt: "pages:%ld reason:%s (%d)", REC->pages,
__print_symbolic(REC->reason,

Quoted 7 lines

    { 0, "flush on task switch" },
    { 1, "remote shootdown" },
    { 2, "local shootdown" },
    { 3, "local mm shootdown" }), REC->reason

I'm still not sure how we can code it :( (It seems that some trick we need when showing the print fmt.)

Believe me, I've tried tons of tricks. I even pulled out my "Elder MACRO Wand", and it too could not execute the enum illuminous valueous (to show both the enum name and value in the same output).

The problem is that an enum name is only known by the compiler itself. The preprocessor does not know what an enum is. And after the compiler is done, the enum name no longer exists, just its value.

Thus, I found no way to have print_fmt display the numbers instead of the names.

Instead of adding an enum mapping file, I could add a way to look at all the events in the system that defined a mapping, and do a "s/ENUM_NAME/ENUM_VALUE/g" do the saved print formats? I'm not sure how much we want to do that in the kernel though.

Quoted 11 lines

Quoted 11 lines

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 export the TLB_REMOTE_SHOOTDOWN enum to use space.

How that is done is with a new file in the debugfs tracing directory.

Quoted 8 lines

Quoted 5 lines

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

BTW, if we can show the enum_map, can we also show the "symbolic" map instead of using the __print_symbolic() ? :)

There's nothing mapping the two in the kernel. And worse yet, some enums are used in operations. Just look at f2fs_submit_read_bio, where in the __print_symbolic() it has:

	(1ULL << __REQ_NOIDLE) | ...

the __REQ_NOIDLE is an enum.

But, if I do just a substitution, then we wont even have to update userspace tools. They should still work.

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Quoted 4 lines

Instead of adding an enum mapping file, I could add a way to look at all the events in the system that defined a mapping, and do a "s/ENUM_NAME/ENUM_VALUE/g" do the saved print formats? I'm not sure how much we want to do that in the kernel though.

No, it's not what I expected...

What I thought was expanding __print_symbolic() macro in TP_printk with a special hash string(start with #), and when showing it via event/format, replace the hash string with the strings generated by the map of symbols. This will introduce a small overhead to show the format as a side effect.

Actually I even have not tried, so it's just an idea yet.

Quoted 5 lines · On Tue, 31 Mar 2015 16:36:51 +0900 Masami Hiramatsu…

Quoted 4 lines

Instead of adding an enum mapping file, I could add a way to look at all the events in the system that defined a mapping, and do a "s/ENUM_NAME/ENUM_VALUE/g" do the saved print formats? I'm not sure how much we want to do that in the kernel though.

Quoted 1 line

No, it's not what I expected...

Actually, I'm going to play with this idea and see how crazy it is or isn't. Converting the ENUMS in TP_printk() might actually be doable.

I'll write up some code and we can see if it seems sane or not. OK?

Quoted 8 lines

What I thought was expanding __print_symbolic() macro in TP_printk with a special hash string(start with #), and when showing it via event/format, replace the hash string with the strings generated by the map of symbols. This will introduce a small overhead to show the format as a side effect.

Actually I even have not tried, so it's just an idea yet.

If you can figure something out, please let me know!

Thanks,

-- Steve

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.

Steven Rostedt (Red Hat) (10):
tracing: Add TRACE_DEFINE_ENUM() macro to map enums to their values
tracing: Allow for modules to export their trace enums as well 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

Quoted 6 lines · 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.

.....

Quoted 11 lines

Steven Rostedt (Red Hat) (10):
tracing: Add TRACE_DEFINE_ENUM() macro to map enums to their values
tracing: Allow for modules to export their trace enums as well 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

So are you expecting subsystem maintainers to modify their tracing code to do this, or is this just the first round of changes you are making? e.g:

$ grep print_symbolic fs/xfs/xfs_trace.h
                  __print_symbolic(__entry->trans_type, XFS_TRANS_TYPES),
                  __print_symbolic(__entry->type, XFS_LI_TYPE_DESC),
                  __print_symbolic(__entry->type, XFS_LI_TYPE_DESC),
                  __print_symbolic(__entry->type, XFS_IO_TYPES),
                  __print_symbolic(__entry->type, XFS_ALLOC_TYPES),
                  __print_symbolic(__entry->otype, XFS_ALLOC_TYPES),
                  __print_symbolic(__entry->which, XFS_SWAPEXT_INODES),
                  __print_symbolic(__entry->format, XFS_INODE_FORMAT_STR),
                  __print_symbolic(__entry->type, XFS_LI_TYPE_DESC),

$

And a quick git grep indicates there are a lot that you haven't converted in this patchset...