diff mbox

[v6,2/4] tools/perf: extend format_alias() to include event parameters

Message ID 1419234567-22784-3-git-send-email-sukadev@linux.vnet.ibm.com (mailing list archive)
State Superseded
Delegated to: Michael Ellerman
Headers show

Commit Message

Sukadev Bhattiprolu Dec. 22, 2014, 7:49 a.m. UTC 
From: Cody P Schafer <cody@linux.vnet.ibm.com>

This causes `perf list pmu` to show parameters for parameterized events
like:

  pmu/event_name,param1=?,param2=?/ [Kernel PMU event]

An example:

  hv_24x7/HPM_TLBIE__PHYS_CORE,core=?/ [Kernel PMU event]

Changelog[v6]
	[Jir Olsa, Sukadev Bhattiprolu] Drop the '$' sign and go back to
	just printing whatevever sysfs provides (which is '=?') to identify
	required parameters. sysfs also now uses parameters like 'core'
	and 'vcpu' rather than 'starting_index'.

Changelog[v5]
	[Jiri Olsa, Peter Zijlstra] Use '$' to prefix parameterized events.

Changelog[v4]
	[Jiri Olsa] If the parameter for an event in sysfs is 'param=val',
	have perf-list show the event as 'param=?' rather than 'val=?'.

CC: Haren Myneni <hbabu@us.ibm.com>
CC: Cody P Schafer <dev@codyps.com>
Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 tools/perf/util/pmu.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

Comments

Jiri Olsa Jan. 6, 2015, 9:39 a.m. UTC | #1 
On Sun, Dec 21, 2014 at 11:49:25PM -0800, Sukadev Bhattiprolu wrote:
> From: Cody P Schafer <cody@linux.vnet.ibm.com>
> 
> This causes `perf list pmu` to show parameters for parameterized events
> like:
> 
>   pmu/event_name,param1=?,param2=?/ [Kernel PMU event]
> 
> An example:
> 
>   hv_24x7/HPM_TLBIE__PHYS_CORE,core=?/ [Kernel PMU event]
> 
> Changelog[v6]
> 	[Jir Olsa, Sukadev Bhattiprolu] Drop the '$' sign and go back to
> 	just printing whatevever sysfs provides (which is '=?') to identify
> 	required parameters. sysfs also now uses parameters like 'core'
> 	and 'vcpu' rather than 'starting_index'.
> 
> Changelog[v5]
> 	[Jiri Olsa, Peter Zijlstra] Use '$' to prefix parameterized events.
> 
> Changelog[v4]
> 	[Jiri Olsa] If the parameter for an event in sysfs is 'param=val',
> 	have perf-list show the event as 'param=?' rather than 'val=?'.
> 
> CC: Haren Myneni <hbabu@us.ibm.com>
> CC: Cody P Schafer <dev@codyps.com>
> Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> ---
>  tools/perf/util/pmu.c | 27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index cb516dd..d208fef 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -810,10 +810,35 @@ void perf_pmu__set_format(unsigned long *bits, long from, long to)
>  		set_bit(b, bits);
>  }
>  
> +static int sub_non_neg(int a, int b)
> +{
> +	if (b > a)
> +		return 0;
> +	return a - b;
> +}
> +
>  static char *format_alias(char *buf, int len, struct perf_pmu *pmu,
>  			  struct perf_pmu_alias *alias)
>  {
> -	snprintf(buf, len, "%s/%s/", pmu->name, alias->name);
> +	struct parse_events_term *term;
> +	int used = snprintf(buf, len, "%s/%s", pmu->name, alias->name);
> +
> +	list_for_each_entry(term, &alias->terms, list)
> +		if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR)
> +			used += snprintf(buf + used, sub_non_neg(len, used),
> +					",%s=%s", term->config,
> +					term->val.str);

why not display PARSE_EVENTS__TERM_TYPE_NUM as well?

jirka
Jiri Olsa Jan. 6, 2015, 9:40 a.m. UTC | #2 
On Sun, Dec 21, 2014 at 11:49:25PM -0800, Sukadev Bhattiprolu wrote:
> From: Cody P Schafer <cody@linux.vnet.ibm.com>
> 
> This causes `perf list pmu` to show parameters for parameterized events
> like:
> 
>   pmu/event_name,param1=?,param2=?/ [Kernel PMU event]
> 
> An example:
> 
>   hv_24x7/HPM_TLBIE__PHYS_CORE,core=?/ [Kernel PMU event]
> 
> Changelog[v6]
> 	[Jir Olsa, Sukadev Bhattiprolu] Drop the '$' sign and go back to
> 	just printing whatevever sysfs provides (which is '=?') to identify
> 	required parameters. sysfs also now uses parameters like 'core'
> 	and 'vcpu' rather than 'starting_index'.
> 
> Changelog[v5]
> 	[Jiri Olsa, Peter Zijlstra] Use '$' to prefix parameterized events.
> 
> Changelog[v4]
> 	[Jiri Olsa] If the parameter for an event in sysfs is 'param=val',
> 	have perf-list show the event as 'param=?' rather than 'val=?'.
> 
> CC: Haren Myneni <hbabu@us.ibm.com>
> CC: Cody P Schafer <dev@codyps.com>
> Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> ---
>  tools/perf/util/pmu.c | 27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index cb516dd..d208fef 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -810,10 +810,35 @@ void perf_pmu__set_format(unsigned long *bits, long from, long to)
>  		set_bit(b, bits);
>  }
>  
> +static int sub_non_neg(int a, int b)
> +{
> +	if (b > a)
> +		return 0;
> +	return a - b;
> +}
> +
>  static char *format_alias(char *buf, int len, struct perf_pmu *pmu,
>  			  struct perf_pmu_alias *alias)
>  {
> -	snprintf(buf, len, "%s/%s/", pmu->name, alias->name);
> +	struct parse_events_term *term;
> +	int used = snprintf(buf, len, "%s/%s", pmu->name, alias->name);
> +
> +	list_for_each_entry(term, &alias->terms, list)
> +		if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR)
> +			used += snprintf(buf + used, sub_non_neg(len, used),
> +					",%s=%s", term->config,
> +					term->val.str);

