[PATCH] perf test stat_all_pmu.sh: Correctly check 'perf stat' result

[PATCH] perf test stat_all_pmu.sh: Correctly check 'perf stat' result

[vmolnaro]

From: Veronika Molnarova <vmolnaro@redhat.com>

Test case "stat_all_pmu.sh" is not correctly checking 'perf stat' output
due to a poor design. Firstly, having the 'set -e' option with a trap
catching the sigexit causes the shell to exit immediately if 'perf stat' ends
with any non-zero value, which is then caught by the trap reporting an
unexpected signal. This causes events that should be parsed by the if-else
statement to be caught by the trap handler and are reported as errors:

    $ perf test -vv "perf all pmu"
    Testing i915/actual-frequency/
    Unexpected signal in main
    Error:
    Access to performance monitoring and observability operations is limited.

Secondly, the if-else branches are not exclusive as the checking if the
event is present in the output log covers also the "<not supported>"
events, which should be accepted, and also the "Bad name events", which
should be rejected.

Remove the "set -e" option from the test case, correctly parse the
"perf stat" output log and check its return value. Add the missing
outputs for the 'perf stat' result and also add logs messages to
report the branch that parsed the event for more info.

Fixes: 7e73ea40295620e7 ("perf test: Ignore security failures in all PMU test")
Signed-off-by: Veronika Molnarova <vmolnaro@redhat.com>
---
 tools/perf/tests/shell/stat_all_pmu.sh | 48 ++++++++++++++++++--------
 1 file changed, 34 insertions(+), 14 deletions(-)

diff --git a/tools/perf/tests/shell/stat_all_pmu.sh b/tools/perf/tests/shell/stat_all_pmu.sh
index 8b148b300be1131e..9c466c0efa857fc2 100755
--- a/tools/perf/tests/shell/stat_all_pmu.sh
+++ b/tools/perf/tests/shell/stat_all_pmu.sh
@@ -2,7 +2,6 @@
 # perf all PMU test (exclusive)
 # SPDX-License-Identifier: GPL-2.0
 
-set -e
 err=0
 result=""
 
@@ -16,34 +15,55 @@ trap trap_cleanup EXIT TERM INT
 # Test all PMU events; however exclude parameterized ones (name contains '?')
 for p in $(perf list --raw-dump pmu | sed 's/[[:graph:]]\+?[[:graph:]]\+[[:space:]]//g')
 do
-  echo "Testing $p"
-  result=$(perf stat -e "$p" true 2>&1)
-  if echo "$result" | grep -q "$p"
+  echo -n "Testing $p -- "
+  output=$(perf stat -e "$p" true 2>&1)
+  stat_result=$?
+  if echo "$output" | grep -q "$p"
   then
     # Event seen in output.
-    continue
-  fi
-  if echo "$result" | grep -q "<not supported>"
-  then
-    # Event not supported, so ignore.
-    continue
+    if [ $stat_result -eq 0 ] && ! echo "$output" | grep -q "<not supported>"
+    then
+      # Event supported.
+      echo "supported"
+      continue
+    elif echo "$output" | grep -q "<not supported>"
+    then
+      # Event not supported, so ignore.
+      echo "not supported"
+      continue
+    elif echo "$output" | grep -q "No permission to enable"
+    then
+      # No permissions, so ignore.
+      echo "no permission to enable"
+      continue
+    elif echo "$output" | grep -q "Bad event name"
+    then
+      # Non-existent event.
+      echo "Error: Bad event name"
+      echo "$output"
+      err=1
+      continue
+    fi
   fi
-  if echo "$result" | grep -q "Access to performance monitoring and observability operations is limited."
+
+  if echo "$output" | grep -q "Access to performance monitoring and observability operations is limited."
   then
     # Access is limited, so ignore.
+    echo "access limited"
     continue
   fi
 
   # We failed to see the event and it is supported. Possibly the workload was
   # too small so retry with something longer.
-  result=$(perf stat -e "$p" perf bench internals synthesize 2>&1)
-  if echo "$result" | grep -q "$p"
+  output=$(perf stat -e "$p" perf bench internals synthesize 2>&1)
+  if echo "$output" | grep -q "$p"
   then
     # Event seen in output.
+    echo "supported"
     continue
   fi
   echo "Error: event '$p' not printed in:"
-  echo "$result"
+  echo "$output"
   err=1
 done
 
-- 
2.43.0

Re: [PATCH] perf test stat_all_pmu.sh: Correctly check 'perf stat' result

[Veronika Molnarova]

On 11/23/24 00:12, vmolnaro@redhat.com wrote:
> From: Veronika Molnarova <vmolnaro@redhat.com>
> 
> Test case "stat_all_pmu.sh" is not correctly checking 'perf stat' output
> due to a poor design. Firstly, having the 'set -e' option with a trap
> catching the sigexit causes the shell to exit immediately if 'perf stat' ends
> with any non-zero value, which is then caught by the trap reporting an
> unexpected signal. This causes events that should be parsed by the if-else
> statement to be caught by the trap handler and are reported as errors:
> 
>     $ perf test -vv "perf all pmu"
>     Testing i915/actual-frequency/
>     Unexpected signal in main
>     Error:
>     Access to performance monitoring and observability operations is limited.
> 
> Secondly, the if-else branches are not exclusive as the checking if the
> event is present in the output log covers also the "<not supported>"
> events, which should be accepted, and also the "Bad name events", which
> should be rejected.
> 
> Remove the "set -e" option from the test case, correctly parse the
> "perf stat" output log and check its return value. Add the missing
> outputs for the 'perf stat' result and also add logs messages to
> report the branch that parsed the event for more info.

Hi Ian,

is there anything that I am missing? If so would be great to know the idea
of your patch. This issue is getting quite old so would be great to get a proper
fix together.

