2022-04-28 16:44:54

by Thomas Richter

[permalink] [raw]
Subject: [PATCH] perf test: Fix test case 81 on s390x

perf test -F 81 -v fails on s390x on the linux-next branch.
The test case is x86 specific can not be executed on s390x.
The test case depends on x86 register names such as

... | egrep -q 'available registers: AX BX CX DX ....'

Skip this test case on s390x.

Output before:
# perf test -F 81
81: perf record tests : FAILED!
#

Output after:
# perf test -F 81
81: perf record tests : Skip
#

Fixes: 24f378e66021 ("perf test: Add basic perf record tests")
Cc: Ian Rogers <[email protected]>
Signed-off-by: Thomas Richter <[email protected]>
---
tools/perf/tests/shell/record.sh | 2 ++
1 file changed, 2 insertions(+)

diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
index cd1cf14259b8..d98f4d4a00e1 100755
--- a/tools/perf/tests/shell/record.sh
+++ b/tools/perf/tests/shell/record.sh
@@ -37,6 +37,8 @@ test_register_capture() {
echo "Register capture test [Success]"
}

+# Test for platform support and return TEST_SKIP
+[ $(uname -m) = s390x ] && exit 2
test_per_thread
test_register_capture
exit $err
--
2.35.1


2022-04-28 20:14:39

by Thomas Richter

[permalink] [raw]
Subject: Re: [PATCH] perf test: Fix test case 81 on s390x

On 4/28/22 15:43, German Gomez wrote:
>
> On 28/04/2022 13:28, Thomas Richter wrote:
>> perf test -F 81 -v fails on s390x on the linux-next branch.
>> The test case is x86 specific can not be executed on s390x.
>> The test case depends on x86 register names such as
>>
>> ... | egrep -q 'available registers: AX BX CX DX ....'
>>
>> Skip this test case on s390x.
>>
>> Output before:
>> # perf test -F 81
>> 81: perf record tests : FAILED!
>> #
>>
>> Output after:
>> # perf test -F 81
>> 81: perf record tests : Skip
>> #
>>
>> Fixes: 24f378e66021 ("perf test: Add basic perf record tests")
>> Cc: Ian Rogers <[email protected]>
>> Signed-off-by: Thomas Richter <[email protected]>
>> ---
>> tools/perf/tests/shell/record.sh | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
>> index cd1cf14259b8..d98f4d4a00e1 100755
>> --- a/tools/perf/tests/shell/record.sh
>> +++ b/tools/perf/tests/shell/record.sh
>> @@ -37,6 +37,8 @@ test_register_capture() {
>> echo "Register capture test [Success]"
>> }
>>
>> +# Test for platform support and return TEST_SKIP
>> +[ $(uname -m) = s390x ] && exit 2
>> test_per_thread
>
> The "test_per_thread" might still be valid though, right?
>

Right,
this issue is the perf record -e instructions:u event
which is not supported on all machine types and configurations
we have in our test suite.

Because of the 'set -e' at the beginning, the first perf
command fails and the shell exits with error code.

Since s390x does not support x86 register names anyway, it is easier
to just omit this test case on s390x. At least I thought so.

The alternative would be to add some more tests to check s390x
machine supports instructions:u event and then just to bail
out on the register_capture test function.

Maybe this helps.

> In my case, the register test skips gracefully (arm64).
>
> $ ./perf test 82 -v
>  82: perf record tests                                               :
> --- start ---
> test child forked, pid 54345
> Basic --per-thread mode test
> Basic --per-thread mode test [Success]
> Register capture test
> Register capture test [Skipped missing instruction]
> test child finished with 0
> ---- end ----
> perf record tests: Ok
>
>> test_register_capture
>> exit $err


--
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294

2022-04-28 21:24:57

by German Gomez

[permalink] [raw]
Subject: Re: [PATCH] perf test: Fix test case 81 on s390x


On 28/04/2022 13:28, Thomas Richter wrote:
> perf test -F 81 -v fails on s390x on the linux-next branch.
> The test case is x86 specific can not be executed on s390x.
> The test case depends on x86 register names such as
>
> ... | egrep -q 'available registers: AX BX CX DX ....'
>
> Skip this test case on s390x.
>
> Output before:
> # perf test -F 81
> 81: perf record tests : FAILED!
> #
>
> Output after:
> # perf test -F 81
> 81: perf record tests : Skip
> #
>
> Fixes: 24f378e66021 ("perf test: Add basic perf record tests")
> Cc: Ian Rogers <[email protected]>
> Signed-off-by: Thomas Richter <[email protected]>
> ---
> tools/perf/tests/shell/record.sh | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
> index cd1cf14259b8..d98f4d4a00e1 100755
> --- a/tools/perf/tests/shell/record.sh
> +++ b/tools/perf/tests/shell/record.sh
> @@ -37,6 +37,8 @@ test_register_capture() {
> echo "Register capture test [Success]"
> }
>
> +# Test for platform support and return TEST_SKIP
> +[ $(uname -m) = s390x ] && exit 2
> test_per_thread

