Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753569AbbLKMDK (ORCPT ); Fri, 11 Dec 2015 07:03:10 -0500 Received: from mail.kernel.org ([198.145.29.136]:45709 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751996AbbLKMDH (ORCPT ); Fri, 11 Dec 2015 07:03:07 -0500 Date: Fri, 11 Dec 2015 09:03:01 -0300 From: Arnaldo Carvalho de Melo To: Wang Nan , Jiri Olsa Cc: namhyung@kernel.org, linux-kernel@vger.kernel.org, pi3orama@163.com, mingo@kernel.org, lizefan@huawei.com, He Kuang , Alexei Starovoitov , Masami Hiramatsu Subject: Re: [PATCH v4 06/16] perf tools: Support perf event alias name Message-ID: <20151211120301.GO17996@kernel.org> References: <1449541544-67621-1-git-send-email-wangnan0@huawei.com> <1449541544-67621-7-git-send-email-wangnan0@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1449541544-67621-7-git-send-email-wangnan0@huawei.com> X-Url: http://acmel.wordpress.com 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 Content-Length: 8795 Lines: 279 Em Tue, Dec 08, 2015 at 02:25:34AM +0000, Wang Nan escreveu: > From: He Kuang > > This patch adds new bison rules for specifying an alias name to a perf > event, which allows cmdline refer to previous defined perf event through > its name. With this patch user can give alias name to a perf event using > following cmdline: Please add Jiri Olsa to any changes that touches the parser changes. Can you reword the above phrase? Does it mean something like: ---- This patches adds new bison rules for specifying an alias to a perf event, which allows referring to this previously defined perf event through this alias." ---- But then, why would I want this aliasing? The provided examples shows a way to obfuscate 'cycles', a perfectly good name, why would one want to call it "mypmu"? > # perf record -e mypmu=cycles ... > > If alias is not provided (normal case): > > # perf record -e cycles ... > > It will be set to event's name automatically ('cycles' in the above > example). Sure, but when would I use 'mypmu'? > To allow parser refer to existing event selector, pass event list to > 'struct parse_events_evlist'. perf_evlist__find_evsel_by_alias() is > introduced to get evsel through its alias. > > Test result: > > Before this patch: > > # ./perf record -e evt=cycles usleep 10 > event syntax error: 'evt=cycles' > \___ parser error > Run 'perf list' for a list of valid events > [SNIP] Sure, before this patch aliases are not supported, so it will fail. > After this patch: > > # ./perf record -e evt=cycles usleep 10 > [ perf record: Woken up 1 times to write data ] > [ perf record: Captured and wrote 0.012 MB perf.data (2 samples) ] So? What are the effects on the output of 'perf evlist'? I guess it will appear just as 'cycles'? Can you provide at least an example where we _use_ this alias and by doing that we gain something? - Arnaldo > Signed-off-by: He Kuang > Signed-off-by: Wang Nan > Cc: Alexei Starovoitov > Cc: Arnaldo Carvalho de Melo > Cc: Masami Hiramatsu > Cc: Namhyung Kim > Cc: Zefan Li > Cc: pi3orama@163.com > --- > tools/perf/util/evlist.c | 16 ++++++++++++++++ > tools/perf/util/evlist.h | 4 ++++ > tools/perf/util/evsel.c | 1 + > tools/perf/util/evsel.h | 1 + > tools/perf/util/parse-events.c | 37 ++++++++++++++++++++++++++++++++----- > tools/perf/util/parse-events.h | 5 +++++ > tools/perf/util/parse-events.y | 15 ++++++++++++++- > 7 files changed, 73 insertions(+), 6 deletions(-) > > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c > index d1b6c20..a822823 100644 > --- a/tools/perf/util/evlist.c > +++ b/tools/perf/util/evlist.c > @@ -1737,3 +1737,19 @@ void perf_evlist__set_tracking_event(struct perf_evlist *evlist, > > tracking_evsel->tracking = true; > } > + > +struct perf_evsel * > +perf_evlist__find_evsel_by_alias(struct perf_evlist *evlist, > + const char *alias) > +{ > + struct perf_evsel *evsel; > + > + evlist__for_each(evlist, evsel) { > + if (!evsel->alias) > + continue; > + if (strcmp(alias, evsel->alias) == 0) > + return evsel; > + } > + > + return NULL; > +} > diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h > index a459fe7..4e25342 100644 > --- a/tools/perf/util/evlist.h > +++ b/tools/perf/util/evlist.h > @@ -292,4 +292,8 @@ void perf_evlist__set_tracking_event(struct perf_evlist *evlist, > struct perf_evsel *tracking_evsel); > > void perf_event_attr__set_max_precise_ip(struct perf_event_attr *attr); > + > +struct perf_evsel * > +perf_evlist__find_evsel_by_alias(struct perf_evlist *evlist, const char *alias); > + > #endif /* __PERF_EVLIST_H */ > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c > index 47f0330..8e0e6f4 100644 > --- a/tools/perf/util/evsel.c > +++ b/tools/perf/util/evsel.c > @@ -1073,6 +1073,7 @@ void perf_evsel__exit(struct perf_evsel *evsel) > thread_map__put(evsel->threads); > zfree(&evsel->group_name); > zfree(&evsel->name); > + zfree(&evsel->alias); > perf_evsel__object.fini(evsel); > } > > diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h > index 5ded1fc..5f6dd57 100644 > --- a/tools/perf/util/evsel.h > +++ b/tools/perf/util/evsel.h > @@ -89,6 +89,7 @@ struct perf_evsel { > int idx; > u32 ids; > char *name; > + char *alias; > double scale; > const char *unit; > struct event_format *tp_format; > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c > index 95775fe..484c8e4 100644 > --- a/tools/perf/util/parse-events.c > +++ b/tools/perf/util/parse-events.c > @@ -653,9 +653,9 @@ parse_events_config_bpf(struct parse_events_evlist *data, > return -EINVAL; > } > > - err = bpf__config_obj(obj, term, NULL, &error_pos); > + err = bpf__config_obj(obj, term, data->evlist, &error_pos); > if (err) { > - bpf__strerror_config_obj(obj, term, NULL, > + bpf__strerror_config_obj(obj, term, data->evlist, > &error_pos, err, errbuf, > sizeof(errbuf)); > data->error->help = strdup( > @@ -1089,6 +1089,30 @@ int parse_events__modifier_group(struct list_head *list, > return parse_events__modifier_event(list, event_mod, true); > } > > +int parse_events__set_event_alias(struct parse_events_evlist *data, > + struct list_head *list, > + const char *str, > + void *loc_alias_) > +{ > + struct perf_evsel *evsel; > + YYLTYPE *loc_alias = loc_alias_; > + > + if (!str) > + return 0; > + > + if (!list_is_singular(list)) { > + struct parse_events_error *err = data->error; > + > + err->idx = loc_alias->first_column; > + err->str = strdup("One alias can be applied to one event only"); > + return -EINVAL; > + } > + > + evsel = list_first_entry(list, struct perf_evsel, node); > + evsel->alias = strdup(str); > + return evsel->alias ? 0 : -ENOMEM; > +} > + > void parse_events__set_leader(char *name, struct list_head *list) > { > struct perf_evsel *leader; > @@ -1281,6 +1305,8 @@ int parse_events_name(struct list_head *list, char *name) > __evlist__for_each(list, evsel) { > if (!evsel->name) > evsel->name = strdup(name); > + if (!evsel->alias) > + evsel->alias = strdup(name); > } > > return 0; > @@ -1442,9 +1468,10 @@ int parse_events(struct perf_evlist *evlist, const char *str, > struct parse_events_error *err) > { > struct parse_events_evlist data = { > - .list = LIST_HEAD_INIT(data.list), > - .idx = evlist->nr_entries, > - .error = err, > + .list = LIST_HEAD_INIT(data.list), > + .idx = evlist->nr_entries, > + .error = err, > + .evlist = evlist, > }; > int ret; > > diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h > index 84694f3..20ad3c2 100644 > --- a/tools/perf/util/parse-events.h > +++ b/tools/perf/util/parse-events.h > @@ -98,6 +98,7 @@ struct parse_events_evlist { > int idx; > int nr_groups; > struct parse_events_error *error; > + struct perf_evlist *evlist; > }; > > struct parse_events_terms { > @@ -171,4 +172,8 @@ extern int is_valid_tracepoint(const char *event_string); > int valid_event_mount(const char *eventfs); > char *parse_events_formats_error_string(char *additional_terms); > > +int parse_events__set_event_alias(struct parse_events_evlist *data, > + struct list_head *list, > + const char *str, > + void *loc_alias_); > #endif /* __PERF_PARSE_EVENTS_H */ > diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y > index 8992d16..c3cbd7a 100644 > --- a/tools/perf/util/parse-events.y > +++ b/tools/perf/util/parse-events.y > @@ -77,6 +77,7 @@ static inc_group_count(struct list_head *list, > %type event_bpf_file > %type event_def > %type event_mod > +%type event_alias > %type event_name > %type event > %type events > @@ -193,13 +194,25 @@ event_name PE_MODIFIER_EVENT > event_name > > event_name: > -PE_EVENT_NAME event_def > +PE_EVENT_NAME event_alias > { > ABORT_ON(parse_events_name($2, $1)); > free($1); > $$ = $2; > } > | > +event_alias > + > +event_alias: > +PE_NAME '=' event_def > +{ > + struct list_head *list = $3; > + struct parse_events_evlist *data = _data; > + > + ABORT_ON(parse_events__set_event_alias(data, list, $1, &@1)); > + $$ = list; > +} > +| > event_def > > event_def: event_pmu | > -- > 1.8.3.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/