Thanks,
Veronika
> 
> Fixes: 7e73ea40295620e7 ("perf test: Ignore security failures in all PMU test")
> Signed-off-by: Veronika Molnarova <vmolnaro@redhat.com>
> ---
>  tools/perf/tests/shell/stat_all_pmu.sh | 48 ++++++++++++++++++--------
>  1 file changed, 34 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/perf/tests/shell/stat_all_pmu.sh b/tools/perf/tests/shell/stat_all_pmu.sh
> index 8b148b300be1131e..9c466c0efa857fc2 100755
> --- a/tools/perf/tests/shell/stat_all_pmu.sh
> +++ b/tools/perf/tests/shell/stat_all_pmu.sh
> @@ -2,7 +2,6 @@
>  # perf all PMU test (exclusive)
>  # SPDX-License-Identifier: GPL-2.0
>  
> -set -e
>  err=0
>  result=""
>  
> @@ -16,34 +15,55 @@ trap trap_cleanup EXIT TERM INT
>  # Test all PMU events; however exclude parameterized ones (name contains '?')
>  for p in $(perf list --raw-dump pmu | sed 's/[[:graph:]]\+?[[:graph:]]\+[[:space:]]//g')
>  do
> -  echo "Testing $p"
> -  result=$(perf stat -e "$p" true 2>&1)
> -  if echo "$result" | grep -q "$p"
> +  echo -n "Testing $p -- "
> +  output=$(perf stat -e "$p" true 2>&1)
> +  stat_result=$?
> +  if echo "$output" | grep -q "$p"
>    then
>      # Event seen in output.
> -    continue
> -  fi
> -  if echo "$result" | grep -q "<not supported>"
> -  then
> -    # Event not supported, so ignore.
> -    continue
> +    if [ $stat_result -eq 0 ] && ! echo "$output" | grep -q "<not supported>"
> +    then
> +      # Event supported.
> +      echo "supported"
> +      continue
> +    elif echo "$output" | grep -q "<not supported>"
> +    then
> +      # Event not supported, so ignore.
> +      echo "not supported"
> +      continue
> +    elif echo "$output" | grep -q "No permission to enable"
> +    then
> +      # No permissions, so ignore.
> +      echo "no permission to enable"
> +      continue
> +    elif echo "$output" | grep -q "Bad event name"
> +    then
> +      # Non-existent event.
> +      echo "Error: Bad event name"
> +      echo "$output"
> +      err=1
> +      continue
> +    fi
>    fi
> -  if echo "$result" | grep -q "Access to performance monitoring and observability operations is limited."
> +
> +  if echo "$output" | grep -q "Access to performance monitoring and observability operations is limited."
>    then
>      # Access is limited, so ignore.
> +    echo "access limited"
>      continue
>    fi
>  
>    # We failed to see the event and it is supported. Possibly the workload was
>    # too small so retry with something longer.
> -  result=$(perf stat -e "$p" perf bench internals synthesize 2>&1)
> -  if echo "$result" | grep -q "$p"
> +  output=$(perf stat -e "$p" perf bench internals synthesize 2>&1)
> +  if echo "$output" | grep -q "$p"
>    then
>      # Event seen in output.
> +    echo "supported"
>      continue
>    fi
>    echo "Error: event '$p' not printed in:"
> -  echo "$result"
> +  echo "$output"
>    err=1
>  done
>  

Re: [PATCH] perf test stat_all_pmu.sh: Correctly check 'perf stat' result

[Veronika Molnarova]

On 12/10/24 16:20, Veronika Molnarova wrote:
> 
> 
> On 11/23/24 00:12, vmolnaro@redhat.com wrote:
>> From: Veronika Molnarova <vmolnaro@redhat.com>
>>
>> Test case "stat_all_pmu.sh" is not correctly checking 'perf stat' output
>> due to a poor design. Firstly, having the 'set -e' option with a trap
>> catching the sigexit causes the shell to exit immediately if 'perf stat' ends
>> with any non-zero value, which is then caught by the trap reporting an
>> unexpected signal. This causes events that should be parsed by the if-else
>> statement to be caught by the trap handler and are reported as errors:
>>
>>     $ perf test -vv "perf all pmu"
>>     Testing i915/actual-frequency/
>>     Unexpected signal in main
>>     Error:
>>     Access to performance monitoring and observability operations is limited.
>>
>> Secondly, the if-else branches are not exclusive as the checking if the
>> event is present in the output log covers also the "<not supported>"
>> events, which should be accepted, and also the "Bad name events", which
>> should be rejected.
>>
>> Remove the "set -e" option from the test case, correctly parse the
>> "perf stat" output log and check its return value. Add the missing
>> outputs for the 'perf stat' result and also add logs messages to
>> report the branch that parsed the event for more info.
> 
> Hi Ian,
> 
> is there anything that I am missing? If so would be great to know the idea
> of your patch. This issue is getting quite old so would be great to get a proper
> fix together.
> 
> Thanks,
> Veronika

Hello,

just wanted to ping the patch. Would be great to sort it out and close the issue.

Thanks,
Veronika

>>
>> Fixes: 7e73ea40295620e7 ("perf test: Ignore security failures in all PMU test")
>> Signed-off-by: Veronika Molnarova <vmolnaro@redhat.com>
>> ---
>>  tools/perf/tests/shell/stat_all_pmu.sh | 48 ++++++++++++++++++--------
>>  1 file changed, 34 insertions(+), 14 deletions(-)
>>
>> diff --git a/tools/perf/tests/shell/stat_all_pmu.sh b/tools/perf/tests/shell/stat_all_pmu.sh
>> index 8b148b300be1131e..9c466c0efa857fc2 100755
>> --- a/tools/perf/tests/shell/stat_all_pmu.sh
>> +++ b/tools/perf/tests/shell/stat_all_pmu.sh
>> @@ -2,7 +2,6 @@
>>  # perf all PMU test (exclusive)
>>  # SPDX-License-Identifier: GPL-2.0
>>  
>> -set -e
>>  err=0
>>  result=""
>>  
>> @@ -16,34 +15,55 @@ trap trap_cleanup EXIT TERM INT
>>  # Test all PMU events; however exclude parameterized ones (name contains '?')
>>  for p in $(perf list --raw-dump pmu | sed 's/[[:graph:]]\+?[[:graph:]]\+[[:space:]]//g')
>>  do
>> -  echo "Testing $p"
>> -  result=$(perf stat -e "$p" true 2>&1)
>> -  if echo "$result" | grep -q "$p"
>> +  echo -n "Testing $p -- "
>> +  output=$(perf stat -e "$p" true 2>&1)
>> +  stat_result=$?
>> +  if echo "$output" | grep -q "$p"
>>    then
>>      # Event seen in output.
>> -    continue
>> -  fi
>> -  if echo "$result" | grep -q "<not supported>"
>> -  then
>> -    # Event not supported, so ignore.
>> -    continue
>> +    if [ $stat_result -eq 0 ] && ! echo "$output" | grep -q "<not supported>"
>> +    then
>> +      # Event supported.
>> +      echo "supported"
>> +      continue
>> +    elif echo "$output" | grep -q "<not supported>"
>> +    then
>> +      # Event not supported, so ignore.
>> +      echo "not supported"
>> +      continue
>> +    elif echo "$output" | grep -q "No permission to enable"
>> +    then
>> +      # No permissions, so ignore.
>> +      echo "no permission to enable"
>> +      continue
>> +    elif echo "$output" | grep -q "Bad event name"
>> +    then
>> +      # Non-existent event.
>> +      echo "Error: Bad event name"
>> +      echo "$output"
>> +      err=1
>> +      continue
>> +    fi
>>    fi
>> -  if echo "$result" | grep -q "Access to performance monitoring and observability operations is limited."
>> +
>> +  if echo "$output" | grep -q "Access to performance monitoring and observability operations is limited."
>>    then
>>      # Access is limited, so ignore.
>> +    echo "access limited"
>>      continue
>>    fi
>>  
>>    # We failed to see the event and it is supported. Possibly the workload was
>>    # too small so retry with something longer.
>> -  result=$(perf stat -e "$p" perf bench internals synthesize 2>&1)
>> -  if echo "$result" | grep -q "$p"
>> +  output=$(perf stat -e "$p" perf bench internals synthesize 2>&1)
>> +  if echo "$output" | grep -q "$p"
>>    then
>>      # Event seen in output.
>> +    echo "supported"
>>      continue
>>    fi
>>    echo "Error: event '$p' not printed in:"
>> -  echo "$result"
>> +  echo "$output"
>>    err=1
>>  done
>>  

