Re: [patch 36/38] x86/ftrace: Make it call depth tracking aware

Peter Zijlstra
2022-07-19T08:48:14Z

On Mon, Jul 18, 2022 at 05:01:23PM -0400, Steven Rostedt wrote:

> From 533f10bd48ffbc4ee5d2a07f0a7fe99aeb1c823a Mon Sep 17 00:00:00 2001
> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> Date: Mon, 18 Jul 2022 16:01:07 -0400
> Subject: [PATCH] ftrace/x86: Remove jumps to ftrace_epilogue
>
> The jumps to ftrace_epilogue were done as a way to make sure all the
> function tracing trampolines ended at the function graph trampoline, as
> the ftrace_epilogue was the location that it would handle that.
>
> With the advent of function graph tracer now being just one of the
> callbacks of the function tracer there is no more requirement that all
> trampolines go to a single location.
>
> Remove the jumps to the ftrace_epilogue and replace them with return
> statements.
>
> Note, the ftrace_epilogue can probably be renamed to ftrace_stub and the
> weak logic for that could probably be removed. But lets leave that as a
> separate change.
>
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
> arch/x86/kernel/ftrace_64.S | 16 +++++++---------
> 1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/kernel/ftrace_64.S b/arch/x86/kernel/ftrace_64.S
> index dfeb227de561..8f225fafa5fb 100644
> --- a/arch/x86/kernel/ftrace_64.S
> +++ b/arch/x86/kernel/ftrace_64.S
> @@ -173,7 +173,9 @@ SYM_INNER_LABEL(ftrace_call, SYM_L_GLOBAL)
> SYM_INNER_LABEL(ftrace_caller_end, SYM_L_GLOBAL)
> ANNOTATE_NOENDBR
>
> - jmp ftrace_epilogue
> + UNWIND_HINT_FUNC
> + ENDBR

Only the RET should do I think; you definitely don't need an ENDBR here
nor do you need to override the unwind hint. Lemme try..

Yeah, the below is sufficient:

diff --git a/arch/x86/kernel/ftrace_64.S b/arch/x86/kernel/ftrace_64.S
index dfeb227de561..d6679b65b6f2 100644
--- a/arch/x86/kernel/ftrace_64.S
+++ b/arch/x86/kernel/ftrace_64.S
@@ -172,8 +172,7 @@ SYM_INNER_LABEL(ftrace_call, SYM_L_GLOBAL)
*/
SYM_INNER_LABEL(ftrace_caller_end, SYM_L_GLOBAL)
ANNOTATE_NOENDBR
-
- jmp ftrace_epilogue
+ RET
SYM_FUNC_END(ftrace_caller);
STACK_FRAME_NON_STANDARD_FP(ftrace_caller)

@@ -269,7 +268,7 @@ SYM_INNER_LABEL(ftrace_regs_caller_jmp, SYM_L_GLOBAL)
*/
SYM_INNER_LABEL(ftrace_regs_caller_end, SYM_L_GLOBAL)
ANNOTATE_NOENDBR
- jmp ftrace_epilogue
+ RET

/* Swap the flags with orig_rax */
1: movq MCOUNT_REG_SIZE(%rsp), %rdi
@@ -280,7 +279,7 @@ SYM_INNER_LABEL(ftrace_regs_caller_end, SYM_L_GLOBAL)
/* Restore flags */
popfq
UNWIND_HINT_FUNC
- jmp ftrace_epilogue
+ RET

SYM_FUNC_END(ftrace_regs_caller)
STACK_FRAME_NON_STANDARD_FP(ftrace_regs_caller)