diff mbox

[26/31] perf tools: Support perf event alias name

Message ID 1444826502-49291-27-git-send-email-wangnan0@huawei.com
State New
Headers show

Commit Message

Wang Nan Oct. 14, 2015, 12:41 p.m. UTC
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:

 # perf record -e mypmu=cycles ...

To allow parser refer to existing event selecter, pass event list to
'struct parse_events_evlist'. perf_evlist__find_evsel_by_alias() is
introduced to get evsel through its alias.

Signed-off-by: He Kuang <hekuang@huawei.com>
Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexei Starovoitov <ast@plumgrid.com>
Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David Ahern <dsahern@gmail.com>
Cc: He Kuang <hekuang@huawei.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kaixu Xia <xiakaixu@huawei.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/n/ebpf-7w1s62o0s6ovqlaqwrmx20v9@git.kernel.org
---
 tools/perf/util/evlist.c       | 16 ++++++++++++++++
 tools/perf/util/evlist.h       |  4 ++++
 tools/perf/util/evsel.h        |  1 +
 tools/perf/util/parse-events.c | 31 ++++++++++++++++++++++++++++---
 tools/perf/util/parse-events.h |  5 +++++
 tools/perf/util/parse-events.y | 15 ++++++++++++++-
 6 files changed, 68 insertions(+), 4 deletions(-)

Comments

Namhyung Kim Oct. 21, 2015, 8:53 a.m. UTC | #1
Hi,

On Wed, Oct 14, 2015 at 12:41:37PM +0000, Wang Nan wrote:
> 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:
> 
>  # perf record -e mypmu=cycles ...
> 
> To allow parser refer to existing event selecter, pass event list to
> 'struct parse_events_evlist'. perf_evlist__find_evsel_by_alias() is
> introduced to get evsel through its alias.

What about using event name directly?  I guess the alias name is used
only to refer an event so it'd be better to use the event name.
Anyway we need alias as well when event has no name or name is complex.

Thanks
Namhyung