Re: [PATCH] perf test stat_all_pmu.sh: Correctly check 'perf stat' result

[Namhyung Kim]

Hello,

On Thu, Jan 16, 2025 at 01:31:21PM +0100, Veronika Molnarova wrote:
> 
> 
> On 12/10/24 16:20, Veronika Molnarova wrote:
> > 
> > 
> > On 11/23/24 00:12, vmolnaro@redhat.com wrote:
> >> From: Veronika Molnarova <vmolnaro@redhat.com>
> >>
> >> Test case "stat_all_pmu.sh" is not correctly checking 'perf stat' output
> >> due to a poor design. Firstly, having the 'set -e' option with a trap
> >> catching the sigexit causes the shell to exit immediately if 'perf stat' ends
> >> with any non-zero value, which is then caught by the trap reporting an
> >> unexpected signal. This causes events that should be parsed by the if-else
> >> statement to be caught by the trap handler and are reported as errors:
> >>
> >>     $ perf test -vv "perf all pmu"
> >>     Testing i915/actual-frequency/
> >>     Unexpected signal in main
> >>     Error:
> >>     Access to performance monitoring and observability operations is limited.
> >>
> >> Secondly, the if-else branches are not exclusive as the checking if the
> >> event is present in the output log covers also the "<not supported>"
> >> events, which should be accepted, and also the "Bad name events", which
> >> should be rejected.
> >>
> >> Remove the "set -e" option from the test case, correctly parse the
> >> "perf stat" output log and check its return value. Add the missing
> >> outputs for the 'perf stat' result and also add logs messages to
> >> report the branch that parsed the event for more info.
> > 
> > Hi Ian,
> > 
> > is there anything that I am missing? If so would be great to know the idea
> > of your patch. This issue is getting quite old so would be great to get a proper
> > fix together.
> > 
> > Thanks,
> > Veronika
> 
> Hello,
> 
> just wanted to ping the patch. Would be great to sort it out and close the issue.

Ian, are you ok with this?

Thanks,
Namhyung
 
> >>
> >> Fixes: 7e73ea40295620e7 ("perf test: Ignore security failures in all PMU test")
> >> Signed-off-by: Veronika Molnarova <vmolnaro@redhat.com>
> >> ---
> >>  tools/perf/tests/shell/stat_all_pmu.sh | 48 ++++++++++++++++++--------
> >>  1 file changed, 34 insertions(+), 14 deletions(-)
> >>
> >> diff --git a/tools/perf/tests/shell/stat_all_pmu.sh b/tools/perf/tests/shell/stat_all_pmu.sh
> >> index 8b148b300be1131e..9c466c0efa857fc2 100755
> >> --- a/tools/perf/tests/shell/stat_all_pmu.sh
> >> +++ b/tools/perf/tests/shell/stat_all_pmu.sh
> >> @@ -2,7 +2,6 @@
> >>  # perf all PMU test (exclusive)
> >>  # SPDX-License-Identifier: GPL-2.0
> >>  
> >> -set -e
> >>  err=0
> >>  result=""
> >>  
> >> @@ -16,34 +15,55 @@ trap trap_cleanup EXIT TERM INT
> >>  # Test all PMU events; however exclude parameterized ones (name contains '?')
> >>  for p in $(perf list --raw-dump pmu | sed 's/[[:graph:]]\+?[[:graph:]]\+[[:space:]]//g')
> >>  do
> >> -  echo "Testing $p"
> >> -  result=$(perf stat -e "$p" true 2>&1)
> >> -  if echo "$result" | grep -q "$p"
> >> +  echo -n "Testing $p -- "
> >> +  output=$(perf stat -e "$p" true 2>&1)
> >> +  stat_result=$?
> >> +  if echo "$output" | grep -q "$p"
> >>    then
> >>      # Event seen in output.
> >> -    continue
> >> -  fi
> >> -  if echo "$result" | grep -q "<not supported>"
> >> -  then
> >> -    # Event not supported, so ignore.
> >> -    continue
> >> +    if [ $stat_result -eq 0 ] && ! echo "$output" | grep -q "<not supported>"
> >> +    then
> >> +      # Event supported.
> >> +      echo "supported"
> >> +      continue
> >> +    elif echo "$output" | grep -q "<not supported>"
> >> +    then
> >> +      # Event not supported, so ignore.
> >> +      echo "not supported"
> >> +      continue
> >> +    elif echo "$output" | grep -q "No permission to enable"
> >> +    then
> >> +      # No permissions, so ignore.
> >> +      echo "no permission to enable"
> >> +      continue
> >> +    elif echo "$output" | grep -q "Bad event name"
> >> +    then
> >> +      # Non-existent event.
> >> +      echo "Error: Bad event name"
> >> +      echo "$output"
> >> +      err=1
> >> +      continue
> >> +    fi
> >>    fi
> >> -  if echo "$result" | grep -q "Access to performance monitoring and observability operations is limited."
> >> +
> >> +  if echo "$output" | grep -q "Access to performance monitoring and observability operations is limited."
> >>    then
> >>      # Access is limited, so ignore.
> >> +    echo "access limited"
> >>      continue
> >>    fi
> >>  
> >>    # We failed to see the event and it is supported. Possibly the workload was
> >>    # too small so retry with something longer.
> >> -  result=$(perf stat -e "$p" perf bench internals synthesize 2>&1)
> >> -  if echo "$result" | grep -q "$p"
> >> +  output=$(perf stat -e "$p" perf bench internals synthesize 2>&1)
> >> +  if echo "$output" | grep -q "$p"
> >>    then
> >>      # Event seen in output.
> >> +    echo "supported"
> >>      continue
> >>    fi
> >>    echo "Error: event '$p' not printed in:"
> >> -  echo "$result"
> >> +  echo "$output"
> >>    err=1
> >>  done
> >>  
> 

