[PATCH v2] perf test: Skip Sigtrap test for arm+aarch64

[PATCH v2] perf test: Skip Sigtrap test for arm+aarch64

[John Garry]

Skip the Sigtrap test for arm + arm64, same as was done for s390 in
commit a840974e96fd ("perf test: Test 73 Sig_trap fails on s390"). For
this, reuse BP_SIGNAL_IS_SUPPORTED - meaning that the arch can use BP to
generate signals - instead of BP_ACCOUNT_IS_SUPPORTED, which is
appropriate.

As described by Will at [0], in the test we get stuck in a loop of handling
the HW breakpoint exception and never making progress. GDB handles this
by stepping over the faulting instruction, but with perf the kernel is
expected to handle the step (which it doesn't for arm).

Dmitry made an attempt to get this work, also mentioned in the same thread
as [0], which was appreciated. But the best thing to do is skip the test
for now.

[0] https://lore.kernel.org/linux-perf-users/20220118124343.GC98966@leoy-ThinkPad-X240s/T/#m13b06c39d2a5100d340f009435df6f4d8ee57b5a

Fixes: Fixes: 5504f67944484 ("perf test sigtrap: Add basic stress test for sigtrap handling")
Signed-off-by: John Garry <john.garry@huawei.com>

diff --git a/tools/perf/tests/sigtrap.c b/tools/perf/tests/sigtrap.c
index 1f147fe6595f..e32ece90e164 100644
--- a/tools/perf/tests/sigtrap.c
+++ b/tools/perf/tests/sigtrap.c
@@ -22,19 +22,6 @@
 #include "tests.h"
 #include "../perf-sys.h"
 
-/*
- * PowerPC and S390 do not support creation of instruction breakpoints using the
- * perf_event interface.
- *
- * Just disable the test for these architectures until these issues are
- * resolved.
- */
-#if defined(__powerpc__) || defined(__s390x__)
-#define BP_ACCOUNT_IS_SUPPORTED 0
-#else
-#define BP_ACCOUNT_IS_SUPPORTED 1
-#endif
-
 #define NUM_THREADS 5
 
 static struct {
@@ -135,7 +122,7 @@ static int test__sigtrap(struct test_suite *test __maybe_unused, int subtest __m
 	char sbuf[STRERR_BUFSIZE];
 	int i, fd, ret = TEST_FAIL;
 
-	if (!BP_ACCOUNT_IS_SUPPORTED) {
+	if (!BP_SIGNAL_IS_SUPPORTED) {
 		pr_debug("Test not supported on this architecture");
 		return TEST_SKIP;
 	}
-- 
2.26.2

Re: [PATCH v2] perf test: Skip Sigtrap test for arm+aarch64

[Marco Elver]

On Fri, 18 Feb 2022 at 10:39, John Garry <john.garry@huawei.com> wrote:
>
> Skip the Sigtrap test for arm + arm64, same as was done for s390 in
> commit a840974e96fd ("perf test: Test 73 Sig_trap fails on s390"). For
> this, reuse BP_SIGNAL_IS_SUPPORTED - meaning that the arch can use BP to
> generate signals - instead of BP_ACCOUNT_IS_SUPPORTED, which is
> appropriate.
>
> As described by Will at [0], in the test we get stuck in a loop of handling
> the HW breakpoint exception and never making progress. GDB handles this
> by stepping over the faulting instruction, but with perf the kernel is
> expected to handle the step (which it doesn't for arm).
>
> Dmitry made an attempt to get this work, also mentioned in the same thread
> as [0], which was appreciated. But the best thing to do is skip the test
> for now.
>
> [0] https://lore.kernel.org/linux-perf-users/20220118124343.GC98966@leoy-ThinkPad-X240s/T/#m13b06c39d2a5100d340f009435df6f4d8ee57b5a
>
> Fixes: Fixes: 5504f67944484 ("perf test sigtrap: Add basic stress test for sigtrap handling")
> Signed-off-by: John Garry <john.garry@huawei.com>

Acked-by: Marco Elver <elver@google.com>

Thanks !

> diff --git a/tools/perf/tests/sigtrap.c b/tools/perf/tests/sigtrap.c
> index 1f147fe6595f..e32ece90e164 100644
> --- a/tools/perf/tests/sigtrap.c
> +++ b/tools/perf/tests/sigtrap.c
> @@ -22,19 +22,6 @@
>  #include "tests.h"
>  #include "../perf-sys.h"
>
> -/*
> - * PowerPC and S390 do not support creation of instruction breakpoints using the
> - * perf_event interface.
> - *
> - * Just disable the test for these architectures until these issues are
> - * resolved.
> - */
> -#if defined(__powerpc__) || defined(__s390x__)
> -#define BP_ACCOUNT_IS_SUPPORTED 0
> -#else
> -#define BP_ACCOUNT_IS_SUPPORTED 1
> -#endif
> -
>  #define NUM_THREADS 5
>
>  static struct {
> @@ -135,7 +122,7 @@ static int test__sigtrap(struct test_suite *test __maybe_unused, int subtest __m
>         char sbuf[STRERR_BUFSIZE];
>         int i, fd, ret = TEST_FAIL;
>
> -       if (!BP_ACCOUNT_IS_SUPPORTED) {
> +       if (!BP_SIGNAL_IS_SUPPORTED) {
>                 pr_debug("Test not supported on this architecture");
>                 return TEST_SKIP;
>         }
> --
> 2.26.2
>

Re: [PATCH v2] perf test: Skip Sigtrap test for arm+aarch64

[Leo Yan]

On Fri, Feb 18, 2022 at 05:33:33PM +0800, John Garry wrote:
> Skip the Sigtrap test for arm + arm64, same as was done for s390 in
> commit a840974e96fd ("perf test: Test 73 Sig_trap fails on s390"). For
> this, reuse BP_SIGNAL_IS_SUPPORTED - meaning that the arch can use BP to
> generate signals - instead of BP_ACCOUNT_IS_SUPPORTED, which is
> appropriate.
> 
> As described by Will at [0], in the test we get stuck in a loop of handling
> the HW breakpoint exception and never making progress. GDB handles this
> by stepping over the faulting instruction, but with perf the kernel is
> expected to handle the step (which it doesn't for arm).
> 
> Dmitry made an attempt to get this work, also mentioned in the same thread
> as [0], which was appreciated. But the best thing to do is skip the test
> for now.
> 
> [0] https://lore.kernel.org/linux-perf-users/20220118124343.GC98966@leoy-ThinkPad-X240s/T/#m13b06c39d2a5100d340f009435df6f4d8ee57b5a
> 
> Fixes: Fixes: 5504f67944484 ("perf test sigtrap: Add basic stress test for sigtrap handling")
> Signed-off-by: John Garry <john.garry@huawei.com>

I tested this patch on my Juno board:

root@Juno:# ./perf test 73
 73: Sigtrap                                                         : Skip

Tested-by: Leo Yan <leo.yan@linaro.org>

Re: [PATCH v2] perf test: Skip Sigtrap test for arm+aarch64

[Arnaldo Carvalho de Melo]

Em Fri, Feb 18, 2022 at 06:46:11PM +0800, Leo Yan escreveu:
> On Fri, Feb 18, 2022 at 05:33:33PM +0800, John Garry wrote:
> > Skip the Sigtrap test for arm + arm64, same as was done for s390 in
> > commit a840974e96fd ("perf test: Test 73 Sig_trap fails on s390"). For
> > this, reuse BP_SIGNAL_IS_SUPPORTED - meaning that the arch can use BP to
> > generate signals - instead of BP_ACCOUNT_IS_SUPPORTED, which is
> > appropriate.
> > 
> > As described by Will at [0], in the test we get stuck in a loop of handling
> > the HW breakpoint exception and never making progress. GDB handles this
> > by stepping over the faulting instruction, but with perf the kernel is
> > expected to handle the step (which it doesn't for arm).
> > 
> > Dmitry made an attempt to get this work, also mentioned in the same thread
> > as [0], which was appreciated. But the best thing to do is skip the test
> > for now.
> > 
> > [0] https://lore.kernel.org/linux-perf-users/20220118124343.GC98966@leoy-ThinkPad-X240s/T/#m13b06c39d2a5100d340f009435df6f4d8ee57b5a
> > 
> > Fixes: Fixes: 5504f67944484 ("perf test sigtrap: Add basic stress test for sigtrap handling")
> > Signed-off-by: John Garry <john.garry@huawei.com>
> 
> I tested this patch on my Juno board:
> 
> root@Juno:# ./perf test 73
>  73: Sigtrap                                                         : Skip
> 
> Tested-by: Leo Yan <leo.yan@linaro.org>

Thanks, applied.

- Arnaldo

Re: [PATCH v2] perf test: Skip Sigtrap test for arm+aarch64

[kajoljain]

On 2/18/22 6:26 PM, Arnaldo Carvalho de Melo wrote:
> Em Fri, Feb 18, 2022 at 06:46:11PM +0800, Leo Yan escreveu:
>> On Fri, Feb 18, 2022 at 05:33:33PM +0800, John Garry wrote:
>>> Skip the Sigtrap test for arm + arm64, same as was done for s390 in
>>> commit a840974e96fd ("perf test: Test 73 Sig_trap fails on s390"). For
>>> this, reuse BP_SIGNAL_IS_SUPPORTED - meaning that the arch can use BP to
>>> generate signals - instead of BP_ACCOUNT_IS_SUPPORTED, which is
>>> appropriate.
>>>
>>> As described by Will at [0], in the test we get stuck in a loop of handling
>>> the HW breakpoint exception and never making progress. GDB handles this
>>> by stepping over the faulting instruction, but with perf the kernel is
>>> expected to handle the step (which it doesn't for arm).
>>>
>>> Dmitry made an attempt to get this work, also mentioned in the same thread
>>> as [0], which was appreciated. But the best thing to do is skip the test
>>> for now.
>>>
>>> [0] https://lore.kernel.org/linux-perf-users/20220118124343.GC98966@leoy-ThinkPad-X240s/T/#m13b06c39d2a5100d340f009435df6f4d8ee57b5a
>>>
>>> Fixes: Fixes: 5504f67944484 ("perf test sigtrap: Add basic stress test for sigtrap handling")
>>> Signed-off-by: John Garry <john.garry@huawei.com>
>>
>> I tested this patch on my Juno board:
>>
>> root@Juno:# ./perf test 73
>>  73: Sigtrap                                                         : Skip
>>
>> Tested-by: Leo Yan <leo.yan@linaro.org>

Sorry for late update.

Patch looks good to me. I tested it on powerpc.

Tested-by: Kajol Jain<kjain@linux.ibm.com>

Thanks,
Kajol Jain
> 
> Thanks, applied.
> 
> - Arnaldo
>