Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754272AbbDRS4E (ORCPT ); Sat, 18 Apr 2015 14:56:04 -0400 Received: from mail-qg0-f41.google.com ([209.85.192.41]:34027 "EHLO mail-qg0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753793AbbDRS4B convert rfc822-to-8bit (ORCPT ); Sat, 18 Apr 2015 14:56:01 -0400 MIME-Version: 1.0 In-Reply-To: <1429372220-6406-7-git-send-email-jolsa@kernel.org> References: <1429372220-6406-1-git-send-email-jolsa@kernel.org> <1429372220-6406-7-git-send-email-jolsa@kernel.org> Date: Sat, 18 Apr 2015 14:56:00 -0400 X-Google-Sender-Auth: pWAuvnXcIzZKrThXCLwJ8OtkOuk Message-ID: Subject: Re: [PATCH 6/7] perf data: Fix duplicate field names and avoid reserved keywords From: =?UTF-8?Q?J=C3=A9r=C3=A9mie_Galarneau?= To: Jiri Olsa Cc: Arnaldo Carvalho de Melo , David Ahern , Frederic Weisbecker , He Kuang , Ingo Molnar , Jeremie Galarneau , Namhyung Kim , Paul Mackerras , Peter Zijlstra , Sebastian Andrzej Siewior , Tom Zanussi , Wang Nan , lkml Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6541 Lines: 178 On Sat, Apr 18, 2015 at 11:50 AM, Jiri Olsa wrote: > From: Wang Nan > > Some parameters of syscall tracepoints named as 'nr', 'event', etc. > When dealing with them, perf convert to ctf meets some problem: > > 1. If a parameter with name 'nr', it will duplicate syscall's > common field 'nr'. One such syscall is io_submit(). > > 2. If a parameter with name 'event', it is denied to be inserted > because 'event' is a babeltrace keywork. One such syscall is > epoll_ctl. Minor nitpick: "event" is a reserved keyword in the CTF specification [1], the rule is only enforced by Babeltrace. Also, keywork -> keyword. Jérémie [1] http://diamon.org/docs/ctf/v1.8.2/#specC.1.2 > > This patch appends '_dupl_X' suffix to avoid problem 1, prepend a '_' > prefix to avoid problem 2. > > Signed-off-by: Wang Nan > Link: http://lkml.kernel.org/n/tip-4tlh4s2u3xnxr76d1w7qmckb@git.kernel.org > [ changed to use format_file::alias ] > Signed-off-by: Jiri Olsa > --- > tools/perf/util/data-convert-bt.c | 86 +++++++++++++++++++++++++++++++++++++-- > 1 file changed, 82 insertions(+), 4 deletions(-) > > diff --git a/tools/perf/util/data-convert-bt.c b/tools/perf/util/data-convert-bt.c > index b35c8d6c291d..8eda4ed628e7 100644 > --- a/tools/perf/util/data-convert-bt.c > +++ b/tools/perf/util/data-convert-bt.c > @@ -14,6 +14,7 @@ > #include > #include > #include > +#include > #include > #include > #include "asm/bug.h" > @@ -184,6 +185,7 @@ static int add_tracepoint_field_value(struct ctf_writer *cw, > unsigned int len; > int ret; > > + name = fmtf->alias; > offset = fmtf->offset; > len = fmtf->size; > if (flags & FIELD_IS_STRING) > @@ -567,6 +569,82 @@ static int process_sample_event(struct perf_tool *tool, > return cs ? 0 : -1; > } > > +/* If dup < 0, add a prefix. Else, add _dupl_X suffix. */ > +static char *change_name(char *name, char *orig_name, int dup) > +{ > + char *new_name = NULL; > + size_t len; > + > + if (!name) > + name = orig_name; > + > + if (dup >= 10) > + goto out; > + /* > + * Add '_' prefix to potential keywork. According to > + * Mathieu Desnoyers (https://lkml.org/lkml/2015/1/23/652), > + * futher CTF spec updating may require us to use '$'. > + */ > + if (dup < 0) > + len = strlen(name) + sizeof("_"); > + else > + len = strlen(orig_name) + sizeof("_dupl_X"); > + > + new_name = malloc(len); > + if (!new_name) > + goto out; > + > + if (dup < 0) > + snprintf(new_name, len, "_%s", name); > + else > + snprintf(new_name, len, "%s_dupl_%d", orig_name, dup); > + > +out: > + if (name != orig_name) > + free(name); > + return new_name; > +} > + > +static int event_class_add_field(struct bt_ctf_event_class *event_class, > + struct bt_ctf_field_type *type, > + struct format_field *field) > +{ > + struct bt_ctf_field_type *t = NULL; > + char *name; > + int dup = 1; > + int ret; > + > + /* alias was already assigned */ > + if (field->alias != field->name) > + return bt_ctf_event_class_add_field(event_class, type, > + (char *)field->alias); > + > + name = field->name; > + > + /* If 'name' is a keywork, add prefix. */ > + if (bt_ctf_validate_identifier(name)) > + name = change_name(name, field->name, -1); > + > + if (!name) { > + pr_err("Failed to fix invalid identifier."); > + return -1; > + } > + while ((t = bt_ctf_event_class_get_field_by_name(event_class, name))) { > + bt_ctf_field_type_put(t); > + name = change_name(name, field->name, dup++); > + if (!name) { > + pr_err("Failed to create dup name for '%s'\n", field->name); > + return -1; > + } > + } > + > + ret = bt_ctf_event_class_add_field(event_class, type, name); > + if (!ret) > + field->alias = name; > + > + return ret; > +} > + > static int add_tracepoint_fields_types(struct ctf_writer *cw, > struct format_field *fields, > struct bt_ctf_event_class *event_class) > @@ -595,14 +673,14 @@ static int add_tracepoint_fields_types(struct ctf_writer *cw, > if (flags & FIELD_IS_ARRAY) > type = bt_ctf_field_type_array_create(type, field->arraylen); > > - ret = bt_ctf_event_class_add_field(event_class, type, > - field->name); > + ret = event_class_add_field(event_class, type, field); > > if (flags & FIELD_IS_ARRAY) > bt_ctf_field_type_put(type); > > if (ret) { > - pr_err("Failed to add field '%s\n", field->name); > + pr_err("Failed to add field '%s': %d\n", > + field->name, ret); > return -1; > } > } > @@ -646,7 +724,7 @@ static int add_generic_types(struct ctf_writer *cw, struct perf_evsel *evsel, > do { \ > pr2(" field '%s'\n", n); \ > if (bt_ctf_event_class_add_field(cl, t, n)) { \ > - pr_err("Failed to add field '%s;\n", n); \ > + pr_err("Failed to add field '%s';\n", n); \ > return -1; \ > } \ > } while (0) > -- > 1.9.3 > -- Jérémie Galarneau EfficiOS Inc. http://www.efficios.com -- 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/