Re: [PATCH] perf test stat_all_pmu.sh: Correctly check 'perf stat' result

[Qiao Zhao]

On Sun, Jan 19, 2025 at 2:15 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Hello,
>
> On Thu, Jan 16, 2025 at 01:31:21PM +0100, Veronika Molnarova wrote:
> >
> >
> > On 12/10/24 16:20, Veronika Molnarova wrote:
> > >
> > >
> > > On 11/23/24 00:12, vmolnaro@redhat.com wrote:
> > >> From: Veronika Molnarova <vmolnaro@redhat.com>
> > >>
> > >> Test case "stat_all_pmu.sh" is not correctly checking 'perf stat' output
> > >> due to a poor design. Firstly, having the 'set -e' option with a trap
> > >> catching the sigexit causes the shell to exit immediately if 'perf stat' ends
> > >> with any non-zero value, which is then caught by the trap reporting an
> > >> unexpected signal. This causes events that should be parsed by the if-else
> > >> statement to be caught by the trap handler and are reported as errors:
> > >>
> > >>     $ perf test -vv "perf all pmu"
> > >>     Testing i915/actual-frequency/
> > >>     Unexpected signal in main
> > >>     Error:
> > >>     Access to performance monitoring and observability operations is limited.
> > >>
> > >> Secondly, the if-else branches are not exclusive as the checking if the
> > >> event is present in the output log covers also the "<not supported>"
> > >> events, which should be accepted, and also the "Bad name events", which
> > >> should be rejected.
> > >>
> > >> Remove the "set -e" option from the test case, correctly parse the
> > >> "perf stat" output log and check its return value. Add the missing
> > >> outputs for the 'perf stat' result and also add logs messages to
> > >> report the branch that parsed the event for more info.
> > >
> > > Hi Ian,
> > >
> > > is there anything that I am missing? If so would be great to know the idea
> > > of your patch. This issue is getting quite old so would be great to get a proper
> > > fix together.
> > >
> > > Thanks,
> > > Veronika
> >
> > Hello,
> >
> > just wanted to ping the patch. Would be great to sort it out and close the issue.
>
> Ian, are you ok with this?
>

Just ping again for this patch, can this fix be acked?

And I already tested this patch and it works well on the ppc64le system.

Tested-by: Qiao Zhao <qzhao@redhat.com>

- Qiao

> Thanks,
> Namhyung
>
> > >>
> > >> Fixes: 7e73ea40295620e7 ("perf test: Ignore security failures in all PMU test")
> > >> Signed-off-by: Veronika Molnarova <vmolnaro@redhat.com>
> > >> ---
> > >>  tools/perf/tests/shell/stat_all_pmu.sh | 48 ++++++++++++++++++--------
> > >>  1 file changed, 34 insertions(+), 14 deletions(-)
> > >>
> > >> diff --git a/tools/perf/tests/shell/stat_all_pmu.sh b/tools/perf/tests/shell/stat_all_pmu.sh
> > >> index 8b148b300be1131e..9c466c0efa857fc2 100755
> > >> --- a/tools/perf/tests/shell/stat_all_pmu.sh
> > >> +++ b/tools/perf/tests/shell/stat_all_pmu.sh
> > >> @@ -2,7 +2,6 @@
> > >>  # perf all PMU test (exclusive)
> > >>  # SPDX-License-Identifier: GPL-2.0
> > >>
> > >> -set -e
> > >>  err=0
> > >>  result=""
> > >>
> > >> @@ -16,34 +15,55 @@ trap trap_cleanup EXIT TERM INT
> > >>  # Test all PMU events; however exclude parameterized ones (name contains '?')
> > >>  for p in $(perf list --raw-dump pmu | sed 's/[[:graph:]]\+?[[:graph:]]\+[[:space:]]//g')
> > >>  do
> > >> -  echo "Testing $p"
> > >> -  result=$(perf stat -e "$p" true 2>&1)
> > >> -  if echo "$result" | grep -q "$p"
> > >> +  echo -n "Testing $p -- "
> > >> +  output=$(perf stat -e "$p" true 2>&1)
> > >> +  stat_result=$?
> > >> +  if echo "$output" | grep -q "$p"
> > >>    then
> > >>      # Event seen in output.
> > >> -    continue
> > >> -  fi
> > >> -  if echo "$result" | grep -q "<not supported>"
> > >> -  then
> > >> -    # Event not supported, so ignore.
> > >> -    continue
> > >> +    if [ $stat_result -eq 0 ] && ! echo "$output" | grep -q "<not supported>"
> > >> +    then
> > >> +      # Event supported.
> > >> +      echo "supported"
> > >> +      continue
> > >> +    elif echo "$output" | grep -q "<not supported>"
> > >> +    then
> > >> +      # Event not supported, so ignore.
> > >> +      echo "not supported"
> > >> +      continue
> > >> +    elif echo "$output" | grep -q "No permission to enable"
> > >> +    then
> > >> +      # No permissions, so ignore.
> > >> +      echo "no permission to enable"
> > >> +      continue
> > >> +    elif echo "$output" | grep -q "Bad event name"
> > >> +    then
> > >> +      # Non-existent event.
> > >> +      echo "Error: Bad event name"
> > >> +      echo "$output"
> > >> +      err=1
> > >> +      continue
> > >> +    fi
> > >>    fi
> > >> -  if echo "$result" | grep -q "Access to performance monitoring and observability operations is limited."
> > >> +
> > >> +  if echo "$output" | grep -q "Access to performance monitoring and observability operations is limited."
> > >>    then
> > >>      # Access is limited, so ignore.
> > >> +    echo "access limited"
> > >>      continue
> > >>    fi
> > >>
> > >>    # We failed to see the event and it is supported. Possibly the workload was
> > >>    # too small so retry with something longer.
> > >> -  result=$(perf stat -e "$p" perf bench internals synthesize 2>&1)
> > >> -  if echo "$result" | grep -q "$p"
> > >> +  output=$(perf stat -e "$p" perf bench internals synthesize 2>&1)
> > >> +  if echo "$output" | grep -q "$p"
> > >>    then
> > >>      # Event seen in output.
> > >> +    echo "supported"
> > >>      continue
> > >>    fi
> > >>    echo "Error: event '$p' not printed in:"
> > >> -  echo "$result"
> > >> +  echo "$output"
> > >>    err=1
> > >>  done
> > >>
> >
>

