From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753140AbbDBHyO (ORCPT ); Thu, 2 Apr 2015 03:54:14 -0400 Received: from LGEMRELSE7Q.lge.com ([156.147.1.151]:35510 "EHLO lgemrelse7q.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752654AbbDBHyL (ORCPT ); Thu, 2 Apr 2015 03:54:11 -0400 X-Original-SENDERIP: 10.177.220.203 X-Original-MAILFROM: namhyung@kernel.org Date: Thu, 2 Apr 2015 16:47:40 +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 06/17 v2] tracing: Add TRACE_DEFINE_ENUM() macro to map enums to their values Message-ID: <20150402074740.GA23913@sejong> References: <20150402015648.249824760@goodmis.org> <20150402020940.953163951@goodmis.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20150402020940.953163951@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 Wed, Apr 01, 2015 at 09:56:54PM -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 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 > Cc: Tony Luck > Cc: Xie XiuQi > Signed-off-by: Steven Rostedt [SNIP] > diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c > index db54dda10ccc..6b7fd0bf5d28 100644 > --- a/kernel/trace/trace_events.c > +++ b/kernel/trace/trace_events.c > @@ -1704,6 +1704,102 @@ __register_event(struct ftrace_event_call *call, struct module *mod) > return 0; > } > > +static char *enum_replace(char *ptr, struct trace_enum_map *map, int len) > +{ > + int rlen; > + int elen; > + > + /* Find the length of the enum value as a string */ > + elen = snprintf(ptr, 0, "%ld", map->enum_value); > + /* Make sure there's enough room to replace the string with the value */ > + if (len < elen) > + return NULL; > + > + snprintf(ptr, elen + 1, "%ld", map->enum_value); > + > + /* Get the rest of the string of ptr */ > + rlen = strlen(ptr + len); > + memmove(ptr + elen, ptr + len, rlen); > + /* Make sure we end the new string */ > + ptr[elen + rlen] = 0; > + > + return ptr + elen; > +} > + > +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++; > + } while (isalnum(*ptr) || *ptr == '_'); > + /* we went one too many */ Hmm.. it looks like to skip variable name after a number. Shouldn't it be do { ptr++; } while (isdigit(*ptr)); ? > + ptr--; > + 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; > + continue; I guess it also needs to decrease the ptr here. > + } > + do { > + ptr++; > + } while (isalnum(*ptr) || *ptr == '_'); > + /* we went one too many */ > + ptr--; > + 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) > + last_i = 0; I think it needs to update the last_system here. Thanks, Namhyung > + > + 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 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/