Re: [PATCH v4 06/16] perf tools: Support perf event alias name

Arnaldo Carvalho de Melo
2015-12-11T12:03:10Z

Em Tue, Dec 08, 2015 at 02:25:34AM +0000, Wang Nan escreveu:
> From: He Kuang <hekuang@huawei.com>
>
> 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 <hekuang@huawei.com>
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Zefan Li <lizefan@huawei.com>
> 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