[PATCH V2] tools/perf/test: Fix perf all PMU test to skip hv_24x7/hv_gpci tests on powerpc

[PATCH V2] tools/perf/test: Fix perf all PMU test to skip hv_24x7/hv_gpci tests on powerpc

[Athira Rajeev]

"perf all PMU test" picks the input events from
"perf list --raw-dump pmu" list and runs "perf stat -e"
for each of the event in the list. In case of powerpc, the
PowerVM environment supports events from hv_24x7 and hv_gpci
PMU which is of example format like below:
- hv_24x7/CPM_ADJUNCT_INST,domain=?,core=?/
- hv_gpci/event,partition_id=?/

The value for "?" needs to be filled in depending on
system and respective event. CPM_ADJUNCT_INST needs have
core value and domain value. hv_gpci event needs partition_id.
Similarly, there are other events for hv_24x7 and hv_gpci
having "?" in event format. Hence skip these events on powerpc
platform since values like partition_id, domain is specific
to system and event.

Fixes: 3d5ac9effcc6 ("perf test: Workload test of all PMUs")
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
Changelog:
 v1 -> v2:
 Instead of checking for platform, used the pmu name
 ie, hv_24x7 and hv_gpci to skip the test since this
 pmu name is specific to powerpc as suggested by
 Michael Ellerman.

 tools/perf/tests/shell/stat_all_pmu.sh | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tools/perf/tests/shell/stat_all_pmu.sh b/tools/perf/tests/shell/stat_all_pmu.sh
index b30dba455f36..7d046bb8a7b9 100755
--- a/tools/perf/tests/shell/stat_all_pmu.sh
+++ b/tools/perf/tests/shell/stat_all_pmu.sh
@@ -5,6 +5,16 @@
 set -e
 
 for p in $(perf list --raw-dump pmu); do
+  # In powerpc, skip the events for hv_24x7 and hv_gpci.
+  # These events needs input values to be filled in for
+  # core, chip, patition id based on system.
+  # Example: hv_24x7/CPM_ADJUNCT_INST,domain=?,core=?/
+  # hv_gpci/event,partition_id=?/
+  # Hence skip these events for ppc.
+  if echo "$p" |grep -Eq 'hv_24x7|hv_gpci' ; then
+    echo "Skipping: Event '$p' in powerpc"
+    continue
+  fi
   echo "Testing $p"
   result=$(perf stat -e "$p" true 2>&1)
   if ! echo "$result" | grep -q "$p" && ! echo "$result" | grep -q "<not supported>" ; then
-- 
2.35.1

Re: [PATCH V2] tools/perf/test: Fix perf all PMU test to skip hv_24x7/hv_gpci tests on powerpc

[Ian Rogers]

On Thu, May 19, 2022 at 8:43 AM Athira Rajeev
<atrajeev@linux.vnet.ibm.com> wrote:
>
> "perf all PMU test" picks the input events from
> "perf list --raw-dump pmu" list and runs "perf stat -e"
> for each of the event in the list. In case of powerpc, the
> PowerVM environment supports events from hv_24x7 and hv_gpci
> PMU which is of example format like below:
> - hv_24x7/CPM_ADJUNCT_INST,domain=?,core=?/
> - hv_gpci/event,partition_id=?/
>
> The value for "?" needs to be filled in depending on
> system and respective event. CPM_ADJUNCT_INST needs have
> core value and domain value. hv_gpci event needs partition_id.
> Similarly, there are other events for hv_24x7 and hv_gpci
> having "?" in event format. Hence skip these events on powerpc
> platform since values like partition_id, domain is specific
> to system and event.
>
> Fixes: 3d5ac9effcc6 ("perf test: Workload test of all PMUs")
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> ---
> Changelog:
>  v1 -> v2:
>  Instead of checking for platform, used the pmu name
>  ie, hv_24x7 and hv_gpci to skip the test since this
>  pmu name is specific to powerpc as suggested by
>  Michael Ellerman.
>
>  tools/perf/tests/shell/stat_all_pmu.sh | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/tools/perf/tests/shell/stat_all_pmu.sh b/tools/perf/tests/shell/stat_all_pmu.sh
> index b30dba455f36..7d046bb8a7b9 100755
> --- a/tools/perf/tests/shell/stat_all_pmu.sh
> +++ b/tools/perf/tests/shell/stat_all_pmu.sh
> @@ -5,6 +5,16 @@
>  set -e
>
>  for p in $(perf list --raw-dump pmu); do
> +  # In powerpc, skip the events for hv_24x7 and hv_gpci.
> +  # These events needs input values to be filled in for
> +  # core, chip, patition id based on system.
nit: s/patition/partition/
> +  # Example: hv_24x7/CPM_ADJUNCT_INST,domain=?,core=?/