Re: [PATCH] perf test stat_all_pmu.sh: Correctly check 'perf stat' result

[Qiao Zhao]

On Mon, Feb 24, 2025 at 2:59 PM Qiao Zhao <qzhao@redhat.com> wrote:
>
> On Sun, Jan 19, 2025 at 2:15 AM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > Hello,
> >
> > On Thu, Jan 16, 2025 at 01:31:21PM +0100, Veronika Molnarova wrote:
> > >
> > >
> > > On 12/10/24 16:20, Veronika Molnarova wrote:
> > > >
> > > >
> > > > On 11/23/24 00:12, vmolnaro@redhat.com wrote:
> > > >> From: Veronika Molnarova <vmolnaro@redhat.com>
> > > >>
> > > >> Test case "stat_all_pmu.sh" is not correctly checking 'perf stat' output
> > > >> due to a poor design. Firstly, having the 'set -e' option with a trap
> > > >> catching the sigexit causes the shell to exit immediately if 'perf stat' ends
> > > >> with any non-zero value, which is then caught by the trap reporting an
> > > >> unexpected signal. This causes events that should be parsed by the if-else
> > > >> statement to be caught by the trap handler and are reported as errors:
> > > >>
> > > >>     $ perf test -vv "perf all pmu"
> > > >>     Testing i915/actual-frequency/
> > > >>     Unexpected signal in main
> > > >>     Error:
> > > >>     Access to performance monitoring and observability operations is limited.
> > > >>
> > > >> Secondly, the if-else branches are not exclusive as the checking if the
> > > >> event is present in the output log covers also the "<not supported>"
> > > >> events, which should be accepted, and also the "Bad name events", which
> > > >> should be rejected.
> > > >>
> > > >> Remove the "set -e" option from the test case, correctly parse the
> > > >> "perf stat" output log and check its return value. Add the missing
> > > >> outputs for the 'perf stat' result and also add logs messages to
> > > >> report the branch that parsed the event for more info.
> > > >
> > > > Hi Ian,
> > > >
> > > > is there anything that I am missing? If so would be great to know the idea
> > > > of your patch. This issue is getting quite old so would be great to get a proper
> > > > fix together.
> > > >
> > > > Thanks,
> > > > Veronika
> > >
> > > Hello,
> > >
> > > just wanted to ping the patch. Would be great to sort it out and close the issue.
> >
> > Ian, are you ok with this?
> >
>
> Just ping again for this patch, can this fix be acked?
>
> And I already tested this patch and it works well on the ppc64le system.
>
> Tested-by: Qiao Zhao <qzhao@redhat.com>

Revert the Tested-by, and when more tests on Intel EMR system, and show other
"Bad event name" and " value too big for format (umask)" errors.
This patch needs more tests and rewrites.

- Qiao

>
> - Qiao
>
> > Thanks,
> > Namhyung
> >
> > > >>
> > > >> Fixes: 7e73ea40295620e7 ("perf test: Ignore security failures in all PMU test")
> > > >> Signed-off-by: Veronika Molnarova <vmolnaro@redhat.com>
> > > >> ---
> > > >>  tools/perf/tests/shell/stat_all_pmu.sh | 48 ++++++++++++++++++--------
> > > >>  1 file changed, 34 insertions(+), 14 deletions(-)
> > > >>
> > > >> diff --git a/tools/perf/tests/shell/stat_all_pmu.sh b/tools/perf/tests/shell/stat_all_pmu.sh
> > > >> index 8b148b300be1131e..9c466c0efa857fc2 100755
> > > >> --- a/tools/perf/tests/shell/stat_all_pmu.sh
> > > >> +++ b/tools/perf/tests/shell/stat_all_pmu.sh
> > > >> @@ -2,7 +2,6 @@
> > > >>  # perf all PMU test (exclusive)
> > > >>  # SPDX-License-Identifier: GPL-2.0
> > > >>
> > > >> -set -e
> > > >>  err=0
> > > >>  result=""
> > > >>
> > > >> @@ -16,34 +15,55 @@ trap trap_cleanup EXIT TERM INT
> > > >>  # Test all PMU events; however exclude parameterized ones (name contains '?')
> > > >>  for p in $(perf list --raw-dump pmu | sed 's/[[:graph:]]\+?[[:graph:]]\+[[:space:]]//g')
> > > >>  do
> > > >> -  echo "Testing $p"
> > > >> -  result=$(perf stat -e "$p" true 2>&1)
> > > >> -  if echo "$result" | grep -q "$p"
> > > >> +  echo -n "Testing $p -- "
> > > >> +  output=$(perf stat -e "$p" true 2>&1)
> > > >> +  stat_result=$?
> > > >> +  if echo "$output" | grep -q "$p"
> > > >>    then
> > > >>      # Event seen in output.
> > > >> -    continue
> > > >> -  fi
> > > >> -  if echo "$result" | grep -q "<not supported>"
> > > >> -  then
> > > >> -    # Event not supported, so ignore.
> > > >> -    continue
> > > >> +    if [ $stat_result -eq 0 ] && ! echo "$output" | grep -q "<not supported>"
> > > >> +    then
> > > >> +      # Event supported.
> > > >> +      echo "supported"
> > > >> +      continue
> > > >> +    elif echo "$output" | grep -q "<not supported>"
> > > >> +    then
> > > >> +      # Event not supported, so ignore.
> > > >> +      echo "not supported"
> > > >> +      continue
> > > >> +    elif echo "$output" | grep -q "No permission to enable"
> > > >> +    then
> > > >> +      # No permissions, so ignore.
> > > >> +      echo "no permission to enable"
> > > >> +      continue
> > > >> +    elif echo "$output" | grep -q "Bad event name"
> > > >> +    then
> > > >> +      # Non-existent event.
> > > >> +      echo "Error: Bad event name"
> > > >> +      echo "$output"
> > > >> +      err=1
> > > >> +      continue
> > > >> +    fi
> > > >>    fi
> > > >> -  if echo "$result" | grep -q "Access to performance monitoring and observability operations is limited."
> > > >> +
> > > >> +  if echo "$output" | grep -q "Access to performance monitoring and observability operations is limited."
> > > >>    then
> > > >>      # Access is limited, so ignore.
> > > >> +    echo "access limited"
> > > >>      continue
> > > >>    fi
> > > >>
> > > >>    # We failed to see the event and it is supported. Possibly the workload was
> > > >>    # too small so retry with something longer.
> > > >> -  result=$(perf stat -e "$p" perf bench internals synthesize 2>&1)
> > > >> -  if echo "$result" | grep -q "$p"
> > > >> +  output=$(perf stat -e "$p" perf bench internals synthesize 2>&1)
> > > >> +  if echo "$output" | grep -q "$p"
> > > >>    then
> > > >>      # Event seen in output.
> > > >> +    echo "supported"
> > > >>      continue
> > > >>    fi
> > > >>    echo "Error: event '$p' not printed in:"
> > > >> -  echo "$result"
> > > >> +  echo "$output"
> > > >>    err=1
> > > >>  done
> > > >>
> > >
> >