also please enclose above loop and if in {}

jirka
Sukadev Bhattiprolu Jan. 7, 2015, 11:41 p.m. UTC | #3 
Jiri Olsa [jolsa@redhat.com] wrote:
| On Sun, Dec 21, 2014 at 11:49:25PM -0800, Sukadev Bhattiprolu wrote:
| > From: Cody P Schafer <cody@linux.vnet.ibm.com>
| > 
| > This causes `perf list pmu` to show parameters for parameterized events
| > like:
| > 
| >   pmu/event_name,param1=?,param2=?/ [Kernel PMU event]
| > 
| > An example:
| > 
| >   hv_24x7/HPM_TLBIE__PHYS_CORE,core=?/ [Kernel PMU event]
| > 
| > Changelog[v6]
| > 	[Jir Olsa, Sukadev Bhattiprolu] Drop the '$' sign and go back to
| > 	just printing whatevever sysfs provides (which is '=?') to identify
| > 	required parameters. sysfs also now uses parameters like 'core'
| > 	and 'vcpu' rather than 'starting_index'.
| > 
| > Changelog[v5]
| > 	[Jiri Olsa, Peter Zijlstra] Use '$' to prefix parameterized events.
| > 
| > Changelog[v4]
| > 	[Jiri Olsa] If the parameter for an event in sysfs is 'param=val',
| > 	have perf-list show the event as 'param=?' rather than 'val=?'.
| > 
| > CC: Haren Myneni <hbabu@us.ibm.com>
| > CC: Cody P Schafer <dev@codyps.com>
| > Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
| > Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
| > ---
| >  tools/perf/util/pmu.c | 27 ++++++++++++++++++++++++++-
| >  1 file changed, 26 insertions(+), 1 deletion(-)
| > 
| > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
| > index cb516dd..d208fef 100644
| > --- a/tools/perf/util/pmu.c
| > +++ b/tools/perf/util/pmu.c
| > @@ -810,10 +810,35 @@ void perf_pmu__set_format(unsigned long *bits, long from, long to)
| >  		set_bit(b, bits);
| >  }
| >  
| > +static int sub_non_neg(int a, int b)
| > +{
| > +	if (b > a)
| > +		return 0;
| > +	return a - b;
| > +}
| > +
| >  static char *format_alias(char *buf, int len, struct perf_pmu *pmu,
| >  			  struct perf_pmu_alias *alias)
| >  {
| > -	snprintf(buf, len, "%s/%s/", pmu->name, alias->name);
| > +	struct parse_events_term *term;
| > +	int used = snprintf(buf, len, "%s/%s", pmu->name, alias->name);
| > +
| > +	list_for_each_entry(term, &alias->terms, list)
| > +		if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR)
| > +			used += snprintf(buf + used, sub_non_neg(len, used),
| > +					",%s=%s", term->config,
| > +					term->val.str);
| 
| why not display PARSE_EVENTS__TERM_TYPE_NUM as well?

Well, we are only trying to list the attributes that user needs to specify:

	hv_24x7/HPM_0THRD_NON_IDLE_CCYC__PHYS_CORE,core=?/

We could print the PARSE_EVENTS__TERM_TYPE_NUM terms as well,

	hv_24x7/HPM_0THRD_NON_IDLE_CCYC__PHYS_CORE,domain=2,offset=e0,core=?,lpar=0/

but it would be needlessly verbose. The attributes with numeric values
like domain and offset, have fixed values for the event and user doesn't
need to know. If they do really need to know, they could just look up in
sysfs ?

Sukadev
diff mbox

Patch

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index cb516dd..d208fef 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -810,10 +810,35 @@  void perf_pmu__set_format(unsigned long *bits, long from, long to)
 		set_bit(b, bits);
 }
 
+static int sub_non_neg(int a, int b)
+{
+	if (b > a)
+		return 0;
+	return a - b;
+}
+
 static char *format_alias(char *buf, int len, struct perf_pmu *pmu,
 			  struct perf_pmu_alias *alias)
 {
-	snprintf(buf, len, "%s/%s/", pmu->name, alias->name);
+	struct parse_events_term *term;
+	int used = snprintf(buf, len, "%s/%s", pmu->name, alias->name);
+
+	list_for_each_entry(term, &alias->terms, list)
+		if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR)
+			used += snprintf(buf + used, sub_non_neg(len, used),
+					",%s=%s", term->config,
+					term->val.str);
+
+	if (sub_non_neg(len, used) > 0) {
+		buf[used] = '/';
+		used++;
+	}
+	if (sub_non_neg(len, used) > 0) {
+		buf[used] = '\0';
+		used++;
+	} else
+		buf[len - 1] = '\0';
+
 	return buf;
 }