The "test_per_thread" might still be valid though, right?

In my case, the register test skips gracefully (arm64).

$ ./perf test 82 -v
 82: perf record tests                                               :
--- start ---
test child forked, pid 54345
Basic --per-thread mode test
Basic --per-thread mode test [Success]
Register capture test
Register capture test [Skipped missing instruction]
test child finished with 0
---- end ----
perf record tests: Ok

> test_register_capture
> exit $err

2022-04-29 10:45:25

by John Garry

[permalink] [raw]
Subject: Re: [PATCH] perf test: Fix test case 81 on s390x

On 28/04/2022 14:25, Arnaldo Carvalho de Melo wrote:
> Em Thu, Apr 28, 2022 at 02:28:21PM +0200, Thomas Richter escreveu:
>> perf test -F 81 -v fails on s390x on the linux-next branch.
>> The test case is x86 specific can not be executed on s390x.
>> The test case depends on x86 register names such as
>>
>> ... | egrep -q 'available registers: AX BX CX DX ....'
>
> Thanks,
>
> Ian, I guess this will break on other !x86 arches as well.

JFYI, arm64 gives on acme perf/core branch @ e0c1b8f9eb this:

john@debian:~/acme/tools/perf$ sudo ./perf test -F 82
82: perf record tests :Basic --per-thread mode test
Basic --per-thread mode test [Success]
Register capture test
Register capture test [Skipped missing instruction]
Ok
john@debian:~/acme/tools/perf$

> Can you
> please take a look?
>
> - Arnaldo
>
>> Skip this test case on s390x.
>>
>> Output before:
>> # perf test -F 81
>> 81: perf record tests : FAILED!
>> #
>>
>> Output after:
>> # perf test -F 81
>> 81: perf record tests : Skip
>> #
>>
>> Fixes: 24f378e66021 ("perf test: Add basic perf record tests")
>> Cc: Ian Rogers <[email protected]>
>> Signed-off-by: Thomas Richter <[email protected]>
>> ---
>> tools/perf/tests/shell/record.sh | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
>> index cd1cf14259b8..d98f4d4a00e1 100755
>> --- a/tools/perf/tests/shell/record.sh
>> +++ b/tools/perf/tests/shell/record.sh
>> @@ -37,6 +37,8 @@ test_register_capture() {
>> echo "Register capture test [Success]"
>> }
>>
>> +# Test for platform support and return TEST_SKIP
>> +[ $(uname -m) = s390x ] && exit 2
>> test_per_thread
>> test_register_capture
>> exit $err
>> --
>> 2.35.1
>

2022-05-01 16:35:11

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf test: Fix test case 81 on s390x

Em Thu, Apr 28, 2022 at 02:28:21PM +0200, Thomas Richter escreveu:
> perf test -F 81 -v fails on s390x on the linux-next branch.
> The test case is x86 specific can not be executed on s390x.
> The test case depends on x86 register names such as
>
> ... | egrep -q 'available registers: AX BX CX DX ....'

Thanks,

Ian, I guess this will break on other !x86 arches as well. Can you
please take a look?

- Arnaldo

> Skip this test case on s390x.
>
> Output before:
> # perf test -F 81
> 81: perf record tests : FAILED!
> #
>
> Output after:
> # perf test -F 81
> 81: perf record tests : Skip
> #
>
> Fixes: 24f378e66021 ("perf test: Add basic perf record tests")
> Cc: Ian Rogers <[email protected]>
> Signed-off-by: Thomas Richter <[email protected]>
> ---
> tools/perf/tests/shell/record.sh | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
> index cd1cf14259b8..d98f4d4a00e1 100755
> --- a/tools/perf/tests/shell/record.sh
> +++ b/tools/perf/tests/shell/record.sh
> @@ -37,6 +37,8 @@ test_register_capture() {
> echo "Register capture test [Success]"
> }
>
> +# Test for platform support and return TEST_SKIP
> +[ $(uname -m) = s390x ] && exit 2
> test_per_thread
> test_register_capture
> exit $err
> --
> 2.35.1

--

- Arnaldo