I've no problem with this patch, we may need to do similar for other
architectures. For this specific problem I wonder if rather than
skipping the event the event can be fixed with domain and core set to
1 ? This would provide a little additional coverage.

Thanks,
Ian

> +  # hv_gpci/event,partition_id=?/
> +  # Hence skip these events for ppc.
> +  if echo "$p" |grep -Eq 'hv_24x7|hv_gpci' ; then
> +    echo "Skipping: Event '$p' in powerpc"
> +    continue
> +  fi
>    echo "Testing $p"
>    result=$(perf stat -e "$p" true 2>&1)
>    if ! echo "$result" | grep -q "$p" && ! echo "$result" | grep -q "<not supported>" ; then
> --
> 2.35.1
>

Re: [PATCH V2] tools/perf/test: Fix perf all PMU test to skip hv_24x7/hv_gpci tests on powerpc

[Athira Rajeev]

> On 20-May-2022, at 12:15 AM, Ian Rogers <irogers@google.com> wrote:
> 
> On Thu, May 19, 2022 at 8:43 AM Athira Rajeev
> <atrajeev@linux.vnet.ibm.com> wrote:
>> 
>> "perf all PMU test" picks the input events from
>> "perf list --raw-dump pmu" list and runs "perf stat -e"
>> for each of the event in the list. In case of powerpc, the
>> PowerVM environment supports events from hv_24x7 and hv_gpci
>> PMU which is of example format like below:
>> - hv_24x7/CPM_ADJUNCT_INST,domain=?,core=?/
>> - hv_gpci/event,partition_id=?/
>> 
>> The value for "?" needs to be filled in depending on
>> system and respective event. CPM_ADJUNCT_INST needs have
>> core value and domain value. hv_gpci event needs partition_id.
>> Similarly, there are other events for hv_24x7 and hv_gpci
>> having "?" in event format. Hence skip these events on powerpc
>> platform since values like partition_id, domain is specific
>> to system and event.
>> 
>> Fixes: 3d5ac9effcc6 ("perf test: Workload test of all PMUs")
>> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
>> ---
>> Changelog:
>> v1 -> v2:
>> Instead of checking for platform, used the pmu name
>> ie, hv_24x7 and hv_gpci to skip the test since this
>> pmu name is specific to powerpc as suggested by
>> Michael Ellerman.
>> 
>> tools/perf/tests/shell/stat_all_pmu.sh | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>> 
>> diff --git a/tools/perf/tests/shell/stat_all_pmu.sh b/tools/perf/tests/shell/stat_all_pmu.sh
>> index b30dba455f36..7d046bb8a7b9 100755
>> --- a/tools/perf/tests/shell/stat_all_pmu.sh
>> +++ b/tools/perf/tests/shell/stat_all_pmu.sh
>> @@ -5,6 +5,16 @@
>> set -e
>> 
>> for p in $(perf list --raw-dump pmu); do
>> +  # In powerpc, skip the events for hv_24x7 and hv_gpci.
>> +  # These events needs input values to be filled in for
>> +  # core, chip, patition id based on system.
> nit: s/patition/partition/
>> +  # Example: hv_24x7/CPM_ADJUNCT_INST,domain=?,core=?/
> 
> I've no problem with this patch, we may need to do similar for other
> architectures. For this specific problem I wonder if rather than
> skipping the event the event can be fixed with domain and core set to
> 1 ? This would provide a little additional coverage.

