From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752696AbbC3CeV (ORCPT ); Sun, 29 Mar 2015 22:34:21 -0400 Received: from LGEMRELSE7Q.lge.com ([156.147.1.151]:38774 "EHLO lgemrelse7q.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752322AbbC3CeT (ORCPT ); Sun, 29 Mar 2015 22:34:19 -0400 X-Original-SENDERIP: 10.177.220.203 X-Original-MAILFROM: namhyung@kernel.org Date: Mon, 30 Mar 2015 11:27:35 +0900 From: Namhyung Kim To: Steven Rostedt Cc: linux-kernel@vger.kernel.org, Ingo Molnar , Andrew Morton , Masami Hiramatsu , Mathieu Desnoyers , Guilherme Cox , Tony Luck , Xie XiuQi Subject: Re: [RFC][PATCH 01/10] tracing: Add TRACE_DEFINE_ENUM() macro to map enums to their values Message-ID: <20150330022735.GA32033@sejong> References: <20150327213704.857765144@goodmis.org> <20150327213813.205684832@goodmis.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20150327213813.205684832@goodmis.org> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Steve, On Fri, Mar 27, 2015 at 05:37:05PM -0400, Steven Rostedt wrote: > From: "Steven Rostedt (Red Hat)" > > 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 > Cc: Tony Luck > Cc: Xie XiuQi > Signed-off-by: Steven Rostedt > --- [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/