Description of "event parameters" from the documentation patch:
Event parameters are a basic way for partial events to be specified in
sysfs with per-event names given to the fields that need to be filled in
when using a particular event.
It is intended for supporting cases where the single 'cpu' parameter is
insufficient. For example, POWER 8 has events for physical
sockets/cores/cpus that are accessible from with virtual machines. To
keep using the single 'cpu' parameter we'd need to perform a mapping
between Linux's cpus and the physical machine's cpus (in this case
Linux is running under a hypervisor). This isn't possible because
bindings between our cpus and physical cpus may not be fixed, and we
probably won't have a "cpu" on each physical cpu.
Description of the sysfs contents when events are parameterized (copied from an
included patch):
Examples:
domain=0x1,offset=0x8,starting_index=$core
In the case of the last example, a value replacing "$core" would need
to be provided by the user selecting the particular event. This is
refered to as "event parameterization". All non-numerical values
indicate an event parameter.
Notes on how perf-list displays parameterized events
PARAMETERIZED EVENTS
--------------------
Some pmu events listed by 'perf list' will be displayed with '$xyz' in
them. For example:
hv_24x7/HPM_THREAD_NAP_CCYC__PHYS_CORE,starting_index=$core/
This means that when provided as an event, a value for $core must also
be supplied. For example:
perf stat -e \
'hv_24x7/HPM_THREAD_NAP_CCYC__PHYS_CORE,starting_index=2'
...
Changelog[v5]
- [Jiri Olsa, Peter Zijlstra] Use '$arg' notation rather than ?
to indicate event parameters.
- [Michael Ellerman] Separate the kernel and tool patches in the
patchset into different patchsets.
Changelog[v4]
- [Jiri Olsa] Rebase to perf/core tree (fix small merge conflict)
Changelog[v3]
- [Jiri Olsa] Changed the event parameters are specified. If
event file specifes 'param=val' make the usage 'param=123'
rather than 'val=123'. (patch 1,2/10)
- Shortened event names using "PHYS" and "VCPU" (patch 4/10)
- Print help message if invalid parameter is specified or required
parameter is missing.
- Moved 3 patches that are unrelated to parametrized events into
a separate patchset.
- Reordered patches so code changes come first.
Changelog[v2]
- [Joe Perches, David Laight] Use beNN_to_cpu() instead of guessing
the size from type.
- Use kmem_cache_free() to free page allocated with kmem_cache_alloc().
- Rebase to recent kernel
Cody P Schafer (4):
tools/perf: support parsing parameterized events
tools/perf: extend format_alias() to include event parameters
perf Documentation: add event parameters
tools/perf: Document parameterized and symbolic events
.../testing/sysfs-bus-event_source-devices-events | 6 ++
tools/perf/Documentation/perf-list.txt | 13 +++
tools/perf/Documentation/perf-record.txt | 12 +++
tools/perf/Documentation/perf-stat.txt | 20 ++++-
tools/perf/util/parse-events.h | 1 +
tools/perf/util/pmu.c | 92 +++++++++++++++++++---
6 files changed, 128 insertions(+), 16 deletions(-)
--
1.8.3.1
From: Cody P Schafer <[email protected]>
Changelog[v6]:
- [Sukadev Bhattiprolu]: Update documentation of perf-list and
perf-record; Added documentation for perf-stat.
CC: Haren Myneni <[email protected]>
CC: Cody P Schafer <[email protected]>
Signed-off-by: Cody P Schafer <[email protected]>
Signed-off-by: Sukadev Bhattiprolu <[email protected]>
---
tools/perf/Documentation/perf-list.txt | 13 +++++++++++++
tools/perf/Documentation/perf-record.txt | 12 ++++++++++++
tools/perf/Documentation/perf-stat.txt | 20 ++++++++++++++++----
3 files changed, 41 insertions(+), 4 deletions(-)
diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
index cbb4f74..8e9a1121 100644
--- a/tools/perf/Documentation/perf-list.txt
+++ b/tools/perf/Documentation/perf-list.txt
@@ -89,6 +89,19 @@ raw encoding of 0x1A8 can be used:
You should refer to the processor specific documentation for getting these
details. Some of them are referenced in the SEE ALSO section below.
+PARAMETERIZED EVENTS
+--------------------
+
+Some pmu events listed by 'perf-list' will be displayed with '$x' in them. For
+example:
+
+ hv_gpci/dtbp_ptitc,starting_index=$core/
+
+This means that when provided as an event, a value for '$core' must
+also be supplied. For example:
+
+ perf stat -C 0 -e 'hv_gpci/dtbp_ptitc,starting_index=0x2/' ...
+
OPTIONS
-------
diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index af9a54e..acdcf3b 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -33,6 +33,18 @@ OPTIONS
- a raw PMU event (eventsel+umask) in the form of rNNN where NNN is a
hexadecimal event descriptor.
+ - a symbolically formed PMU event like 'pmu/param1=0x3,param2/' where
+ 'param1', 'param2', etc are defined as formats for the PMU in
+ /sys/bus/event_sources/devices/<pmu>/format/*.
+
+ - a symbolically formed event like 'pmu/config=M,config1=N,config3=K/'
+
+ where M, N, K are numbers (in decimal, hex, octal format). Acceptable
+ values for each of 'config', 'config1' and 'config2' are defined by
+ corresponding entries in /sys/bus/event_sources/devices/<pmu>/format/*
+ param1 and param2 are defined as formats for the PMU in:
+ /sys/bus/event_sources/devices/<pmu>/format/*
+
- a hardware breakpoint event in the form of '\mem:addr[:access]'
where addr is the address in memory you want to break in.
Access is the memory access type (read, write, execute) it can
diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index 29ee857..04e150d 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -25,10 +25,22 @@ OPTIONS
-e::
--event=::
- Select the PMU event. Selection can be a symbolic event name
- (use 'perf list' to list all events) or a raw PMU
- event (eventsel+umask) in the form of rNNN where NNN is a
- hexadecimal event descriptor.
+ Select the PMU event. Selection can be:
+
+ - a symbolic event name (use 'perf list' to list all events)
+
+ - a raw PMU event (eventsel+umask) in the form of rNNN where NNN is a
+ hexadecimal event descriptor.
+
+ - a symbolically formed event like 'pmu/param1=0x3,param2/' where
+ param1 and param2 are defined as formats for the PMU in
+ /sys/bus/event_sources/devices/<pmu>/format/*
+
+ - a symbolically formed event like 'pmu/config=M,config1=N,config2=K/'
+ where M, N, K are numbers (in decimal, hex, octal format).
+ Acceptable values for each of 'config', 'config1' and 'config2'
+ parameters are defined by corresponding entries in
+ /sys/bus/event_sources/devices/<pmu>/format/*
-i::
--no-inherit::
--
1.8.3.1
From: Cody P Schafer <[email protected]>
Event parameters are a basic way for partial events to be specified in
sysfs with per-event names given to the fields that need to be filled in
when using a particular event.
It is intended for supporting cases where the single 'cpu' parameter is
insufficient. For example, POWER 8 has events for physical
sockets/cores/cpus that are accessible from with virtual machines. To
keep using the single 'cpu' parameter we'd need to perform a mapping
between Linux's cpus and the physical machine's cpus (in this case
Linux is running under a hypervisor). This isn't possible because
bindings between our cpus and physical cpus may not be fixed, and we
probably won't have a "cpu" on each physical cpu.
CC: Sukadev Bhattiprolu <[email protected]>
CC: Haren Myneni <[email protected]>
CC: Cody P Schafer <[email protected]>
Signed-off-by: Cody P Schafer <[email protected]>
---
Documentation/ABI/testing/sysfs-bus-event_source-devices-events | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-event_source-devices-events b/Documentation/ABI/testing/sysfs-bus-event_source-devices-events
index 20979f8..f584b16 100644
--- a/Documentation/ABI/testing/sysfs-bus-event_source-devices-events
+++ b/Documentation/ABI/testing/sysfs-bus-event_source-devices-events
@@ -52,12 +52,18 @@ Description: Per-pmu performance monitoring events specific to the running syste
event=0x2abc
event=0x423,inv,cmask=0x3
domain=0x1,offset=0x8,starting_index=0xffff
+ domain=0x1,offset=0x8,starting_index=$phys_cpu
Each of the assignments indicates a value to be assigned to a
particular set of bits (as defined by the format file
corresponding to the <term>) in the perf_event structure passed
to the perf_open syscall.
+ In the case of the last example, a value replacing "$phys_cpu"
+ would need to be provided by the user selecting the particular
+ event. This is referred to as "event parameterization". All
+ non-numerical values indicate an event parameter.
+
What: /sys/bus/event_source/devices/<pmu>/events/<event>.unit
Date: 2014/02/24
Contact: Linux kernel mailing list <[email protected]>
--
1.8.3.1
From: Cody P Schafer <[email protected]>
Enable event specification like:
pmu/event_name,param1=0x1,param2=0x4/
Assuming that
/sys/bus/event_source/devices/pmu/events/event_name
Contains something like
param2=$foo,bar=1,param1=$baz
Changelog[v4]:
[Jiri Olsa] Merge to recent perf-core and fix a small conflict.
Changelog[v3]:
[Jiri Olsa] If the sysfs event file specifies 'param=val', make the
usage 'hv_24x7/event,param=123/' rather than 'hv_24x7/event,val=123/'.
CC: Haren Myneni <[email protected]>
CC: Cody P Schafer <[email protected]>
Signed-off-by: Sukadev Bhattiprolu <[email protected]>
Signed-off-by: Cody P Schafer <[email protected]>
Conflicts:
tools/perf/util/pmu.c
---
tools/perf/util/parse-events.h | 1 +
tools/perf/util/pmu.c | 65 +++++++++++++++++++++++++++++++++++-------
2 files changed, 55 insertions(+), 11 deletions(-)
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index db2cf78..ca226ce 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -71,6 +71,7 @@ struct parse_events_term {
int type_val;
int type_term;
struct list_head list;
+ bool used;
};
struct parse_events_evlist {
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 5c9c494..cb516dd 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -551,31 +551,68 @@ static void pmu_format_value(unsigned long *format, __u64 value, __u64 *v,
}
/*
+ * Term is a string term, and might be a param-term. Try to look up it's value
+ * in the remaining terms.
+ * - We have a term like "base-or-format-term=param-term",
+ * - We need to find the value supplied for "param-term" (with param-term named
+ * in a config string) later on in the term list.
+ */
+static int pmu_resolve_param_term(struct parse_events_term *term,
+ struct list_head *head_terms,
+ __u64 *value)
+{
+ struct parse_events_term *t;
+
+ list_for_each_entry(t, head_terms, list) {
+ if (t->type_val == PARSE_EVENTS__TERM_TYPE_NUM) {
+ if (!strcmp(t->config, term->config)) {
+ t->used = true;
+ *value = t->val.num;
+ return 0;
+ }
+ }
+ }
+
+ if (verbose)
+ printf("Required parameter '%s' not specified\n", term->config);
+
+ return -1;
+}
+
+/*
* Setup one of config[12] attr members based on the
* user input data - term parameter.
*/
static int pmu_config_term(struct list_head *formats,
struct perf_event_attr *attr,
struct parse_events_term *term,
+ struct list_head *head_terms,
bool zero)
{
struct perf_pmu_format *format;
__u64 *vp;
+ __u64 val;
+
+ /*
+ * If this is a parameter we've already used for parameterized-eval,
+ * skip it in normal eval.
+ */
+ if (term->used)
+ return 0;
/*
- * Support only for hardcoded and numnerial terms.
* Hardcoded terms should be already in, so nothing
* to be done for them.
*/
if (parse_events__is_hardcoded_term(term))
return 0;
- if (term->type_val != PARSE_EVENTS__TERM_TYPE_NUM)
- return -EINVAL;
-
format = pmu_find_format(formats, term->config);
- if (!format)
+ if (!format) {
+ if (verbose)
+ printf("Invalid event/parameter '%s'\n", term->config);
return -EINVAL;
+ }
switch (format->value) {
case PERF_PMU_FORMAT_VALUE_CONFIG:
@@ -592,11 +629,16 @@ static int pmu_config_term(struct list_head *formats,
}
/*
- * XXX If we ever decide to go with string values for
- * non-hardcoded terms, here's the place to translate
- * them into value.
+ * Either directly use a numeric term, or try to translate string terms
+ * using event parameters.
*/
- pmu_format_value(format->bits, term->val.num, vp, zero);
+ if (term->type_val == PARSE_EVENTS__TERM_TYPE_NUM)
+ val = term->val.num;
+ else
+ if (pmu_resolve_param_term(term, head_terms, &val))
+ return -EINVAL;
+
+ pmu_format_value(format->bits, val, vp, zero);
return 0;
}
@@ -607,9 +649,10 @@ int perf_pmu__config_terms(struct list_head *formats,
{
struct parse_events_term *term;
- list_for_each_entry(term, head_terms, list)
- if (pmu_config_term(formats, attr, term, zero))
+ list_for_each_entry(term, head_terms, list) {
+ if (pmu_config_term(formats, attr, term, head_terms, zero))
return -EINVAL;
+ }
return 0;
}
--
1.8.3.1
From: Cody P Schafer <[email protected]>
This causes `perf list pmu` to show parameters for parameterized events
like:
pmu/event_name,param1=$param1,param2=$param2/ [Kernel PMU event]
Example:
hv_24x7/HPM_TLBIE__PHYS_CORE,starting_index=$core/ [Kernel PMU event]
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 <[email protected]>
CC: Cody P Schafer <[email protected]>
Signed-off-by: Cody P Schafer <[email protected]>
Signed-off-by: Sukadev Bhattiprolu <[email protected]>
---
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..f8674c1 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;
}
--
1.8.3.1
On Tue, Dec 02, 2014 at 06:09:35PM -0800, Sukadev Bhattiprolu wrote:
> From: Cody P Schafer <[email protected]>
>
> Enable event specification like:
>
> pmu/event_name,param1=0x1,param2=0x4/
>
> Assuming that
>
> /sys/bus/event_source/devices/pmu/events/event_name
>
> Contains something like
>
> param2=$foo,bar=1,param1=$baz
oops.. sorry to be PITA on this one.. I might have missed something
in the previous discussion but I guess I might have finally some
opinion on this ;-)
here's how I think your patchset works:
in /sys/bus/event_source/devices/pmu/events/event_name you can actually have:
param2=foo,bar=1,param1=baz
notice no '$', thats what you add later in 'perf list' output, right?
Moreover it actually does not matter whats in value 'param2=HERE',
because it's not used in the config code at all apart from the
'perf list' display processing.
So when we discussed the '$' name way, I thought it'd be like:
in /sys/bus/event_source/devices/pmu/events/event_name you have:
param2=$foo,bar=1,param1=$baz
and on command line you'd use:
pmu/event_name,foo=0x1,bar=0x4/
to assign directly to the $var, which would justify the $var
syntax I think..
anyway we could assign directly to the param term name as you do,
but I think we just need to mark the term as parametrized, like:
in /sys/bus/event_source/devices/pmu/events/event_name you have:
param2=?,bar=1,param1=?
and on command line you'd use:
pmu/event_name,param2=0x1,param1=0x4/
while the config code would check that the param substitution is
done only for terms with '?' in value, like 'param2=?' and not
for all PARSE_EVENTS__TERM_TYPE_STR type terms (as of now)
thanks,
jirka
On Thu, Dec 4, 2014 at 7:44 AM, Jiri Olsa <[email protected]> wrote:
> On Tue, Dec 02, 2014 at 06:09:35PM -0800, Sukadev Bhattiprolu wrote:
>> From: Cody P Schafer <[email protected]>
>>
>> Enable event specification like:
>>
>> pmu/event_name,param1=0x1,param2=0x4/
>>
>> Assuming that
>>
>> /sys/bus/event_source/devices/pmu/events/event_name
>>
>> Contains something like
>>
>> param2=$foo,bar=1,param1=$baz
>
> oops.. sorry to be PITA on this one.. I might have missed something
> in the previous discussion but I guess I might have finally some
> opinion on this ;-)
>
> here's how I think your patchset works:
>
> in /sys/bus/event_source/devices/pmu/events/event_name you can actually have:
>
> param2=foo,bar=1,param1=baz
>
> notice no '$', thats what you add later in 'perf list' output, right?
>
> Moreover it actually does not matter whats in value 'param2=HERE',
> because it's not used in the config code at all apart from the
> 'perf list' display processing.
>
> So when we discussed the '$' name way, I thought it'd be like:
>
> in /sys/bus/event_source/devices/pmu/events/event_name you have:
> param2=$foo,bar=1,param1=$baz
>
> and on command line you'd use:
> pmu/event_name,foo=0x1,bar=0x4/
>
> to assign directly to the $var, which would justify the $var
> syntax I think..
>
Agreed, what you've described above sounds like a good idea.
Compared to monopolizing all strings (which is what I did when
initialy writing this), using a '$' prefix would allow less pain when
some events suddenly need non-integer parameters.
> anyway we could assign directly to the param term name as you do,
> but I think we just need to mark the term as parametrized, like:
>
> in /sys/bus/event_source/devices/pmu/events/event_name you have:
> param2=?,bar=1,param1=?
>
> and on command line you'd use:
> pmu/event_name,param2=0x1,param1=0x4/
>
> while the config code would check that the param substitution is
> done only for terms with '?' in value, like 'param2=?' and not
> for all PARSE_EVENTS__TERM_TYPE_STR type terms (as of now)
I prefer the `foo=0x1` as mentioned previously: it makes the user
interface much less painful as we can have event-specific names for
register/hcall fields.
I'm pretty sure the code used to do this, not sure when it was removed
(haven't been following this patchset closely).
That said: I haven't fiddled with this code in a while (it's Suka's at
this point), and there might be arguments the other way on both of
those.
On Fri, Dec 05, 2014 at 06:05:26PM -0500, Cody P Schafer wrote:
> On Thu, Dec 4, 2014 at 7:44 AM, Jiri Olsa <[email protected]> wrote:
> > On Tue, Dec 02, 2014 at 06:09:35PM -0800, Sukadev Bhattiprolu wrote:
> >> From: Cody P Schafer <[email protected]>
> >>
> >> Enable event specification like:
> >>
> >> pmu/event_name,param1=0x1,param2=0x4/
> >>
> >> Assuming that
> >>
> >> /sys/bus/event_source/devices/pmu/events/event_name
> >>
> >> Contains something like
> >>
> >> param2=$foo,bar=1,param1=$baz
> >
> > oops.. sorry to be PITA on this one.. I might have missed something
> > in the previous discussion but I guess I might have finally some
> > opinion on this ;-)
> >
> > here's how I think your patchset works:
> >
> > in /sys/bus/event_source/devices/pmu/events/event_name you can actually have:
> >
> > param2=foo,bar=1,param1=baz
> >
> > notice no '$', thats what you add later in 'perf list' output, right?
> >
> > Moreover it actually does not matter whats in value 'param2=HERE',
> > because it's not used in the config code at all apart from the
> > 'perf list' display processing.
> >
> > So when we discussed the '$' name way, I thought it'd be like:
> >
> > in /sys/bus/event_source/devices/pmu/events/event_name you have:
> > param2=$foo,bar=1,param1=$baz
> >
> > and on command line you'd use:
> > pmu/event_name,foo=0x1,bar=0x4/
> >
> > to assign directly to the $var, which would justify the $var
> > syntax I think..
> >
>
> Agreed, what you've described above sounds like a good idea.
>
> Compared to monopolizing all strings (which is what I did when
> initialy writing this), using a '$' prefix would allow less pain when
> some events suddenly need non-integer parameters.
>
> > anyway we could assign directly to the param term name as you do,
> > but I think we just need to mark the term as parametrized, like:
> >
> > in /sys/bus/event_source/devices/pmu/events/event_name you have:
> > param2=?,bar=1,param1=?
> >
> > and on command line you'd use:
> > pmu/event_name,param2=0x1,param1=0x4/
> >
> > while the config code would check that the param substitution is
> > done only for terms with '?' in value, like 'param2=?' and not
> > for all PARSE_EVENTS__TERM_TYPE_STR type terms (as of now)
>
> I prefer the `foo=0x1` as mentioned previously: it makes the user
> interface much less painful as we can have event-specific names for
> register/hcall fields.
>
> I'm pretty sure the code used to do this, not sure when it was removed
> (haven't been following this patchset closely).
right, I recall seeing the 2 indirect assignments earlier,
but it was without the '$' marks
>
> That said: I haven't fiddled with this code in a while (it's Suka's at
> this point), and there might be arguments the other way on both of
> those.
I guess I'm ok with both ways, maybe slightly inclined to
the '$' variable style one ;-)
jirka
Jiri Olsa [[email protected]] wrote:
| anyway we could assign directly to the param term name as you do,
| but I think we just need to mark the term as parametrized, like:
|
| in /sys/bus/event_source/devices/pmu/events/event_name you have:
| param2=?,bar=1,param1=?
I like the idea of just using a single ? for required parameters, but
the problem I had with this approach can be seen with these two sysfs
entries:
$ cat HPM_0THRD_NON_IDLE_CCYC__PHYS_CORE
domain=0x2,offset=0xe0,starting_index=core,lpar=0x0
$ cat HPM_0THRD_NON_IDLE_CCYC__VCPU_HOME_CORE
domain=0x3,offset=0xe0,starting_index=vcpu,lpar=sibling_guest_id
The parameter 'starting_index' refers to a core in one event and vcpu in
another event. We were trying to give a hint as to what it refers to.
Given that, 'starting_index' is not very intuitive, how about discarding
starting_index and replacing with what it really means for the event and,
use a simple '?' to indicate required parameter).
$ cat HPM_0THRD_NON_IDLE_CCYC__PHYS_CORE
domain=0x2,offset=0xe0,core=?,lpar=0x0
$ cat HPM_0THRD_NON_IDLE_CCYC__VCPU_HOME_CORE
domain=0x3,offset=0xe0,vcpu=?,lpar=?
perf list shows these as:
hv_24x7/HPM_0THRD_NON_IDLE_CCYC__PHYS_CORE,core=?/
hv_24x7/HPM_0THRD_NON_IDLE_CCYC__VCPU_HOME_CHIP,vcpu=?,lpar=?/
command line would be
-e hv_24x7/HPM_0THRD_NON_IDLE_CCYC__PHYS_CORE,core=2/
or
-e hv_24x7/HPM_0THRD_NON_IDLE_CCYC__VCPU_HOME_CHIP,vcpu=2,lpar=7/
and would fail if a required parameter is missing.
This would eliminate the need for new strings like 'sibling_guest_id' (or
as Cody calls it monopolizing strings...)
Following quick patch on top of the patchset shows the changes:
diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 73d5bfc..a82bc64 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -27,6 +27,8 @@
#include "hv-24x7-catalog.h"
#include "hv-common.h"
+
+#if 0
static const char *domain_to_index_string(unsigned domain)
{
switch (domain) {
@@ -40,6 +42,7 @@ static const char *domain_to_index_string(unsigned domain)
return "UNKNOWN_DOMAIN_INDEX_STRING";
}
}
+#endif
static const char *event_domain_suffix(unsigned domain)
{
@@ -114,7 +117,8 @@ static bool catalog_entry_domain_is_valid(unsigned domain)
/* u3 0-6, one of HV_24X7_PERF_DOMAIN */
EVENT_DEFINE_RANGE_FORMAT(domain, config, 0, 3);
/* u16 */
-EVENT_DEFINE_RANGE_FORMAT(starting_index, config, 16, 31);
+EVENT_DEFINE_RANGE_FORMAT(core, config, 16, 31);
+EVENT_DEFINE_RANGE_FORMAT(vcpu, config, 16, 31);
/* u32, see "data_offset" */
EVENT_DEFINE_RANGE_FORMAT(offset, config, 32, 63);
/* u16 */
@@ -127,7 +131,8 @@ EVENT_DEFINE_RANGE(reserved3, config2, 0, 63);
static struct attribute *format_attrs[] = {
&format_attr_domain.attr,
&format_attr_offset.attr,
- &format_attr_starting_index.attr,
+ &format_attr_core.attr,
+ &format_attr_vcpu.attr,
&format_attr_lpar.attr,
NULL,
};
@@ -280,19 +285,23 @@ static unsigned core_domains[] = {
static char *event_fmt(struct hv_24x7_event_data *event, unsigned domain)
{
+ const char *sindex;
const char *lpar;
- if (is_physical_domain(domain))
+ if (is_physical_domain(domain)) {
lpar = "0x0";
- else
- lpar = "$sibling_guest_id";
+ sindex = "core";
+ } else {
+ lpar = "?";
+ sindex = "vcpu";
+ }
return kasprintf(GFP_KERNEL,
- "domain=0x%x,offset=0x%x,starting_index=%s,lpar=%s",
+ "domain=0x%x,offset=0x%x,%s=?,lpar=%s",
domain,
be16_to_cpu(event->event_counter_offs) +
be16_to_cpu(event->event_group_record_offs),
- domain_to_index_string(domain),
+ sindex,
lpar);
}
@@ -1061,9 +1070,17 @@ out:
static unsigned long event_24x7_request(struct perf_event *event, u64 *res,
bool success_expected)
{
+ u16 idx;
+ unsigned domain = event_get_domain(event);
+
+ if (is_physical_domain(domain))
+ idx = event_get_core(event);
+ else
+ idx = event_get_vcpu(event);
+
return single_24x7_request(event_get_domain(event),
event_get_offset(event),
- event_get_starting_index(event),
+ idx,
event_get_lpar(event),
res,
success_expected);
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index f8674c1..d208fef 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -826,7 +826,7 @@ static char *format_alias(char *buf, int len, struct perf_pmu *pmu,
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,
+ ",%s=%s", term->config,
term->val.str);
if (sub_non_neg(len, used) > 0) {
On Sat, Dec 06, 2014 at 11:37:24PM -0800, Sukadev Bhattiprolu wrote:
> Jiri Olsa [[email protected]] wrote:
>
> | anyway we could assign directly to the param term name as you do,
> | but I think we just need to mark the term as parametrized, like:
> |
> | in /sys/bus/event_source/devices/pmu/events/event_name you have:
> | param2=?,bar=1,param1=?
>
> I like the idea of just using a single ? for required parameters, but
> the problem I had with this approach can be seen with these two sysfs
> entries:
>
> $ cat HPM_0THRD_NON_IDLE_CCYC__PHYS_CORE
> domain=0x2,offset=0xe0,starting_index=core,lpar=0x0
>
> $ cat HPM_0THRD_NON_IDLE_CCYC__VCPU_HOME_CORE
> domain=0x3,offset=0xe0,starting_index=vcpu,lpar=sibling_guest_id
>
> The parameter 'starting_index' refers to a core in one event and vcpu in
> another event. We were trying to give a hint as to what it refers to.
>
> Given that, 'starting_index' is not very intuitive, how about discarding
> starting_index and replacing with what it really means for the event and,
> use a simple '?' to indicate required parameter).
>
> $ cat HPM_0THRD_NON_IDLE_CCYC__PHYS_CORE
> domain=0x2,offset=0xe0,core=?,lpar=0x0
>
> $ cat HPM_0THRD_NON_IDLE_CCYC__VCPU_HOME_CORE
> domain=0x3,offset=0xe0,vcpu=?,lpar=?
>
> perf list shows these as:
>
> hv_24x7/HPM_0THRD_NON_IDLE_CCYC__PHYS_CORE,core=?/
> hv_24x7/HPM_0THRD_NON_IDLE_CCYC__VCPU_HOME_CHIP,vcpu=?,lpar=?/
>
> command line would be
>
> -e hv_24x7/HPM_0THRD_NON_IDLE_CCYC__PHYS_CORE,core=2/
>
> or
>
> -e hv_24x7/HPM_0THRD_NON_IDLE_CCYC__VCPU_HOME_CHIP,vcpu=2,lpar=7/
>
> and would fail if a required parameter is missing.
that sounds good to me
jirka