Re: [PATCH] perf test stat_all_pmu.sh: Correctly check 'perf stat' result

[Namhyung Kim]

Hello,

On Mon, Feb 24, 2025 at 05:34:15PM +0800, Qiao Zhao wrote:
> On Mon, Feb 24, 2025 at 2:59 PM Qiao Zhao <qzhao@redhat.com> wrote:
> >
> > On Sun, Jan 19, 2025 at 2:15 AM Namhyung Kim <namhyung@kernel.org> wrote:
> > >
> > > Hello,
> > >
> > > On Thu, Jan 16, 2025 at 01:31:21PM +0100, Veronika Molnarova wrote:
> > > >
> > > >
> > > > On 12/10/24 16:20, Veronika Molnarova wrote:
> > > > >
> > > > >
> > > > > On 11/23/24 00:12, vmolnaro@redhat.com wrote:
> > > > >> From: Veronika Molnarova <vmolnaro@redhat.com>
> > > > >>
> > > > >> Test case "stat_all_pmu.sh" is not correctly checking 'perf stat' output
> > > > >> due to a poor design. Firstly, having the 'set -e' option with a trap
> > > > >> catching the sigexit causes the shell to exit immediately if 'perf stat' ends
> > > > >> with any non-zero value, which is then caught by the trap reporting an
> > > > >> unexpected signal. This causes events that should be parsed by the if-else
> > > > >> statement to be caught by the trap handler and are reported as errors:
> > > > >>
> > > > >>     $ perf test -vv "perf all pmu"
> > > > >>     Testing i915/actual-frequency/
> > > > >>     Unexpected signal in main
> > > > >>     Error:
> > > > >>     Access to performance monitoring and observability operations is limited.
> > > > >>
> > > > >> Secondly, the if-else branches are not exclusive as the checking if the
> > > > >> event is present in the output log covers also the "<not supported>"
> > > > >> events, which should be accepted, and also the "Bad name events", which
> > > > >> should be rejected.
> > > > >>
> > > > >> Remove the "set -e" option from the test case, correctly parse the
> > > > >> "perf stat" output log and check its return value. Add the missing
> > > > >> outputs for the 'perf stat' result and also add logs messages to
> > > > >> report the branch that parsed the event for more info.
> > > > >
> > > > > Hi Ian,
> > > > >
> > > > > is there anything that I am missing? If so would be great to know the idea
> > > > > of your patch. This issue is getting quite old so would be great to get a proper
> > > > > fix together.
> > > > >
> > > > > Thanks,
> > > > > Veronika
> > > >
> > > > Hello,
> > > >
> > > > just wanted to ping the patch. Would be great to sort it out and close the issue.
> > >
> > > Ian, are you ok with this?
> > >
> >
> > Just ping again for this patch, can this fix be acked?
> >
> > And I already tested this patch and it works well on the ppc64le system.
> >
> > Tested-by: Qiao Zhao <qzhao@redhat.com>
> 
> Revert the Tested-by, and when more tests on Intel EMR system, and show other
> "Bad event name" and " value too big for format (umask)" errors.
> This patch needs more tests and rewrites.

Thanks for the test, though!
Namhyung

Re: [PATCH] perf test stat_all_pmu.sh: Correctly check 'perf stat' result

[Qiao Zhao]

Some more tests with Veronika's patch:

Pick one failed event and use '-vv' to show more debug info like this:
>>>>>>>>>>>
# ./perf stat -vv -e 'unc_iio_req_from_pcie_pass_cmpl.wr' true
Using CPUID GenuineIntel-6-CF-2
Attempt to add: uncore_iio_0/unc_iio_req_from_pcie_pass_cmpl.wr/
..after resolving event:
uncore_iio_0/event=0x90,ch_mask=0xff,fc_mask=0x7,umask=0x70ff010/
Attempt to add: uncore_iio_2/unc_iio_req_from_pcie_pass_cmpl.wr/
..after resolving event:
uncore_iio_2/event=0x90,ch_mask=0xff,fc_mask=0x7,umask=0x70ff010/
Attempt to add: uncore_iio_6/unc_iio_req_from_pcie_pass_cmpl.wr/
..after resolving event:
uncore_iio_6/event=0x90,ch_mask=0xff,fc_mask=0x7,umask=0x70ff010/
Attempt to add: uncore_iio_8/unc_iio_req_from_pcie_pass_cmpl.wr/
..after resolving event:
uncore_iio_8/event=0x90,ch_mask=0xff,fc_mask=0x7,umask=0x70ff010/
Attempt to add: uncore_iio_1/unc_iio_req_from_pcie_pass_cmpl.wr/
..after resolving event:
uncore_iio_1/event=0x90,ch_mask=0xff,fc_mask=0x7,umask=0x70ff010/
Attempt to add: uncore_iio_3/unc_iio_req_from_pcie_pass_cmpl.wr/
..after resolving event:
uncore_iio_3/event=0x90,ch_mask=0xff,fc_mask=0x7,umask=0x70ff010/
Attempt to add: uncore_iio_7/unc_iio_req_from_pcie_pass_cmpl.wr/
..after resolving event:
uncore_iio_7/event=0x90,ch_mask=0xff,fc_mask=0x7,umask=0x70ff010/
event syntax error: 'unc_iio_req_from_pcie_pass_cmpl.wr'
                     \___ Bad event name

Unable to find event on a PMU of 'unc_iio_req_from_pcie_pass_cmpl.wr'

event syntax error: 'unc_iio_req_from_pcie_pass_cmpl.wr'
                     \___ value too big for format (umask), maximum is 255

event syntax error: 'unc_iio_req_from_pcie_pass_cmpl.wr'
                     \___ value too big for format (umask), maximum is 255