Hi Ian,

Thanks for review. Yeah, we looked at the option of having default value of 0.
But then we could still fail due to the partition privilege. These PMU counters expose per-core and per-chip
data and partition needs additional privileges to get these pmu data.
Hence the skip :)

Thanks
Athira

> 
> Thanks,
> Ian
> 
>> +  # hv_gpci/event,partition_id=?/
>> +  # Hence skip these events for ppc.
>> +  if echo "$p" |grep -Eq 'hv_24x7|hv_gpci' ; then
>> +    echo "Skipping: Event '$p' in powerpc"
>> +    continue
>> +  fi
>>   echo "Testing $p"
>>   result=$(perf stat -e "$p" true 2>&1)
>>   if ! echo "$result" | grep -q "$p" && ! echo "$result" | grep -q "<not supported>" ; then
>> --
>> 2.35.1

Re: [PATCH V2] tools/perf/test: Fix perf all PMU test to skip hv_24x7/hv_gpci tests on powerpc

[Arnaldo Carvalho de Melo]

Em Thu, May 19, 2022 at 11:45:07AM -0700, Ian Rogers escreveu:
> On Thu, May 19, 2022 at 8:43 AM Athira Rajeev
> <atrajeev@linux.vnet.ibm.com> wrote:
> >
> > "perf all PMU test" picks the input events from
> > "perf list --raw-dump pmu" list and runs "perf stat -e"
> > for each of the event in the list. In case of powerpc, the
> > PowerVM environment supports events from hv_24x7 and hv_gpci
> > PMU which is of example format like below:
> > - hv_24x7/CPM_ADJUNCT_INST,domain=?,core=?/
> > - hv_gpci/event,partition_id=?/
> >
> > The value for "?" needs to be filled in depending on
> > system and respective event. CPM_ADJUNCT_INST needs have
> > core value and domain value. hv_gpci event needs partition_id.
> > Similarly, there are other events for hv_24x7 and hv_gpci
> > having "?" in event format. Hence skip these events on powerpc
> > platform since values like partition_id, domain is specific
> > to system and event.
> >
> > Fixes: 3d5ac9effcc6 ("perf test: Workload test of all PMUs")
> > Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> > ---
> > Changelog:
> >  v1 -> v2:
> >  Instead of checking for platform, used the pmu name
> >  ie, hv_24x7 and hv_gpci to skip the test since this
> >  pmu name is specific to powerpc as suggested by
> >  Michael Ellerman.
> >
> >  tools/perf/tests/shell/stat_all_pmu.sh | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/tools/perf/tests/shell/stat_all_pmu.sh b/tools/perf/tests/shell/stat_all_pmu.sh
> > index b30dba455f36..7d046bb8a7b9 100755
> > --- a/tools/perf/tests/shell/stat_all_pmu.sh
> > +++ b/tools/perf/tests/shell/stat_all_pmu.sh
> > @@ -5,6 +5,16 @@
> >  set -e
> >
> >  for p in $(perf list --raw-dump pmu); do
> > +  # In powerpc, skip the events for hv_24x7 and hv_gpci.
> > +  # These events needs input values to be filled in for
> > +  # core, chip, patition id based on system.
> nit: s/patition/partition/
> > +  # Example: hv_24x7/CPM_ADJUNCT_INST,domain=?,core=?/
> 
> I've no problem with this patch, we may need to do similar for other

Can I take this as an Acked-by?

- Arnaldo

