[PATCH 1/2 v2] perf test: Fix perf test 114 perf record test subtest precise_max for s390

[PATCH 1/2 v2] perf test: Fix perf test 114 perf record test subtest precise_max for s390

[Thomas Richter]

On s390 the event instructions can not be used for recording.
This event is only supported by perf stat.

Change the event instruction to cycles for subtest test_precise_max
as suggested by James Clark.

Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
Suggested-by: James Clark <james.clark@linaro.org>
---
 tools/perf/tests/shell/record.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
index 0fc7a909ae9b..957ee10aff44 100755
--- a/tools/perf/tests/shell/record.sh
+++ b/tools/perf/tests/shell/record.sh
@@ -287,9 +287,9 @@ test_precise_max() {
     return
   fi
   # On AMD, cycles and instructions events are treated differently
-  if ! perf record -o "${perfdata}" -e "instructions:P" true 2> /dev/null
+  if ! perf record -o "${perfdata}" -e "cycles:P" true 2> /dev/null
   then
-    echo "precise_max attribute [Failed instructions:P event]"
+    echo "precise_max attribute [Failed cycles:P event]"
     err=1
     return
   fi
-- 
2.48.1

[PATCH 2/2 v2] perf test: Skip perf test 114 perf record test subtest test_leader_sampling for s390

[Thomas Richter]

On s390 the event instructions can not be used for recording.
This event is only supported by perf stat.

Omit subtest test_leader_sampling on s390.

Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
---
 tools/perf/tests/shell/record.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
index 957ee10aff44..9e645eb09d9e 100755
--- a/tools/perf/tests/shell/record.sh
+++ b/tools/perf/tests/shell/record.sh
@@ -307,7 +307,8 @@ test_system_wide
 test_workload
 test_branch_counter
 test_cgroup
-test_leader_sampling
+# On s390 event instructions can not be used for recording
+[ $(uname -m) != s390x ] && test_leader_sampling
 test_topdown_leader_sampling
 test_precise_max
 
-- 
2.48.1

Re: [PATCH 2/2 v2] perf test: Skip perf test 114 perf record test subtest test_leader_sampling for s390

[James Clark]

On 29/01/2025 7:11 am, Thomas Richter wrote:
> On s390 the event instructions can not be used for recording.
> This event is only supported by perf stat.
> 
> Omit subtest test_leader_sampling on s390.
> 
> Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
> ---
>   tools/perf/tests/shell/record.sh | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
> index 957ee10aff44..9e645eb09d9e 100755
> --- a/tools/perf/tests/shell/record.sh
> +++ b/tools/perf/tests/shell/record.sh
> @@ -307,7 +307,8 @@ test_system_wide
>   test_workload
>   test_branch_counter
>   test_cgroup
> -test_leader_sampling
> +# On s390 event instructions can not be used for recording
> +[ $(uname -m) != s390x ] && test_leader_sampling
>   test_topdown_leader_sampling
>   test_precise_max
>   

I still think we should use the cycles event instead of skipping. My 
intention was to use the most commonly available event with 80fd0c1eac7 
("perf tests: Make leader sampling test work without branch event"), 
it's not important which event is used exactly

Re: [PATCH 1/2 v2] perf test: Fix perf test 114 perf record test subtest precise_max for s390

[James Clark]

Is this a v2 of "[PATCH] perf test: Fix perf test 114 perf record test 
for s390"? Or maybe the v2 is a typo?

On 29/01/2025 7:11 am, Thomas Richter wrote:
> On s390 the event instructions can not be used for recording.
> This event is only supported by perf stat.
> 
> Change the event instruction to cycles for subtest test_precise_max
> as suggested by James Clark.
> 
> Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
> Suggested-by: James Clark <james.clark@linaro.org>
> ---
>   tools/perf/tests/shell/record.sh | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
> index 0fc7a909ae9b..957ee10aff44 100755
> --- a/tools/perf/tests/shell/record.sh
> +++ b/tools/perf/tests/shell/record.sh
> @@ -287,9 +287,9 @@ test_precise_max() {
>       return
>     fi
>     # On AMD, cycles and instructions events are treated differently
> -  if ! perf record -o "${perfdata}" -e "instructions:P" true 2> /dev/null
> +  if ! perf record -o "${perfdata}" -e "cycles:P" true 2> /dev/null

I'm not sure if this is the right fix. Doesn't this make the test test 
cycles:P twice now?

I think the fix is to first test if cycles can be sampled, then test 
cycles:P. Then test if instructions can be sampled, then test 
instructions:P.

At the moment it checks the events with perf stat, and it checks them 
both at the same time. Obviously that doesn't work if an event works for 
perf stat but not perf record. And checking them both at the same time 
doesn't allow for one existing and not the other.

>     then
> -    echo "precise_max attribute [Failed instructions:P event]"
> +    echo "precise_max attribute [Failed cycles:P event]"
>       err=1
>       return
>     fi

Re: [PATCH 1/2 v2] perf test: Fix perf test 114 perf record test subtest precise_max for s390

[James Clark]

On 29/01/2025 9:57 am, James Clark wrote:
> 
> Is this a v2 of "[PATCH] perf test: Fix perf test 114 perf record test 
> for s390"? Or maybe the v2 is a typo?
> 

Nevermind, the CC list is different on the two patches that's why I got 
confused.

> On 29/01/2025 7:11 am, Thomas Richter wrote:
>> On s390 the event instructions can not be used for recording.
>> This event is only supported by perf stat.
>>
>> Change the event instruction to cycles for subtest test_precise_max
>> as suggested by James Clark.
>>
>> Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
>> Suggested-by: James Clark <james.clark@linaro.org>
>> ---
>>   tools/perf/tests/shell/record.sh | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/ 
>> shell/record.sh
>> index 0fc7a909ae9b..957ee10aff44 100755
>> --- a/tools/perf/tests/shell/record.sh
>> +++ b/tools/perf/tests/shell/record.sh
>> @@ -287,9 +287,9 @@ test_precise_max() {
>>       return
>>     fi
>>     # On AMD, cycles and instructions events are treated differently
>> -  if ! perf record -o "${perfdata}" -e "instructions:P" true 2> /dev/ 
>> null
>> +  if ! perf record -o "${perfdata}" -e "cycles:P" true 2> /dev/null
> 
> I'm not sure if this is the right fix. Doesn't this make the test test 
> cycles:P twice now?
> 
> I think the fix is to first test if cycles can be sampled, then test 
> cycles:P. Then test if instructions can be sampled, then test 
> instructions:P.
> 
> At the moment it checks the events with perf stat, and it checks them 
> both at the same time. Obviously that doesn't work if an event works for 
> perf stat but not perf record. And checking them both at the same time 
> doesn't allow for one existing and not the other.
> 
>>     then
>> -    echo "precise_max attribute [Failed instructions:P event]"
>> +    echo "precise_max attribute [Failed cycles:P event]"
>>       err=1
>>       return
>>     fi
>