LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] perf list: Add s390 support for detailed/verbose pmu event description
@ 2018-04-09 11:00 Thomas Richter
2018-04-09 11:37 ` Mark Rutland
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Richter @ 2018-04-09 11:00 UTC (permalink / raw)
To: linux-kernel, linux-perf-users, acme
Cc: brueckner, schwidefsky, heiko.carstens, Thomas Richter
From: Thomas Richter <tmricht@linux.vnet.ibm.com>
Perf list with flags -d and -v print a description (-d) or
a very verbose explanation (-v) of CPU specific counter events.
These descriptions are provided with the json files in
directory pmu-events/arch/s390/*.json.
Display of these descriptions on s390 requires the
corresponding json files.
On s390 this does not work because function is_pmu_core()
does not detect the s390 directory name where the
CPU specific events are listed. On x86 it is
/sys/bus/event_source/devices/cpu
whereas on s390 it is
/sys/bus/event_source/devices/cpum_cf
/sys/bus/event_source/devices/cpum_sf
Fix this by adding s390 directory name testing to
function is_pmu_core(). This is the same approach as taken for
arm platform.
Output before:
[root@s35lp76 perf]# ./perf list -d pmu
List of pre-defined events (to be used in -e):
cpum_cf/AES_BLOCKED_CYCLES/ [Kernel PMU event]
cpum_cf/AES_BLOCKED_FUNCTIONS/ [Kernel PMU event]
cpum_cf/AES_CYCLES/ [Kernel PMU event]
cpum_cf/AES_FUNCTIONS/ [Kernel PMU event]
....
cpum_cf/TX_NC_TEND/ [Kernel PMU event]
cpum_cf/VX_BCD_EXECUTION_SLOTS/ [Kernel PMU event]
cpum_sf/SF_CYCLES_BASIC/ [Kernel PMU event]
Output after:
[root@s35lp76 perf]# ./perf list -d pmu
List of pre-defined events (to be used in -e):
cpum_cf/AES_BLOCKED_CYCLES/ [Kernel PMU event]
cpum_cf/AES_BLOCKED_FUNCTIONS/ [Kernel PMU event]
cpum_cf/AES_CYCLES/ [Kernel PMU event]
cpum_cf/AES_FUNCTIONS/ [Kernel PMU event]
....
cpum_cf/TX_NC_TEND/ [Kernel PMU event]
cpum_cf/VX_BCD_EXECUTION_SLOTS/ [Kernel PMU event]
cpum_sf/SF_CYCLES_BASIC/ [Kernel PMU event]
3906:
bcd_dfp_execution_slots
[BCD DFP Execution Slots]
decimal_instructions
[Decimal Instructions]
dtlb2_gpage_writes
[DTLB2 GPAGE Writes]
dtlb2_hpage_writes
[DTLB2 HPAGE Writes]
dtlb2_misses
[DTLB2 Misses]
dtlb2_writes
[DTLB2 Writes]
itlb2_misses
[ITLB2 Misses]
itlb2_writes
[ITLB2 Writes]
l1c_tlb2_misses
[L1C TLB2 Misses]
.....
cfvn 3:
cpu_cycles
[CPU Cycles]
instructions
[Instructions]
l1d_dir_writes
[L1D Directory Writes]
l1d_penalty_cycles
[L1D Penalty Cycles]
l1i_dir_writes
[L1I Directory Writes]
l1i_penalty_cycles
[L1I Penalty Cycles]
problem_state_cpu_cycles
[Problem State CPU Cycles]
problem_state_instructions
[Problem State Instructions]
....
csvn generic:
aes_blocked_cycles
[AES Blocked Cycles]
aes_blocked_functions
[AES Blocked Functions]
aes_cycles
[AES Cycles]
aes_functions
[AES Functions]
dea_blocked_cycles
[DEA Blocked Cycles]
dea_blocked_functions
[DEA Blocked Functions]
....
Signed-off-by: Thomas Richter <tmricht@linux.vnet.ibm.com>
Reviewed-by: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
---
tools/perf/util/pmu.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 1111d5bf15ca..8675ddf558c6 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -562,6 +562,12 @@ static int is_pmu_core(const char *name)
if (stat(path, &st) == 0)
return 1;
+ /* Look for cpu sysfs (specific to s390) */
+ scnprintf(path, PATH_MAX, "%s/bus/event_source/devices/%s",
+ sysfs, name);
+ if (stat(path, &st) == 0)
+ return 1;
+
return 0;
}
--
2.14.3
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf list: Add s390 support for detailed/verbose pmu event description
2018-04-09 11:00 [PATCH] perf list: Add s390 support for detailed/verbose pmu event description Thomas Richter
@ 2018-04-09 11:37 ` Mark Rutland
2018-04-09 13:03 ` Thomas-Mich Richter
0 siblings, 1 reply; 6+ messages in thread
From: Mark Rutland @ 2018-04-09 11:37 UTC (permalink / raw)
To: Thomas Richter
Cc: linux-kernel, linux-perf-users, acme, brueckner, schwidefsky,
heiko.carstens, Thomas Richter
Hi,
On Mon, Apr 09, 2018 at 01:00:15PM +0200, Thomas Richter wrote:
> From: Thomas Richter <tmricht@linux.vnet.ibm.com>
>
> Perf list with flags -d and -v print a description (-d) or
> a very verbose explanation (-v) of CPU specific counter events.
> These descriptions are provided with the json files in
> directory pmu-events/arch/s390/*.json.
>
> Display of these descriptions on s390 requires the
> corresponding json files.
>
> On s390 this does not work because function is_pmu_core()
> does not detect the s390 directory name where the
> CPU specific events are listed. On x86 it is
> /sys/bus/event_source/devices/cpu
> whereas on s390 it is
> /sys/bus/event_source/devices/cpum_cf
> /sys/bus/event_source/devices/cpum_sf
>
> Fix this by adding s390 directory name testing to
> function is_pmu_core(). This is the same approach as taken for
> arm platform.
I don't think this is quite right.
On arm, we specifically look for PMU directories which have a
file called CPUs. e.g.
/sys/bus/event_source/devices/arm_pmuv3/cpus
... where "arm_pmuv3" is the PMU name.
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 1111d5bf15ca..8675ddf558c6 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -562,6 +562,12 @@ static int is_pmu_core(const char *name)
> if (stat(path, &st) == 0)
> return 1;
>
> + /* Look for cpu sysfs (specific to s390) */
> + scnprintf(path, PATH_MAX, "%s/bus/event_source/devices/%s",
> + sysfs, name);
> + if (stat(path, &st) == 0)
> + return 1;
IIUC here we return true if the PMU has a sysfs directory, but all PMUs
(including uncore PMUs) should have such a directory, so this will make
is_pmu_core() always return true.
Can you match the "cpum_" prefix specifically, instead?
Thanks,
Mark.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf list: Add s390 support for detailed/verbose pmu event description
2018-04-09 11:37 ` Mark Rutland
@ 2018-04-09 13:03 ` Thomas-Mich Richter
2018-04-09 14:18 ` Mark Rutland
0 siblings, 1 reply; 6+ messages in thread
From: Thomas-Mich Richter @ 2018-04-09 13:03 UTC (permalink / raw)
To: Mark Rutland, Thomas Richter
Cc: linux-kernel, linux-perf-users, acme, brueckner, schwidefsky,
heiko.carstens
On 04/09/2018 01:37 PM, Mark Rutland wrote:
> Hi,
>
> On Mon, Apr 09, 2018 at 01:00:15PM +0200, Thomas Richter wrote:
>> From: Thomas Richter <tmricht@linux.vnet.ibm.com>
>>
>> Perf list with flags -d and -v print a description (-d) or
>> a very verbose explanation (-v) of CPU specific counter events.
>> These descriptions are provided with the json files in
>> directory pmu-events/arch/s390/*.json.
>>
>> Display of these descriptions on s390 requires the
>> corresponding json files.
>>
>> On s390 this does not work because function is_pmu_core()
>> does not detect the s390 directory name where the
>> CPU specific events are listed. On x86 it is
>> /sys/bus/event_source/devices/cpu
>> whereas on s390 it is
>> /sys/bus/event_source/devices/cpum_cf
>> /sys/bus/event_source/devices/cpum_sf
>>
>> Fix this by adding s390 directory name testing to
>> function is_pmu_core(). This is the same approach as taken for
>> arm platform.
>
> I don't think this is quite right.
>
> On arm, we specifically look for PMU directories which have a
> file called CPUs. e.g.
>
> /sys/bus/event_source/devices/arm_pmuv3/cpus
>
> ... where "arm_pmuv3" is the PMU name.
>
>> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
>> index 1111d5bf15ca..8675ddf558c6 100644
>> --- a/tools/perf/util/pmu.c
>> +++ b/tools/perf/util/pmu.c
>> @@ -562,6 +562,12 @@ static int is_pmu_core(const char *name)
>> if (stat(path, &st) == 0)
>> return 1;
>>
>> + /* Look for cpu sysfs (specific to s390) */
>> + scnprintf(path, PATH_MAX, "%s/bus/event_source/devices/%s",
>> + sysfs, name);
>> + if (stat(path, &st) == 0)
>> + return 1;
>
> IIUC here we return true if the PMU has a sysfs directory, but all PMUs
> (including uncore PMUs) should have such a directory, so this will make
> is_pmu_core() always return true.
>
> Can you match the "cpum_" prefix specifically, instead?
>
> Thanks,
> Mark.
>
I am sorry, I don't follow you.
When I look at the code function sequence
perf_pmu__scan()
+---> pmu_read_sysfs()
This functions scans directory /sys/bus/event_source/devices/
and calls for each entry in this directory. For s390 these entries exist:
cpum_sf cpum_cf tracepoint and software
+---> perf_pmu__find();
+---> pmu_lookup()
+---> pmu_format()
Looks for file /sys/bus/event_source/devices/cpum_cf/format/event
and parses its contents. This returns 0 on s390.
+---> pmu_type()
Looks for file /sys/bus/event_source/devices/cpum_cf/type and
and parses its contents. This returns 0 on s390.
+---> pmu_aliases()
Looks for directory /sys/bus/event_source/devices/cpum_cf/events and
reads out every entry name which is treated as a event name and added
to the list of PMU-Aliases together with the file contents. For example
/sys/bus/event_source/devices/cpum_cf/events/SF_CYCLES_BASIC with the
file contents event=0xb0000. This is the raw event number.
+----> pmu_aliases_parse() + perf_pmu__new_alias()
Check and add alias name to a list.
+---> pmu_cpumask()
This tests for files /sys/bus/event_source/devices/cpum_cf/cpus or
/sys/bus/event_source/devices/cpum_cf/cpumask which do not exist on
s390.
+---> pmu_add_cpu_aliases() adds the list of aliases such as cpum_sf/SF_CYCLES_BASIC/
to the global list of aliases. But ths happens only when
+---> is_pmu_core()
function returns true.
And for s390 it needs to test for /sys/bus/event_source/devices/cpum_sf/ and
/sys/bus/event_source/devices/cpum_cf/
directories.
These directories are used to read the alias names in function
pmu_aliases() above.
Maybe I misunderstand this whole scheme, but with this patch the perf list commands works...
--
Thomas Richter, Dept 3303, IBM LTC Boeblingen Germany
--
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf list: Add s390 support for detailed/verbose pmu event description
2018-04-09 13:03 ` Thomas-Mich Richter
@ 2018-04-09 14:18 ` Mark Rutland
2018-04-10 7:15 ` Thomas-Mich Richter
0 siblings, 1 reply; 6+ messages in thread
From: Mark Rutland @ 2018-04-09 14:18 UTC (permalink / raw)
To: Thomas-Mich Richter
Cc: Thomas Richter, linux-kernel, linux-perf-users, acme, brueckner,
schwidefsky, heiko.carstens
On Mon, Apr 09, 2018 at 03:03:32PM +0200, Thomas-Mich Richter wrote:
> On 04/09/2018 01:37 PM, Mark Rutland wrote:
> > On Mon, Apr 09, 2018 at 01:00:15PM +0200, Thomas Richter wrote:
> >> @@ -562,6 +562,12 @@ static int is_pmu_core(const char *name)
> >> if (stat(path, &st) == 0)
> >> return 1;
> >>
> >> + /* Look for cpu sysfs (specific to s390) */
> >> + scnprintf(path, PATH_MAX, "%s/bus/event_source/devices/%s",
> >> + sysfs, name);
> >> + if (stat(path, &st) == 0)
> >> + return 1;
> >
> > IIUC here we return true if the PMU has a sysfs directory, but all PMUs
> > (including uncore PMUs) should have such a directory, so this will make
> > is_pmu_core() always return true.
> >
> > Can you match the "cpum_" prefix specifically, instead?
> >
> > Thanks,
> > Mark.
>
> I am sorry, I don't follow you.
>
> When I look at the code function sequence
>
> perf_pmu__scan()
> +---> pmu_read_sysfs()
> This functions scans directory /sys/bus/event_source/devices/
> and calls for each entry in this directory. For s390 these entries exist:
> cpum_sf cpum_cf tracepoint and software
... and we want is:
is_pmu_core("cpum_sf") == true
is_pmu_core("cpum_cf") == true
is_pmu_core("tracepoint") == false
is_pmu_core("software") == false
> +---> perf_pmu__find();
> +---> pmu_lookup()
> +---> pmu_add_cpu_aliases() adds the list of aliases such as cpum_sf/SF_CYCLES_BASIC/
> to the global list of aliases. But ths happens only when
> +---> is_pmu_core()
> function returns true.
> And for s390 it needs to test for /sys/bus/event_source/devices/cpum_sf/ and
> /sys/bus/event_source/devices/cpum_cf/
> directories.
> These directories are used to read the alias names in function
> pmu_aliases() above.
However, your code also returns true for "tracepoint" and "software".
You check if there's a directory under event_source/devices/ for the PMU
name, and every PMU should have such a directory.
For example, on my x86 box I have
[mark@lakrids:~]% ls /sys/bus/event_source/devices
breakpoint power uncore_cbox_13 uncore_cbox_9 uncore_qpi_0
cpu software uncore_cbox_2 uncore_ha_0 uncore_qpi_1
cstate_core tracepoint uncore_cbox_3 uncore_ha_1 uncore_r2pcie
cstate_pkg uncore_cbox_0 uncore_cbox_4 uncore_imc_0 uncore_r3qpi_0
intel_bts uncore_cbox_1 uncore_cbox_5 uncore_imc_1 uncore_r3qpi_1
intel_cqm uncore_cbox_10 uncore_cbox_6 uncore_imc_4 uncore_ubox
intel_pt uncore_cbox_11 uncore_cbox_7 uncore_imc_5
msr uncore_cbox_12 uncore_cbox_8 uncore_pcu
... and with your patch and the below hack applied:
----
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index bd0a32b03523..cec6bf551fe3 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -675,6 +675,12 @@ static void pmu_add_cpu_aliases(struct list_head *head, struct perf_pmu *pmu)
break;
}
+ if (is_pmu_core(name)) {
+ printf ("is_pmu_core(\"%s\") is true\n", name);
+ } else {
+ printf ("is_pmu_core(\"%s\") is false\n", name);
+ }
+
if (!is_pmu_core(name)) {
/* check for uncore devices */
if (pe->pmu == NULL)
----
... is_pmu_core() returns true for every PMU:
[mark@lakrids:~/src/linux/tools/perf]% ./perf list | grep 'is true' | uniq
is_pmu_core("uncore_imc_4") is true
is_pmu_core("uncore_cbox_5") is true
is_pmu_core("uncore_ha_0") is true
is_pmu_core("uncore_cbox_3") is true
is_pmu_core("uncore_qpi_0") is true
is_pmu_core("cstate_pkg") is true
is_pmu_core("breakpoint") is true
is_pmu_core("uncore_imc_0") is true
is_pmu_core("uncore_ubox") is true
is_pmu_core("uncore_pcu") is true
is_pmu_core("uncore_cbox_1") is true
is_pmu_core("uncore_r3qpi_0") is true
is_pmu_core("uncore_cbox_13") is true
is_pmu_core("intel_cqm") is true
is_pmu_core("power") is true
is_pmu_core("cpu") is true
is_pmu_core("intel_pt") is true
is_pmu_core("uncore_cbox_11") is true
is_pmu_core("uncore_cbox_8") is true
is_pmu_core("uncore_imc_5") is true
is_pmu_core("software") is true
is_pmu_core("uncore_cbox_6") is true
is_pmu_core("uncore_ha_1") is true
is_pmu_core("uncore_r2pcie") is true
is_pmu_core("uncore_cbox_4") is true
is_pmu_core("intel_bts") is true
is_pmu_core("uncore_qpi_1") is true
is_pmu_core("uncore_imc_1") is true
is_pmu_core("uncore_cbox_2") is true
is_pmu_core("uncore_r3qpi_1") is true
is_pmu_core("uncore_cbox_0") is true
is_pmu_core("cstate_core") is true
is_pmu_core("uncore_cbox_12") is true
is_pmu_core("tracepoint") is true
is_pmu_core("uncore_cbox_9") is true
is_pmu_core("msr") is true
is_pmu_core("uncore_cbox_10") is true
is_pmu_core("uncore_cbox_7") is true
[mark@lakrids:~/src/linux/tools/perf]% ./perf list | grep 'is false' | wc -l
0
... whereas previously this was only the case for the CPU PMU:
[mark@lakrids:~/src/linux/tools/perf]% ./perf list | grep 'is true' | uniq
is_pmu_core("cpu") is true
[mark@lakrids:~/src/linux/tools/perf]% ./perf list | grep 'is false' | uniq
is_pmu_core("uncore_imc_4") is false
is_pmu_core("uncore_cbox_5") is false
is_pmu_core("uncore_ha_0") is false
is_pmu_core("uncore_cbox_3") is false
is_pmu_core("uncore_qpi_0") is false
is_pmu_core("cstate_pkg") is false
is_pmu_core("breakpoint") is false
is_pmu_core("uncore_imc_0") is false
is_pmu_core("uncore_ubox") is false
is_pmu_core("uncore_pcu") is false
is_pmu_core("uncore_cbox_1") is false
is_pmu_core("uncore_r3qpi_0") is false
is_pmu_core("uncore_cbox_13") is false
is_pmu_core("intel_cqm") is false
is_pmu_core("power") is false
is_pmu_core("intel_pt") is false
is_pmu_core("uncore_cbox_11") is false
is_pmu_core("uncore_cbox_8") is false
is_pmu_core("uncore_imc_5") is false
is_pmu_core("software") is false
is_pmu_core("uncore_cbox_6") is false
is_pmu_core("uncore_ha_1") is false
is_pmu_core("uncore_r2pcie") is false
is_pmu_core("uncore_cbox_4") is false
is_pmu_core("intel_bts") is false
is_pmu_core("uncore_qpi_1") is false
is_pmu_core("uncore_imc_1") is false
is_pmu_core("uncore_cbox_2") is false
is_pmu_core("uncore_r3qpi_1") is false
is_pmu_core("uncore_cbox_0") is false
is_pmu_core("cstate_core") is false
is_pmu_core("uncore_cbox_12") is false
is_pmu_core("tracepoint") is false
is_pmu_core("uncore_cbox_9") is false
is_pmu_core("msr") is false
is_pmu_core("uncore_cbox_10") is false
is_pmu_core("uncore_cbox_7") is false
If it's fine to have a tautological is_pmu_core(), then we can remove it
entirely.
My understanding was that we have the is_pmu_core() check to ensure that
we don't associate aliases with other PMUs in the system.
For example, on some arm platforms the CPU PMU isn't available, and we
shouldn't add the CPU PMU aliases just because we have a software PMU.
> Maybe I misunderstand this whole scheme, but with this patch the perf
> list commands works...
It looks like it works on my x86 box, at least, but I do think this is
wrong in some cases.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf list: Add s390 support for detailed/verbose pmu event description
2018-04-09 14:18 ` Mark Rutland
@ 2018-04-10 7:15 ` Thomas-Mich Richter
0 siblings, 0 replies; 6+ messages in thread
From: Thomas-Mich Richter @ 2018-04-10 7:15 UTC (permalink / raw)
To: Mark Rutland
Cc: Thomas Richter, linux-kernel, linux-perf-users, acme, brueckner,
schwidefsky, heiko.carstens
On 04/09/2018 04:18 PM, Mark Rutland wrote:
> On Mon, Apr 09, 2018 at 03:03:32PM +0200, Thomas-Mich Richter wrote:
>> On 04/09/2018 01:37 PM, Mark Rutland wrote:
>>> On Mon, Apr 09, 2018 at 01:00:15PM +0200, Thomas Richter wrote:
>>>> @@ -562,6 +562,12 @@ static int is_pmu_core(const char *name)
>>>> if (stat(path, &st) == 0)
>>>> return 1;
>>>>
>>>> + /* Look for cpu sysfs (specific to s390) */
>>>> + scnprintf(path, PATH_MAX, "%s/bus/event_source/devices/%s",
>>>> + sysfs, name);
>>>> + if (stat(path, &st) == 0)
>>>> + return 1;
>>>
>>> IIUC here we return true if the PMU has a sysfs directory, but all PMUs
>>> (including uncore PMUs) should have such a directory, so this will make
>>> is_pmu_core() always return true.
>>>
>>> Can you match the "cpum_" prefix specifically, instead?
>>>
>>> Thanks,
>>> Mark.
>>
>> I am sorry, I don't follow you.
>>
>> When I look at the code function sequence
>>
>> perf_pmu__scan()
>> +---> pmu_read_sysfs()
>> This functions scans directory /sys/bus/event_source/devices/
>> and calls for each entry in this directory. For s390 these entries exist:
>> cpum_sf cpum_cf tracepoint and software
>
> ... and we want is:
>
> is_pmu_core("cpum_sf") == true
> is_pmu_core("cpum_cf") == true
> is_pmu_core("tracepoint") == false
> is_pmu_core("software") == false
>
>> +---> perf_pmu__find();
>> +---> pmu_lookup()
>
>> +---> pmu_add_cpu_aliases() adds the list of aliases such as cpum_sf/SF_CYCLES_BASIC/
>> to the global list of aliases. But ths happens only when
>> +---> is_pmu_core()
>> function returns true.
>> And for s390 it needs to test for /sys/bus/event_source/devices/cpum_sf/ and
>> /sys/bus/event_source/devices/cpum_cf/
>> directories.
>> These directories are used to read the alias names in function
>> pmu_aliases() above.
>
> However, your code also returns true for "tracepoint" and "software".
>
> You check if there's a directory under event_source/devices/ for the PMU
> name, and every PMU should have such a directory.
>
> For example, on my x86 box I have
>
> [mark@lakrids:~]% ls /sys/bus/event_source/devices
> breakpoint power uncore_cbox_13 uncore_cbox_9 uncore_qpi_0
> cpu software uncore_cbox_2 uncore_ha_0 uncore_qpi_1
> cstate_core tracepoint uncore_cbox_3 uncore_ha_1 uncore_r2pcie
> cstate_pkg uncore_cbox_0 uncore_cbox_4 uncore_imc_0 uncore_r3qpi_0
> intel_bts uncore_cbox_1 uncore_cbox_5 uncore_imc_1 uncore_r3qpi_1
> intel_cqm uncore_cbox_10 uncore_cbox_6 uncore_imc_4 uncore_ubox
> intel_pt uncore_cbox_11 uncore_cbox_7 uncore_imc_5
> msr uncore_cbox_12 uncore_cbox_8 uncore_pcu
>
>
> ... and with your patch and the below hack applied:
>
> ----
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index bd0a32b03523..cec6bf551fe3 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -675,6 +675,12 @@ static void pmu_add_cpu_aliases(struct list_head *head, struct perf_pmu *pmu)
> break;
> }
>
> + if (is_pmu_core(name)) {
> + printf ("is_pmu_core(\"%s\") is true\n", name);
> + } else {
> + printf ("is_pmu_core(\"%s\") is false\n", name);
> + }
> +
> if (!is_pmu_core(name)) {
> /* check for uncore devices */
> if (pe->pmu == NULL)
> ----
>
> ... is_pmu_core() returns true for every PMU:
>
> [mark@lakrids:~/src/linux/tools/perf]% ./perf list | grep 'is true' | uniq
> is_pmu_core("uncore_imc_4") is true
> is_pmu_core("uncore_cbox_5") is true
> is_pmu_core("uncore_ha_0") is true
> is_pmu_core("uncore_cbox_3") is true
> is_pmu_core("uncore_qpi_0") is true
> is_pmu_core("cstate_pkg") is true
> is_pmu_core("breakpoint") is true
> is_pmu_core("uncore_imc_0") is true
> is_pmu_core("uncore_ubox") is true
> is_pmu_core("uncore_pcu") is true
> is_pmu_core("uncore_cbox_1") is true
> is_pmu_core("uncore_r3qpi_0") is true
> is_pmu_core("uncore_cbox_13") is true
> is_pmu_core("intel_cqm") is true
> is_pmu_core("power") is true
> is_pmu_core("cpu") is true
> is_pmu_core("intel_pt") is true
> is_pmu_core("uncore_cbox_11") is true
> is_pmu_core("uncore_cbox_8") is true
> is_pmu_core("uncore_imc_5") is true
> is_pmu_core("software") is true
> is_pmu_core("uncore_cbox_6") is true
> is_pmu_core("uncore_ha_1") is true
> is_pmu_core("uncore_r2pcie") is true
> is_pmu_core("uncore_cbox_4") is true
> is_pmu_core("intel_bts") is true
> is_pmu_core("uncore_qpi_1") is true
> is_pmu_core("uncore_imc_1") is true
> is_pmu_core("uncore_cbox_2") is true
> is_pmu_core("uncore_r3qpi_1") is true
> is_pmu_core("uncore_cbox_0") is true
> is_pmu_core("cstate_core") is true
> is_pmu_core("uncore_cbox_12") is true
> is_pmu_core("tracepoint") is true
> is_pmu_core("uncore_cbox_9") is true
> is_pmu_core("msr") is true
> is_pmu_core("uncore_cbox_10") is true
> is_pmu_core("uncore_cbox_7") is true
>
> [mark@lakrids:~/src/linux/tools/perf]% ./perf list | grep 'is false' | wc -l
> 0
>
>
> ... whereas previously this was only the case for the CPU PMU:
>
> [mark@lakrids:~/src/linux/tools/perf]% ./perf list | grep 'is true' | uniq
> is_pmu_core("cpu") is true
>
> [mark@lakrids:~/src/linux/tools/perf]% ./perf list | grep 'is false' | uniq
> is_pmu_core("uncore_imc_4") is false
> is_pmu_core("uncore_cbox_5") is false
> is_pmu_core("uncore_ha_0") is false
> is_pmu_core("uncore_cbox_3") is false
> is_pmu_core("uncore_qpi_0") is false
> is_pmu_core("cstate_pkg") is false
> is_pmu_core("breakpoint") is false
> is_pmu_core("uncore_imc_0") is false
> is_pmu_core("uncore_ubox") is false
> is_pmu_core("uncore_pcu") is false
> is_pmu_core("uncore_cbox_1") is false
> is_pmu_core("uncore_r3qpi_0") is false
> is_pmu_core("uncore_cbox_13") is false
> is_pmu_core("intel_cqm") is false
> is_pmu_core("power") is false
> is_pmu_core("intel_pt") is false
> is_pmu_core("uncore_cbox_11") is false
> is_pmu_core("uncore_cbox_8") is false
> is_pmu_core("uncore_imc_5") is false
> is_pmu_core("software") is false
> is_pmu_core("uncore_cbox_6") is false
> is_pmu_core("uncore_ha_1") is false
> is_pmu_core("uncore_r2pcie") is false
> is_pmu_core("uncore_cbox_4") is false
> is_pmu_core("intel_bts") is false
> is_pmu_core("uncore_qpi_1") is false
> is_pmu_core("uncore_imc_1") is false
> is_pmu_core("uncore_cbox_2") is false
> is_pmu_core("uncore_r3qpi_1") is false
> is_pmu_core("uncore_cbox_0") is false
> is_pmu_core("cstate_core") is false
> is_pmu_core("uncore_cbox_12") is false
> is_pmu_core("tracepoint") is false
> is_pmu_core("uncore_cbox_9") is false
> is_pmu_core("msr") is false
> is_pmu_core("uncore_cbox_10") is false
> is_pmu_core("uncore_cbox_7") is false
>
> If it's fine to have a tautological is_pmu_core(), then we can remove it
> entirely.
>
> My understanding was that we have the is_pmu_core() check to ensure that
> we don't associate aliases with other PMUs in the system.
>
> For example, on some arm platforms the CPU PMU isn't available, and we
> shouldn't add the CPU PMU aliases just because we have a software PMU.
>
>> Maybe I misunderstand this whole scheme, but with this patch the perf
>> list commands works...
>
> It looks like it works on my x86 box, at least, but I do think this is
> wrong in some cases.
>
> Thanks,
> Mark.
>
Thank you very much for your explanation. I think I got your point
and will provide a reworked patch which returns ok for the s390
PMUs named cpum_cf and cpum_sf.
--
Thomas Richter, Dept 3303, IBM LTC Boeblingen Germany
--
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] perf list: Add s390 support for detailed/verbose pmu event description
@ 2018-03-21 6:53 Thomas Richter
0 siblings, 0 replies; 6+ messages in thread
From: Thomas Richter @ 2018-03-21 6:53 UTC (permalink / raw)
To: linux-kernel, linux-perf-users, acme
Cc: brueckner, schwidefsky, heiko.carstens, Thomas Richter
Perf list with flags -d and -v print a description (-d) or
a very verbose explanation (-v) of CPU specific counter events.
These descriptions are provided with the json files in
directory pmu-events/arch/s390/*.json.
Display of these descriptions on s390 requires the
corresponding json files.
On s390 this does not work because function is_pmu_core()
does not detect the s390 directory name where the
CPU specific events are listed. On x86 it is
/sys/bus/event_source/devices/cpu
whereas on s390 it is
/sys/bus/event_source/devices/cpum_cf
/sys/bus/event_source/devices/cpum_sf
Fix this by adding s390 directory name testing to
function is_pmu_core(). This is the same approach as taken for
arm platform.
Output before:
[root@s35lp76 perf]# ./perf list -d pmu
List of pre-defined events (to be used in -e):
cpum_cf/AES_BLOCKED_CYCLES/ [Kernel PMU event]
cpum_cf/AES_BLOCKED_FUNCTIONS/ [Kernel PMU event]
cpum_cf/AES_CYCLES/ [Kernel PMU event]
cpum_cf/AES_FUNCTIONS/ [Kernel PMU event]
....
cpum_cf/TX_NC_TEND/ [Kernel PMU event]
cpum_cf/VX_BCD_EXECUTION_SLOTS/ [Kernel PMU event]
cpum_sf/SF_CYCLES_BASIC/ [Kernel PMU event]
Output after:
[root@s35lp76 perf]# ./perf list -d pmu
List of pre-defined events (to be used in -e):
cpum_cf/AES_BLOCKED_CYCLES/ [Kernel PMU event]
cpum_cf/AES_BLOCKED_FUNCTIONS/ [Kernel PMU event]
cpum_cf/AES_CYCLES/ [Kernel PMU event]
cpum_cf/AES_FUNCTIONS/ [Kernel PMU event]
....
cpum_cf/TX_NC_TEND/ [Kernel PMU event]
cpum_cf/VX_BCD_EXECUTION_SLOTS/ [Kernel PMU event]
cpum_sf/SF_CYCLES_BASIC/ [Kernel PMU event]
3906:
bcd_dfp_execution_slots
[BCD DFP Execution Slots]
decimal_instructions
[Decimal Instructions]
dtlb2_gpage_writes
[DTLB2 GPAGE Writes]
dtlb2_hpage_writes
[DTLB2 HPAGE Writes]
dtlb2_misses
[DTLB2 Misses]
dtlb2_writes
[DTLB2 Writes]
itlb2_misses
[ITLB2 Misses]
itlb2_writes
[ITLB2 Writes]
l1c_tlb2_misses
[L1C TLB2 Misses]
.....
cfvn 3:
cpu_cycles
[CPU Cycles]
instructions
[Instructions]
l1d_dir_writes
[L1D Directory Writes]
l1d_penalty_cycles
[L1D Penalty Cycles]
l1i_dir_writes
[L1I Directory Writes]
l1i_penalty_cycles
[L1I Penalty Cycles]
problem_state_cpu_cycles
[Problem State CPU Cycles]
problem_state_instructions
[Problem State Instructions]
....
csvn generic:
aes_blocked_cycles
[AES Blocked Cycles]
aes_blocked_functions
[AES Blocked Functions]
aes_cycles
[AES Cycles]
aes_functions
[AES Functions]
dea_blocked_cycles
[DEA Blocked Cycles]
dea_blocked_functions
[DEA Blocked Functions]
....
Signed-off-by: Thomas Richter <tmricht@linux.vnet.ibm.com>
Reviewed-by: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
---
tools/perf/util/pmu.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 1111d5bf15ca..8675ddf558c6 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -562,6 +562,12 @@ static int is_pmu_core(const char *name)
if (stat(path, &st) == 0)
return 1;
+ /* Look for cpu sysfs (specific to s390) */
+ scnprintf(path, PATH_MAX, "%s/bus/event_source/devices/%s",
+ sysfs, name);
+ if (stat(path, &st) == 0)
+ return 1;
+
return 0;
}
--
2.14.3
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-04-10 7:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-09 11:00 [PATCH] perf list: Add s390 support for detailed/verbose pmu event description Thomas Richter
2018-04-09 11:37 ` Mark Rutland
2018-04-09 13:03 ` Thomas-Mich Richter
2018-04-09 14:18 ` Mark Rutland
2018-04-10 7:15 ` Thomas-Mich Richter
-- strict thread matches above, loose matches on Subject: below --
2018-03-21 6:53 Thomas Richter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).