FUNCTION_GRAPH_TRACER + UNWINDER_FRAME_POINTER broken on arm

[Uwe Kleine-König]

Hello Arnd,

On Mon, May 27, 2024 at 11:18:32AM +0200, Arnd Bergmann wrote:
> On Sat, May 25, 2024, at 09:59, Uwe Kleine-König wrote:
> >
> > With FUNCTION_GRAPH_TRACER=y and CONFIG_UNWINDER_FRAME_POINTER=y I
> > experience the following issue:
> >
> > 	# echo function_graph > /sys/kernel/tracing/current_tracer
> > 	[  303.812295] Kernel panic - not syncing: �e
> > 	[  303.816738] ---[ end Kernel panic - not syncing: �e ]---
> >
> > This reproduces reliably on v6.9 (on an i.MX25 based machine).
> >
> > Bisection points on v5.18-rc1~150^2~7^2~6 ("ARM: ftrace: enable
> 
> I took a look at what the remaining uses of the frame pointer
> unwinder are, or if we could just use the Arm unwinder
> everywhere. For OABI kernels, the frame pointer unwinder is
> still required of course, but the Arm unwinder is already the
> default for all CONFIG_AEABI kernels and required for
> all v6/v7/v7m ones. Until two years ago with commit
> 41918ec82eb6 ("ARM: ftrace: enable the graph tracer with
> the EABI unwinder"), the frame pointer unwinder was also
> required for CONFIG_FUNCTION_GRAPH_TRACER, and that is
> likely why you had that configuration, but we no longer
> depend on it for anything else.
> 
> However, I came across this bit from arch/arm/include/asm/ftrace.h
> that seems to be the only downside of CONFIG_ARM_UNWIND:
> 
> #if defined(CONFIG_FRAME_POINTER) && !defined(CONFIG_ARM_UNWIND)
> /*
>  * return_address uses walk_stackframe to do it's work.  If both
>  * CONFIG_FRAME_POINTER=y and CONFIG_ARM_UNWIND=y walk_stackframe uses unwind
>  * information.  For this to work in the function tracer many functions would
>  * have to be marked with __notrace.  So for now just depend on
>  * !CONFIG_ARM_UNWIND.
>  */
> void *return_address(unsigned int);
> #else
> static inline void *return_address(unsigned int level)
> {
>        return NULL;
> }
> #endif
> #define ftrace_return_address(n) return_address(n)
> 
> You added this in 4bf1fa5a34aa ("[ARM] 5613/1: implement
> CALLER_ADDRESSx") 15 years ago, and it appears nobody ever
> got around to finish the job for return_address() if I read
> this correctly. Is this something we should finally add,
> and then make the frame pointer unwinder specific to OABI
> kernels?

Oh, now you mention it I remember, but only on a high level and not the
deep details. I guess nobody came around finishing it because having the
dummy implementation for return_address doesn't hurt?

Completing it would mean adding the numerous __notrace annotions??

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20240527/0df627e5/attachment.sig>