Re: [PATCH RFC 09/13] arm64: ftrace: Maintain Tasks RCU trampoline nesting in ftrace_caller

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Build failure due to missing `ftrace_static_tramp_end` symbol when `CONFIG_DYNAMIC_FTRACE_WITH_ARGS` is disabled.
--

commit 459a25e6b720c08167da420eb83180b2856610e3
Author: Josef Bacik <josef@toxicpanda.com>

    arm64: ftrace: Maintain Tasks RCU trampoline nesting in ftrace_caller
    
    Bracket the call out to the ftrace_ops callback in ftrace_caller with an
    increment/decrement of current->rcu_tramp_nesting, using x12/w13 which
    are scratch there.  The read-modify-write is not atomic, but only current
    modifies the count and every interrupting user is balanced, so nothing is
    lost.

> diff --git a/arch/arm64/kernel/entry-ftrace.S b/arch/arm64/kernel/entry-ftrace.S
> index 025140caafe74..46a102e7199ac 100644
> --- a/arch/arm64/kernel/entry-ftrace.S
> +++ b/arch/arm64/kernel/entry-ftrace.S
> @@ -14,6 +14,33 @@
>  #include <asm/insn.h>
>  
>  #ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS

[ ... ]

> @@ -178,6 +209,10 @@ SYM_CODE_START(ftrace_stub_direct_tramp)
>  SYM_CODE_END(ftrace_stub_direct_tramp)
>  #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
>  
> +/* End of [ftrace_caller, ...) for arch_rcu_tasks_ip_in_trampoline(). */
> +SYM_CODE_START(ftrace_static_tramp_end)
> +SYM_CODE_END(ftrace_static_tramp_end)
> +
>  #else /* CONFIG_DYNAMIC_FTRACE_WITH_ARGS */
>  
>  /*

[Severity: Low]
Will this cause a linker error when compiling with
CONFIG_RCU_TASKS_PREEMPT_QS=y but CONFIG_DYNAMIC_FTRACE_WITH_ARGS=n?

The ftrace_static_tramp_end label is defined here exclusively inside the
CONFIG_DYNAMIC_FTRACE_WITH_ARGS block. The legacy mcount-based ftrace path
in the #else block lacks this definition.

> diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c
> index e1a3c0b3a0514..1b7ac2afed0df 100644
> --- a/arch/arm64/kernel/ftrace.c
> +++ b/arch/arm64/kernel/ftrace.c
> @@ -17,6 +17,22 @@
>  #include <asm/insn.h>
>  #include <asm/text-patching.h>
>  
> +#ifdef CONFIG_RCU_TASKS_PREEMPT_QS
> +extern void ftrace_static_tramp_end(void);
> +
> +/*
> + * See rcu_tasks_ip_in_trampoline().  ftrace_caller and ftrace_stub_direct_tramp
> + * are core kernel text but must be treated as trampolines: a task preempted in
> + * them may be carrying an ops pointer (x11) or a direct-call BPF trampoline
> + * address (x17) whose lifetime is guarded only by Tasks RCU.
> + */
> +bool arch_rcu_tasks_ip_in_trampoline(unsigned long ip)
> +{
> +	return ip >= (unsigned long)ftrace_caller &&
> +	       ip <  (unsigned long)ftrace_static_tramp_end;
> +}
> +#endif

[Severity: Low]
Since arch_rcu_tasks_ip_in_trampoline() unconditionally references
ftrace_static_tramp_end here under CONFIG_RCU_TASKS_PREEMPT_QS, this might
lead to an undefined reference at link time if CONFIG_DYNAMIC_FTRACE_WITH_ARGS
is disabled.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910-b4-rcu-tasks-preempt-qs-v1-0-d4469f4cc101@toxicpanda.com?part=9