Hi Steve, On Thu, Apr 02, 2015 at 09:38:09PM -0400, Steven Rostedt wrote: > From: "Steven Rostedt (Red Hat)" <rost...@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 <c...@computer.org> > Cc: Tony Luck <tony.l...@gmail.com> > Cc: Xie XiuQi <xiexi...@huawei.com> > Signed-off-by: Steven Rostedt <rost...@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 > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/