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

[Arnaldo Carvalho de Melo <acme@kernel.org>]

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 <head> event_bpf_file
>  %type <head> event_def
>  %type <head> event_mod
> +%type <head> event_alias
>  %type <head> event_name
>  %type <head> event
>  %type <head> 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