On Mon, Oct 21, 2019 at 11:18:54PM -0000, tip-bot2 for Leo Yan wrote: > The following commit has been merged into the perf/core branch of tip: > > Commit-ID: 6a5f3d94cb69a185b921cb92c39888dc31009acb > Gitweb: > https://git.kernel.org/tip/6a5f3d94cb69a185b921cb92c39888dc31009acb > Author: Leo Yan <[email protected]> > AuthorDate: Fri, 18 Oct 2019 16:55:31 +08:00 > Committer: Arnaldo Carvalho de Melo <[email protected]> > CommitterDate: Sat, 19 Oct 2019 15:35:01 -03:00 > > perf tests: Disable bp_signal testing for arm64 > > As there are several discussions for enabling perf breakpoint signal > testing on arm64 platform: arm64 needs to rely on single-step to execute > the breakpointed instruction and then reinstall the breakpoint exception > handler. But if we hook the breakpoint with a signal, the signal > handler will do the stepping rather than the breakpointed instruction, > this causes infinite loops as below: > > Kernel space | Userspace > ---------------------------------|-------------------------------- > | __test_function() -> hit > | breakpoint > breakpoint_handler() | > `-> user_enable_single_step() | > do_signal() | > | sig_handler() -> Step one > | instruction and > | trap to kernel > single_step_handler() | > `-> reinstall_suspended_bps() | > | __test_function() -> hit > | breakpoint again and > | repeat up flow infinitely > > As Will Deacon mentioned [1]: "that we require the overflow handler to > do the stepping on arm/arm64, which is relied upon by GDB/ptrace. The > hw_breakpoint code is a complete disaster so my preference would be to > rip out the perf part and just implement something directly in ptrace, > but it's a pretty horrible job". Though Will commented this on arm > architecture, but the comment also can apply on arm64 architecture. > > For complete information, I searched online and found a few years back, > Wang Nan sent one patch 'arm64: Store breakpoint single step state into > pstate' [2]; the patch tried to resolve this issue by avoiding single > stepping in signal handler and defer to enable the signal stepping when > return to __test_function(). The fixing was not merged due to the > concern for missing to handle different usage cases. > > Based on the info, the most feasible way is to skip Perf breakpoint > signal testing for arm64 and this could avoid the duplicate > investigation efforts when people see the failure. This patch skips > this case on arm64 platform, which is same with arm architecture. > > [1] https://lkml.org/lkml/2018/11/15/205 > [2] https://lkml.org/lkml/2015/12/23/477 > > Signed-off-by: Leo Yan <[email protected]> > Cc: Adrian Hunter <[email protected]> > Cc: Alexander Shishkin <[email protected]> > Cc: Brajeswar Ghosh <[email protected]> > Cc: Florian Fainelli <[email protected]> > Cc: Jiri Olsa <[email protected]> > Cc: Mark Rutland <[email protected]> > Cc: Michael Petlan <[email protected]> > Cc: Namhyung Kim <[email protected]> > Cc: Peter Zijlstra <[email protected]> > Cc: Song Liu <[email protected]> > Cc: Souptick Joarder <[email protected]> > Cc: Will Deacon <[email protected]> > Link: http://lore.kernel.org/lkml/[email protected] > Signed-off-by: Arnaldo Carvalho de Melo <[email protected]> > --- > tools/perf/tests/bp_signal.c | 15 ++++++--------- > 1 file changed, 6 insertions(+), 9 deletions(-) > > diff --git a/tools/perf/tests/bp_signal.c b/tools/perf/tests/bp_signal.c > index c1c2c13..166f411 100644 > --- a/tools/perf/tests/bp_signal.c > +++ b/tools/perf/tests/bp_signal.c > @@ -49,14 +49,6 @@ asm ( > "__test_function:\n" > "incq (%rdi)\n" > "ret\n"); > -#elif defined (__aarch64__) > -extern void __test_function(volatile long *ptr); > -asm ( > - ".globl __test_function\n" > - "__test_function:\n" > - "str x30, [x0]\n" > - "ret\n"); > - > #else > static void __test_function(volatile long *ptr) > { > @@ -302,10 +294,15 @@ bool test__bp_signal_is_supported(void) > * stepping into the SIGIO handler and getting stuck on the > * breakpointed instruction. > * > + * Since arm64 has the same issue with arm for the single-step > + * handling, this case also gets suck on the breakpointed > + * instruction.
Freudian slip? Will