event syntax error: 'unc_iio_req_from_pcie_pass_cmpl.wr'
                     \___ value too big for format (umask), maximum is 255

event syntax error: 'unc_iio_req_from_pcie_pass_cmpl.wr'
                     \___ value too big for format (umask), maximum is 255

event syntax error: 'unc_iio_req_from_pcie_pass_cmpl.wr'
                     \___ value too big for format (umask), maximum is 255

event syntax error: 'unc_iio_req_from_pcie_pass_cmpl.wr'
                     \___ value too big for format (umask), maximum is 255

event syntax error: 'unc_iio_req_from_pcie_pass_cmpl.wr'
                     \___ value too big for format (umask), maximum is 255
Run 'perf list' for a list of valid events

 Usage: perf stat [<options>] [<command>]

    -e, --event <event>   event selector. use 'perf list' to list
available events
>>>>>>>>>>
And the event umask shows "umask=0x70ff010", this value is higher than
maximum value "255".

After talking with Michael, this "Bad event name" issue should be
fixed by Ian Rogers's patch
https://lore.kernel.org/lkml/20250211213031.114209-20-irogers@google.com/#r
  perf vendor events: Update Sapphirerapids events/metrics

Will try to test Veronika's patch again after Roger's patch merges.

- Qiao




- Qiao

On Tue, Feb 25, 2025 at 7:41 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Hello,
>
> On Mon, Feb 24, 2025 at 05:34:15PM +0800, Qiao Zhao wrote:
> > On Mon, Feb 24, 2025 at 2:59 PM Qiao Zhao <qzhao@redhat.com> wrote:
> > >
> > > On Sun, Jan 19, 2025 at 2:15 AM Namhyung Kim <namhyung@kernel.org> wrote:
> > > >
> > > > Hello,
> > > >
> > > > On Thu, Jan 16, 2025 at 01:31:21PM +0100, Veronika Molnarova wrote:
> > > > >
> > > > >
> > > > > On 12/10/24 16:20, Veronika Molnarova wrote:
> > > > > >
> > > > > >
> > > > > > On 11/23/24 00:12, vmolnaro@redhat.com wrote:
> > > > > >> From: Veronika Molnarova <vmolnaro@redhat.com>
> > > > > >>
> > > > > >> Test case "stat_all_pmu.sh" is not correctly checking 'perf stat' output
> > > > > >> due to a poor design. Firstly, having the 'set -e' option with a trap
> > > > > >> catching the sigexit causes the shell to exit immediately if 'perf stat' ends
> > > > > >> with any non-zero value, which is then caught by the trap reporting an
> > > > > >> unexpected signal. This causes events that should be parsed by the if-else
> > > > > >> statement to be caught by the trap handler and are reported as errors:
> > > > > >>
> > > > > >>     $ perf test -vv "perf all pmu"
> > > > > >>     Testing i915/actual-frequency/
> > > > > >>     Unexpected signal in main
> > > > > >>     Error:
> > > > > >>     Access to performance monitoring and observability operations is limited.
> > > > > >>
> > > > > >> Secondly, the if-else branches are not exclusive as the checking if the
> > > > > >> event is present in the output log covers also the "<not supported>"
> > > > > >> events, which should be accepted, and also the "Bad name events", which
> > > > > >> should be rejected.
> > > > > >>
> > > > > >> Remove the "set -e" option from the test case, correctly parse the
> > > > > >> "perf stat" output log and check its return value. Add the missing
> > > > > >> outputs for the 'perf stat' result and also add logs messages to
> > > > > >> report the branch that parsed the event for more info.
> > > > > >
> > > > > > Hi Ian,
> > > > > >
> > > > > > is there anything that I am missing? If so would be great to know the idea
> > > > > > of your patch. This issue is getting quite old so would be great to get a proper
> > > > > > fix together.
> > > > > >
> > > > > > Thanks,
> > > > > > Veronika
> > > > >
> > > > > Hello,
> > > > >
> > > > > just wanted to ping the patch. Would be great to sort it out and close the issue.
> > > >
> > > > Ian, are you ok with this?
> > > >
> > >
> > > Just ping again for this patch, can this fix be acked?
> > >
> > > And I already tested this patch and it works well on the ppc64le system.
> > >
> > > Tested-by: Qiao Zhao <qzhao@redhat.com>
> >
> > Revert the Tested-by, and when more tests on Intel EMR system, and show other
> > "Bad event name" and " value too big for format (umask)" errors.
> > This patch needs more tests and rewrites.
>
> Thanks for the test, though!
> Namhyung
>
>

Re: [PATCH] perf test stat_all_pmu.sh: Correctly check 'perf stat' result

[Qiao Zhao]

Hi Namhyung,

Sorry for causing some trouble and confusion.
Veronika's patch fixed the "check 'perf stat' result" issue, and the
test passed.
About the "Bad event name" issue, it is another EMR/SPR uncore issue,
not caused by Veronike's patch.
So if no other is missing, can we merge this issue?

Tested-by: Qiao Zhao <qzhao@redhat.com>

- Qiao

