Re: [RFC][PATCH 01/10] tracing: Add TRACE_DEFINE_ENUM() macro to map enums to their values

Namhyung Kim
2015-03-30T02:34:21Z

Hi Steve,

On Fri, Mar 27, 2015 at 05:37:05PM -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 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 <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]
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 62c6506d663f..53b449c522a7 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -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
> @@ -3908,6 +3911,96 @@ static const struct file_operations tracing_saved_cmdlines_size_fops = {
> .write = tracing_saved_cmdlines_size_write,
> };
>
> +static void *enum_map_next(struct seq_file *m, void *v, loff_t *pos)
> +{
> + struct trace_enum_map *ptr = v;
> +
> + if (!ptr->enum_string)
> + return NULL;

Do we really need to check NULL string here? Seems like duplicated..

Thanks,
Namhyung

> +
> + 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)
> --
> 2.1.4
>
>
> --
> 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/