> architectures. For this specific problem I wonder if rather than
> skipping the event the event can be fixed with domain and core set to
> 1 ? This would provide a little additional coverage.
> 
> Thanks,
> Ian
> 
> > +  # hv_gpci/event,partition_id=?/
> > +  # Hence skip these events for ppc.
> > +  if echo "$p" |grep -Eq 'hv_24x7|hv_gpci' ; then
> > +    echo "Skipping: Event '$p' in powerpc"
> > +    continue
> > +  fi
> >    echo "Testing $p"
> >    result=$(perf stat -e "$p" true 2>&1)
> >    if ! echo "$result" | grep -q "$p" && ! echo "$result" | grep -q "<not supported>" ; then
> > --
> > 2.35.1
> >

-- 

- Arnaldo

Re: [PATCH V2] tools/perf/test: Fix perf all PMU test to skip hv_24x7/hv_gpci tests on powerpc

[Ian Rogers]

On Fri, May 20, 2022 at 6:53 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Thu, May 19, 2022 at 11:45:07AM -0700, Ian Rogers escreveu:
> > On Thu, May 19, 2022 at 8:43 AM Athira Rajeev
> > <atrajeev@linux.vnet.ibm.com> wrote:
> > >
> > > "perf all PMU test" picks the input events from
> > > "perf list --raw-dump pmu" list and runs "perf stat -e"
> > > for each of the event in the list. In case of powerpc, the
> > > PowerVM environment supports events from hv_24x7 and hv_gpci
> > > PMU which is of example format like below:
> > > - hv_24x7/CPM_ADJUNCT_INST,domain=?,core=?/
> > > - hv_gpci/event,partition_id=?/
> > >
> > > The value for "?" needs to be filled in depending on
> > > system and respective event. CPM_ADJUNCT_INST needs have
> > > core value and domain value. hv_gpci event needs partition_id.
> > > Similarly, there are other events for hv_24x7 and hv_gpci
> > > having "?" in event format. Hence skip these events on powerpc
> > > platform since values like partition_id, domain is specific
> > > to system and event.
> > >
> > > Fixes: 3d5ac9effcc6 ("perf test: Workload test of all PMUs")
> > > Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> > > ---
> > > Changelog:
> > >  v1 -> v2:
> > >  Instead of checking for platform, used the pmu name
> > >  ie, hv_24x7 and hv_gpci to skip the test since this
> > >  pmu name is specific to powerpc as suggested by
> > >  Michael Ellerman.
> > >
> > >  tools/perf/tests/shell/stat_all_pmu.sh | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > >
> > > diff --git a/tools/perf/tests/shell/stat_all_pmu.sh b/tools/perf/tests/shell/stat_all_pmu.sh
> > > index b30dba455f36..7d046bb8a7b9 100755
> > > --- a/tools/perf/tests/shell/stat_all_pmu.sh
> > > +++ b/tools/perf/tests/shell/stat_all_pmu.sh
> > > @@ -5,6 +5,16 @@
> > >  set -e
> > >
> > >  for p in $(perf list --raw-dump pmu); do
> > > +  # In powerpc, skip the events for hv_24x7 and hv_gpci.
> > > +  # These events needs input values to be filled in for
> > > +  # core, chip, patition id based on system.
> > nit: s/patition/partition/
> > > +  # Example: hv_24x7/CPM_ADJUNCT_INST,domain=?,core=?/
> >
> > I've no problem with this patch, we may need to do similar for other
>
> Can I take this as an Acked-by?
>
> - Arnaldo


Acked-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> > architectures. For this specific problem I wonder if rather than
> > skipping the event the event can be fixed with domain and core set to
> > 1 ? This would provide a little additional coverage.
> >
> > Thanks,
> > Ian
> >
> > > +  # hv_gpci/event,partition_id=?/
> > > +  # Hence skip these events for ppc.
> > > +  if echo "$p" |grep -Eq 'hv_24x7|hv_gpci' ; then
> > > +    echo "Skipping: Event '$p' in powerpc"
> > > +    continue
> > > +  fi
> > >    echo "Testing $p"
> > >    result=$(perf stat -e "$p" true 2>&1)
> > >    if ! echo "$result" | grep -q "$p" && ! echo "$result" | grep -q "<not supported>" ; then
> > > --
> > > 2.35.1
> > >
>
> --
>
> - Arnaldo