> 
> Signed-off-by: He Kuang <hekuang@huawei.com>
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Alexei Starovoitov <ast@plumgrid.com>
> Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: He Kuang <hekuang@huawei.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Kaixu Xia <xiakaixu@huawei.com>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Zefan Li <lizefan@huawei.com>
> Cc: pi3orama@163.com
> Link: http://lkml.kernel.org/n/ebpf-7w1s62o0s6ovqlaqwrmx20v9@git.kernel.org
> ---
>  tools/perf/util/evlist.c       | 16 ++++++++++++++++
>  tools/perf/util/evlist.h       |  4 ++++
>  tools/perf/util/evsel.h        |  1 +
>  tools/perf/util/parse-events.c | 31 ++++++++++++++++++++++++++++---
>  tools/perf/util/parse-events.h |  5 +++++
>  tools/perf/util/parse-events.y | 15 ++++++++++++++-
>  6 files changed, 68 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index d139219..8dd59aa 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -1753,3 +1753,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.h b/tools/perf/util/evsel.h
> index a60b5d5..9a95e73 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -87,6 +87,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 4849dbd..06ba5a6 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -1020,6 +1020,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;
> @@ -1373,9 +1397,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 8f17c83..b525353 100644
> --- a/tools/perf/util/parse-events.h
> +++ b/tools/perf/util/parse-events.h
> @@ -96,6 +96,7 @@ struct parse_events_evlist {
>  	int			   idx;
>  	int			   nr_groups;
>  	struct parse_events_error *error;
> +	struct perf_evlist	  *evlist;
>  };
>  
>  struct parse_events_terms {
> @@ -168,4 +169,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 ad37996..90e382f 100644
> --- a/tools/perf/util/parse-events.y
> +++ b/tools/perf/util/parse-events.y
> @@ -76,6 +76,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
> @@ -192,13 +193,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/
Wang Nan Oct. 21, 2015, 1 p.m. UTC | #2
On 2015/10/21 16:53, Namhyung Kim wrote:
> Hi,
>
> On Wed, Oct 14, 2015 at 12:41:37PM +0000, Wang Nan wrote:
>> 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:
>>
>>   # perf record -e mypmu=cycles ...
>>
>> To allow parser refer to existing event selecter, pass event list to
>> 'struct parse_events_evlist'. perf_evlist__find_evsel_by_alias() is
>> introduced to get evsel through its alias.
> What about using event name directly?  I guess the alias name is used
> only to refer an event so it'd be better to use the event name.
> Anyway we need alias as well when event has no name or name is complex.

It is possible to trigger two perf events with same PMU but
different config options:

  # perf record -e cycles/period=9999/ -e cycles/period=99999/ -a sleep 1

In this case the name of events are:

cycles/period=9999/ `
cycles/period=99999/

Using it in perf cmdline is painful:

  # perf record -e cycles/period=9999/ -e cycles/period=99999/ -e 
bpf_program.c/myevent=cycles/period=9999//...

Thank you.

> Thanks
> Namhyung
>
>
>> Signed-off-by: He Kuang <hekuang@huawei.com>
>> Signed-off-by: Wang Nan <wangnan0@huawei.com>
>> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
>> Cc: Alexei Starovoitov <ast@plumgrid.com>
>> Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
>> Cc: Daniel Borkmann <daniel@iogearbox.net>
>> Cc: David Ahern <dsahern@gmail.com>
>> Cc: He Kuang <hekuang@huawei.com>
>> Cc: Jiri Olsa <jolsa@kernel.org>
>> Cc: Kaixu Xia <xiakaixu@huawei.com>
>> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
>> Cc: Zefan Li <lizefan@huawei.com>
>> Cc: pi3orama@163.com
>> Link: http://lkml.kernel.org/n/ebpf-7w1s62o0s6ovqlaqwrmx20v9@git.kernel.org
>> ---
>>   tools/perf/util/evlist.c       | 16 ++++++++++++++++
>>   tools/perf/util/evlist.h       |  4 ++++
>>   tools/perf/util/evsel.h        |  1 +
>>   tools/perf/util/parse-events.c | 31 ++++++++++++++++++++++++++++---
>>   tools/perf/util/parse-events.h |  5 +++++
>>   tools/perf/util/parse-events.y | 15 ++++++++++++++-
>>   6 files changed, 68 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
>> index d139219..8dd59aa 100644
>> --- a/tools/perf/util/evlist.c
>> +++ b/tools/perf/util/evlist.c
>> @@ -1753,3 +1753,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.h b/tools/perf/util/evsel.h
>> index a60b5d5..9a95e73 100644
>> --- a/tools/perf/util/evsel.h
>> +++ b/tools/perf/util/evsel.h
>> @@ -87,6 +87,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 4849dbd..06ba5a6 100644
>> --- a/tools/perf/util/parse-events.c
>> +++ b/tools/perf/util/parse-events.c
>> @@ -1020,6 +1020,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;
>> @@ -1373,9 +1397,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 8f17c83..b525353 100644
>> --- a/tools/perf/util/parse-events.h
>> +++ b/tools/perf/util/parse-events.h
>> @@ -96,6 +96,7 @@ struct parse_events_evlist {
>>   	int			   idx;
>>   	int			   nr_groups;
>>   	struct parse_events_error *error;
>> +	struct perf_evlist	  *evlist;
>>   };
>>   
>>   struct parse_events_terms {
>> @@ -168,4 +169,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 ad37996..90e382f 100644
>> --- a/tools/perf/util/parse-events.y
>> +++ b/tools/perf/util/parse-events.y
>> @@ -76,6 +76,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
>> @@ -192,13 +193,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/
Namhyung Kim Oct. 22, 2015, 7:16 a.m. UTC | #3
Hi,

On Wed, Oct 21, 2015 at 10:00 PM, Wangnan (F) <wangnan0@huawei.com> wrote:
>
>
> On 2015/10/21 16:53, Namhyung Kim wrote:
>>
>> Hi,
>>
>> On Wed, Oct 14, 2015 at 12:41:37PM +0000, Wang Nan wrote:
>>>
>>> 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:
>>>
>>>   # perf record -e mypmu=cycles ...
>>>
>>> To allow parser refer to existing event selecter, pass event list to
>>> 'struct parse_events_evlist'. perf_evlist__find_evsel_by_alias() is
>>> introduced to get evsel through its alias.
>>
>> What about using event name directly?  I guess the alias name is used
>> only to refer an event so it'd be better to use the event name.
>> Anyway we need alias as well when event has no name or name is complex.
>
>
> It is possible to trigger two perf events with same PMU but
> different config options:
>
>  # perf record -e cycles/period=9999/ -e cycles/period=99999/ -a sleep 1
>
> In this case the name of events are:
>
> cycles/period=9999/ `
> cycles/period=99999/
>
> Using it in perf cmdline is painful:
>
>  # perf record -e cycles/period=9999/ -e cycles/period=99999/ -e
> bpf_program.c/myevent=cycles/period=9999//...

I understand the need of using aliases but I think it's more natural
to use event name for simple cases..

Thanks,
Namhyung
--
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/
Wang Nan Oct. 22, 2015, 7:29 a.m. UTC | #4
On 2015/10/22 15:16, Namhyung Kim wrote:
> Hi,
>
> On Wed, Oct 21, 2015 at 10:00 PM, Wangnan (F) <wangnan0@huawei.com> wrote:
>>
>> On 2015/10/21 16:53, Namhyung Kim wrote:
>>> Hi,
>>>
>>> On Wed, Oct 14, 2015 at 12:41:37PM +0000, Wang Nan wrote:
>>>> 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:
>>>>
>>>>    # perf record -e mypmu=cycles ...
>>>>
>>>> To allow parser refer to existing event selecter, pass event list to
>>>> 'struct parse_events_evlist'. perf_evlist__find_evsel_by_alias() is
>>>> introduced to get evsel through its alias.
>>> What about using event name directly?  I guess the alias name is used
>>> only to refer an event so it'd be better to use the event name.
>>> Anyway we need alias as well when event has no name or name is complex.
>>
>> It is possible to trigger two perf events with same PMU but
>> different config options:
>>
>>   # perf record -e cycles/period=9999/ -e cycles/period=99999/ -a sleep 1
>>
>> In this case the name of events are:
>>
>> cycles/period=9999/ `
>> cycles/period=99999/
>>
>> Using it in perf cmdline is painful:
>>
>>   # perf record -e cycles/period=9999/ -e cycles/period=99999/ -e
>> bpf_program.c/myevent=cycles/period=9999//...
> I understand the need of using aliases but I think it's more natural
> to use event name for simple cases..

I will consider this. However, if we allow using event name directly 
like this:

  # perf record -e cycles -e test_pmu.c/myevent=cycles/ ...

Then two '-e' seems redundant, right? Why not directly using:

  # perf record -e test_pmu.c/myevent=cycles/ ...

and make perf creates cycles event for test_pmu.c?

We can make syntax like

  # perf record -e test_pmu.c/myevent=cycles/ ...

as a syntax sugar of

  # perf record -e randomname=cycles -e test_pmu.c/myevent=randomname/ ...

and don't need to find evsel through their names if alias not exist.

So this is a new feature, and worth another patch.

Thought?

> Thanks,
> Namhyung


--
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/
Namhyung Kim Oct. 22, 2015, 7:53 a.m. UTC | #5
On Thu, Oct 22, 2015 at 4:29 PM, Wangnan (F) <wangnan0@huawei.com> wrote:
>
>
> On 2015/10/22 15:16, Namhyung Kim wrote:
>>
>> Hi,
>>
>> On Wed, Oct 21, 2015 at 10:00 PM, Wangnan (F) <wangnan0@huawei.com> wrote:
>>>
>>>
>>> On 2015/10/21 16:53, Namhyung Kim wrote:
>>>>
>>>> Hi,
>>>>
>>>> On Wed, Oct 14, 2015 at 12:41:37PM +0000, Wang Nan wrote:
>>>>>
>>>>> 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:
>>>>>
>>>>>    # perf record -e mypmu=cycles ...
>>>>>
>>>>> To allow parser refer to existing event selecter, pass event list to
>>>>> 'struct parse_events_evlist'. perf_evlist__find_evsel_by_alias() is
>>>>> introduced to get evsel through its alias.
>>>>
>>>> What about using event name directly?  I guess the alias name is used
>>>> only to refer an event so it'd be better to use the event name.
>>>> Anyway we need alias as well when event has no name or name is complex.
>>>
>>>
>>> It is possible to trigger two perf events with same PMU but
>>> different config options:
>>>
>>>   # perf record -e cycles/period=9999/ -e cycles/period=99999/ -a sleep 1
>>>
>>> In this case the name of events are:
>>>
>>> cycles/period=9999/ `
>>> cycles/period=99999/
>>>
>>> Using it in perf cmdline is painful:
>>>
>>>   # perf record -e cycles/period=9999/ -e cycles/period=99999/ -e
>>> bpf_program.c/myevent=cycles/period=9999//...
>>
>> I understand the need of using aliases but I think it's more natural
>> to use event name for simple cases..
>
>
> I will consider this. However, if we allow using event name directly like
> this:
>
>  # perf record -e cycles -e test_pmu.c/myevent=cycles/ ...
>
> Then two '-e' seems redundant, right? Why not directly using:
>
>  # perf record -e test_pmu.c/myevent=cycles/ ...
>
> and make perf creates cycles event for test_pmu.c?
>
> We can make syntax like
>
>  # perf record -e test_pmu.c/myevent=cycles/ ...
>
> as a syntax sugar of
>
>  # perf record -e randomname=cycles -e test_pmu.c/myevent=randomname/ ...
>
> and don't need to find evsel through their names if alias not exist.
>
> So this is a new feature, and worth another patch.
>
> Thought?

Not sure it's worth.  It can confuse users IMHO.

Isn't it enough to give them in a single argument?

  # perf record -e cycles,test_pmu.c/myevent=cycles/

Thanks,
Namhyung
--
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/
Wang Nan Oct. 22, 2015, 7:59 a.m. UTC | #6
On 2015/10/22 15:53, Namhyung Kim wrote:
> On Thu, Oct 22, 2015 at 4:29 PM, Wangnan (F) <wangnan0@huawei.com> wrote:
>>

[SNIP]
>>> I understand the need of using aliases but I think it's more natural
>>> to use event name for simple cases..
>>
>> I will consider this. However, if we allow using event name directly like
>> this:
>>
>>   # perf record -e cycles -e test_pmu.c/myevent=cycles/ ...
>>
>> Then two '-e' seems redundant, right? Why not directly using:
>>
>>   # perf record -e test_pmu.c/myevent=cycles/ ...
>>
>> and make perf creates cycles event for test_pmu.c?
>>
>> We can make syntax like
>>
>>   # perf record -e test_pmu.c/myevent=cycles/ ...
>>
>> as a syntax sugar of
>>
>>   # perf record -e randomname=cycles -e test_pmu.c/myevent=randomname/ ...
>>
>> and don't need to find evsel through their names if alias not exist.
>>
>> So this is a new feature, and worth another patch.
>>
>> Thought?
> Not sure it's worth.  It can confuse users IMHO.
>
> Isn't it enough to give them in a single argument?
>
>    # perf record -e cycles,test_pmu.c/myevent=cycles/

OK. I have put it on my todo-list.

Thank you.
> Thanks,
> Namhyung


--
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/
diff mbox

Patch

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index d139219..8dd59aa 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1753,3 +1753,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.h b/tools/perf/util/evsel.h
index a60b5d5..9a95e73 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -87,6 +87,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 4849dbd..06ba5a6 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1020,6 +1020,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;
@@ -1373,9 +1397,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 8f17c83..b525353 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -96,6 +96,7 @@  struct parse_events_evlist {
 	int			   idx;
 	int			   nr_groups;
 	struct parse_events_error *error;
+	struct perf_evlist	  *evlist;
 };
 
 struct parse_events_terms {
@@ -168,4 +169,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 ad37996..90e382f 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -76,6 +76,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
@@ -192,13 +193,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 |