On Mon, Mar 3, 2025 at 2:14 PM Qiao Zhao <qzhao@redhat.com> wrote:
>
> Some more tests with Veronika's patch:
>
> Pick one failed event and use '-vv' to show more debug info like this:
> >>>>>>>>>>>
> # ./perf stat -vv -e 'unc_iio_req_from_pcie_pass_cmpl.wr' true
> Using CPUID GenuineIntel-6-CF-2
> Attempt to add: uncore_iio_0/unc_iio_req_from_pcie_pass_cmpl.wr/
> ..after resolving event:
> uncore_iio_0/event=0x90,ch_mask=0xff,fc_mask=0x7,umask=0x70ff010/
> Attempt to add: uncore_iio_2/unc_iio_req_from_pcie_pass_cmpl.wr/
> ..after resolving event:
> uncore_iio_2/event=0x90,ch_mask=0xff,fc_mask=0x7,umask=0x70ff010/
> Attempt to add: uncore_iio_6/unc_iio_req_from_pcie_pass_cmpl.wr/
> ..after resolving event:
> uncore_iio_6/event=0x90,ch_mask=0xff,fc_mask=0x7,umask=0x70ff010/
> Attempt to add: uncore_iio_8/unc_iio_req_from_pcie_pass_cmpl.wr/
> ..after resolving event:
> uncore_iio_8/event=0x90,ch_mask=0xff,fc_mask=0x7,umask=0x70ff010/
> Attempt to add: uncore_iio_1/unc_iio_req_from_pcie_pass_cmpl.wr/
> ..after resolving event:
> uncore_iio_1/event=0x90,ch_mask=0xff,fc_mask=0x7,umask=0x70ff010/
> Attempt to add: uncore_iio_3/unc_iio_req_from_pcie_pass_cmpl.wr/
> ..after resolving event:
> uncore_iio_3/event=0x90,ch_mask=0xff,fc_mask=0x7,umask=0x70ff010/
> Attempt to add: uncore_iio_7/unc_iio_req_from_pcie_pass_cmpl.wr/
> ..after resolving event:
> uncore_iio_7/event=0x90,ch_mask=0xff,fc_mask=0x7,umask=0x70ff010/
> event syntax error: 'unc_iio_req_from_pcie_pass_cmpl.wr'
>                      \___ Bad event name
>
> Unable to find event on a PMU of 'unc_iio_req_from_pcie_pass_cmpl.wr'
>
> event syntax error: 'unc_iio_req_from_pcie_pass_cmpl.wr'
>                      \___ value too big for format (umask), maximum is 255
>
> event syntax error: 'unc_iio_req_from_pcie_pass_cmpl.wr'
>                      \___ value too big for format (umask), maximum is 255
>
> event syntax error: 'unc_iio_req_from_pcie_pass_cmpl.wr'
>                      \___ value too big for format (umask), maximum is 255
>
> event syntax error: 'unc_iio_req_from_pcie_pass_cmpl.wr'
>                      \___ value too big for format (umask), maximum is 255
>
> event syntax error: 'unc_iio_req_from_pcie_pass_cmpl.wr'
>                      \___ value too big for format (umask), maximum is 255
>
> event syntax error: 'unc_iio_req_from_pcie_pass_cmpl.wr'
>                      \___ value too big for format (umask), maximum is 255
>
> event syntax error: 'unc_iio_req_from_pcie_pass_cmpl.wr'
>                      \___ value too big for format (umask), maximum is 255
> Run 'perf list' for a list of valid events
>
>  Usage: perf stat [<options>] [<command>]
>
>     -e, --event <event>   event selector. use 'perf list' to list
> available events
> >>>>>>>>>>
> And the event umask shows "umask=0x70ff010", this value is higher than
> maximum value "255".
>
> After talking with Michael, this "Bad event name" issue should be
> fixed by Ian Rogers's patch
> https://lore.kernel.org/lkml/20250211213031.114209-20-irogers@google.com/#r
>   perf vendor events: Update Sapphirerapids events/metrics
>
> Will try to test Veronika's patch again after Roger's patch merges.
>
> - Qiao
>
>
>
>
> - Qiao
>
> On Tue, Feb 25, 2025 at 7:41 AM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > Hello,
> >
> > On Mon, Feb 24, 2025 at 05:34:15PM +0800, Qiao Zhao wrote:
> > > On Mon, Feb 24, 2025 at 2:59 PM Qiao Zhao <qzhao@redhat.com> wrote:
> > > >
> > > > On Sun, Jan 19, 2025 at 2:15 AM Namhyung Kim <namhyung@kernel.org> wrote:
> > > > >
> > > > > Hello,
> > > > >
> > > > > On Thu, Jan 16, 2025 at 01:31:21PM +0100, Veronika Molnarova wrote:
> > > > > >
> > > > > >
> > > > > > On 12/10/24 16:20, Veronika Molnarova wrote:
> > > > > > >
> > > > > > >
> > > > > > > On 11/23/24 00:12, vmolnaro@redhat.com wrote:
> > > > > > >> From: Veronika Molnarova <vmolnaro@redhat.com>
> > > > > > >>
> > > > > > >> Test case "stat_all_pmu.sh" is not correctly checking 'perf stat' output
> > > > > > >> due to a poor design. Firstly, having the 'set -e' option with a trap
> > > > > > >> catching the sigexit causes the shell to exit immediately if 'perf stat' ends
> > > > > > >> with any non-zero value, which is then caught by the trap reporting an
> > > > > > >> unexpected signal. This causes events that should be parsed by the if-else
> > > > > > >> statement to be caught by the trap handler and are reported as errors:
> > > > > > >>
> > > > > > >>     $ perf test -vv "perf all pmu"
> > > > > > >>     Testing i915/actual-frequency/
> > > > > > >>     Unexpected signal in main
> > > > > > >>     Error:
> > > > > > >>     Access to performance monitoring and observability operations is limited.
> > > > > > >>
> > > > > > >> Secondly, the if-else branches are not exclusive as the checking if the
> > > > > > >> event is present in the output log covers also the "<not supported>"
> > > > > > >> events, which should be accepted, and also the "Bad name events", which
> > > > > > >> should be rejected.
> > > > > > >>
> > > > > > >> Remove the "set -e" option from the test case, correctly parse the
> > > > > > >> "perf stat" output log and check its return value. Add the missing
> > > > > > >> outputs for the 'perf stat' result and also add logs messages to
> > > > > > >> report the branch that parsed the event for more info.
> > > > > > >
> > > > > > > Hi Ian,
> > > > > > >
> > > > > > > is there anything that I am missing? If so would be great to know the idea
> > > > > > > of your patch. This issue is getting quite old so would be great to get a proper
> > > > > > > fix together.
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Veronika
> > > > > >
> > > > > > Hello,
> > > > > >
> > > > > > just wanted to ping the patch. Would be great to sort it out and close the issue.
> > > > >
> > > > > Ian, are you ok with this?
> > > > >
> > > >
> > > > Just ping again for this patch, can this fix be acked?
> > > >
> > > > And I already tested this patch and it works well on the ppc64le system.
> > > >
> > > > Tested-by: Qiao Zhao <qzhao@redhat.com>
> > >
> > > Revert the Tested-by, and when more tests on Intel EMR system, and show other
> > > "Bad event name" and " value too big for format (umask)" errors.
> > > This patch needs more tests and rewrites.
> >
> > Thanks for the test, though!
> > Namhyung
> >
> >

Re: [PATCH] perf test stat_all_pmu.sh: Correctly check 'perf stat' result

[Namhyung Kim]

On Sat, 23 Nov 2024 00:12:33 +0100, vmolnaro@redhat.com wrote:
> Test case "stat_all_pmu.sh" is not correctly checking 'perf stat' output
> due to a poor design. Firstly, having the 'set -e' option with a trap
> catching the sigexit causes the shell to exit immediately if 'perf stat' ends
> with any non-zero value, which is then caught by the trap reporting an
> unexpected signal. This causes events that should be parsed by the if-else
> statement to be caught by the trap handler and are reported as errors:
> 
> [...]
Applied to perf-tools-next, thanks!

Best regards,
Namhyung