Messages in this thread | | | Date | Mon, 30 Mar 2015 11:27:35 +0900 | From | Namhyung Kim <> | Subject | Re: [RFC][PATCH 01/10] tracing: Add TRACE_DEFINE_ENUM() macro to map enums to their values |
| |
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/
| |