* [PATCH v5 2/2] arm64/ftrace: Make function graph use ftrace directly
2022-04-20 16:00 [PATCH v5 1/2] ftrace: cleanup ftrace_graph_caller enable and disable Chengming Zhou
@ 2022-04-20 16:00 ` Chengming Zhou
2022-04-25 9:51 ` [PATCH v5 1/2] ftrace: cleanup ftrace_graph_caller enable and disable Mark Rutland
2022-04-29 19:19 ` Catalin Marinas
2 siblings, 0 replies; 6+ messages in thread
From: Chengming Zhou @ 2022-04-20 16:00 UTC (permalink / raw)
To: rostedt, mark.rutland, mingo, catalin.marinas, will, tglx,
dave.hansen, broonie, ardb
Cc: x86, linux-arm-kernel, linux-kernel, duanxiongchun, songmuchun,
zhengqi.arch, Chengming Zhou
As we do in commit 0c0593b45c9b ("x86/ftrace: Make function graph
use ftrace directly"), we don't need special hook for graph tracer,
but instead we use graph_ops:func function to install return_hooker.
Since commit 3b23e4991fb6 ("arm64: implement ftrace with regs") add
implementation for FTRACE_WITH_REGS on arm64, we can easily adopt
the same cleanup on arm64.
And this cleanup only changes the FTRACE_WITH_REGS implementation,
so the mcount-based implementation is unaffected.
While in theory it would be possible to make a similar cleanup for
!FTRACE_WITH_REGS, this will require rework of the core code, and
so for now we only change the FTRACE_WITH_REGS implementation.
Tested-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>
---
v5:
- Add reason why this patch only changed FTRACE_WITH_REGS
implementation in the commit message as suggested by Mark.
- Change to use nicer comment in ftrace_graph_func() as suggested
by Mark, because the previous comment sounds like there's an bug
to be fixed.
- Add Tested-by and Reviewed-by tags from Mark, thanks.
v3:
- Add comments in ftrace_graph_func() as suggested by Steve. Thanks.
v2:
- Remove FTRACE_WITH_REGS ftrace_graph_caller asm, thanks Mark.
---
arch/arm64/include/asm/ftrace.h | 7 +++++++
arch/arm64/kernel/entry-ftrace.S | 17 -----------------
arch/arm64/kernel/ftrace.c | 17 +++++++++++++++++
3 files changed, 24 insertions(+), 17 deletions(-)
diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h
index 1494cfa8639b..dbc45a4157fa 100644
--- a/arch/arm64/include/asm/ftrace.h
+++ b/arch/arm64/include/asm/ftrace.h
@@ -80,8 +80,15 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
struct dyn_ftrace;
+struct ftrace_ops;
+struct ftrace_regs;
+
int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec);
#define ftrace_init_nop ftrace_init_nop
+
+void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *op, struct ftrace_regs *fregs);
+#define ftrace_graph_func ftrace_graph_func
#endif
#define ftrace_return_address(n) return_address(n)
diff --git a/arch/arm64/kernel/entry-ftrace.S b/arch/arm64/kernel/entry-ftrace.S
index e535480a4069..d42a205ef625 100644
--- a/arch/arm64/kernel/entry-ftrace.S
+++ b/arch/arm64/kernel/entry-ftrace.S
@@ -97,12 +97,6 @@ SYM_CODE_START(ftrace_common)
SYM_INNER_LABEL(ftrace_call, SYM_L_GLOBAL)
bl ftrace_stub
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-SYM_INNER_LABEL(ftrace_graph_call, SYM_L_GLOBAL) // ftrace_graph_caller();
- nop // If enabled, this will be replaced
- // "b ftrace_graph_caller"
-#endif
-
/*
* At the callsite x0-x8 and x19-x30 were live. Any C code will have preserved
* x19-x29 per the AAPCS, and we created frame records upon entry, so we need
@@ -127,17 +121,6 @@ ftrace_common_return:
ret x9
SYM_CODE_END(ftrace_common)
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-SYM_CODE_START(ftrace_graph_caller)
- ldr x0, [sp, #S_PC]
- sub x0, x0, #AARCH64_INSN_SIZE // ip (callsite's BL insn)
- add x1, sp, #S_LR // parent_ip (callsite's LR)
- ldr x2, [sp, #PT_REGS_SIZE] // parent fp (callsite's FP)
- bl prepare_ftrace_return
- b ftrace_common_return
-SYM_CODE_END(ftrace_graph_caller)
-#endif
-
#else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
/*
diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c
index 4506c4a90ac1..f447c4a36f69 100644
--- a/arch/arm64/kernel/ftrace.c
+++ b/arch/arm64/kernel/ftrace.c
@@ -268,6 +268,22 @@ void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent,
}
#ifdef CONFIG_DYNAMIC_FTRACE
+
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *op, struct ftrace_regs *fregs)
+{
+ /*
+ * When DYNAMIC_FTRACE_WITH_REGS is selected, `fregs` can never be NULL
+ * and arch_ftrace_get_regs(fregs) will always give a non-NULL pt_regs
+ * in which we can safely modify the LR.
+ */
+ struct pt_regs *regs = arch_ftrace_get_regs(fregs);
+ unsigned long *parent = (unsigned long *)&procedure_link_pointer(regs);
+
+ prepare_ftrace_return(ip, parent, frame_pointer(regs));
+}
+#else
/*
* Turn on/off the call to ftrace_graph_caller() in ftrace_caller()
* depending on @enable.
@@ -297,5 +313,6 @@ int ftrace_disable_ftrace_graph_caller(void)
{
return ftrace_modify_graph_caller(false);
}
+#endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
#endif /* CONFIG_DYNAMIC_FTRACE */
#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
--
2.35.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v5 1/2] ftrace: cleanup ftrace_graph_caller enable and disable
2022-04-20 16:00 [PATCH v5 1/2] ftrace: cleanup ftrace_graph_caller enable and disable Chengming Zhou
2022-04-20 16:00 ` [PATCH v5 2/2] arm64/ftrace: Make function graph use ftrace directly Chengming Zhou
@ 2022-04-25 9:51 ` Mark Rutland
2022-04-26 1:27 ` Steven Rostedt
2022-04-29 19:19 ` Catalin Marinas
2 siblings, 1 reply; 6+ messages in thread
From: Mark Rutland @ 2022-04-25 9:51 UTC (permalink / raw)
To: Chengming Zhou, rostedt
Cc: mingo, catalin.marinas, will, tglx, dave.hansen, broonie, ardb,
x86, linux-arm-kernel, linux-kernel, duanxiongchun, songmuchun,
zhengqi.arch
On Thu, Apr 21, 2022 at 12:00:05AM +0800, Chengming Zhou wrote:
> The ftrace_[enable,disable]_ftrace_graph_caller() are used to do
> special hooks for graph tracer, which are not needed on some ARCHs
> that use graph_ops:func function to install return_hooker.
>
> So introduce the weak version in ftrace core code to cleanup
> in x86.
>
> Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>
FWIW:
Acked-by: Mark Rutland <mark.rutland@arm.com>
Steve, are you happy if this goes via the arm64 tree along with patch 2?
Thanks,
Mark.
> v4:
> - put weak ftrace_enable,disable_ftrace_graph_caller() in
> fgraph.c instead of ftrace.c as suggested by Steve.
>
> v3:
> - consolidate two #if into a single #if, suggested by Steve. Thanks.
> ---
> arch/x86/kernel/ftrace.c | 17 ++---------------
> kernel/trace/fgraph.c | 18 ++++++++++++++++++
> 2 files changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> index 1e31c7d21597..b09d73c2ba89 100644
> --- a/arch/x86/kernel/ftrace.c
> +++ b/arch/x86/kernel/ftrace.c
> @@ -579,9 +579,7 @@ void arch_ftrace_trampoline_free(struct ftrace_ops *ops)
>
> #ifdef CONFIG_FUNCTION_GRAPH_TRACER
>
> -#ifdef CONFIG_DYNAMIC_FTRACE
> -
> -#ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
> +#if defined(CONFIG_DYNAMIC_FTRACE) && !defined(CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS)
> extern void ftrace_graph_call(void);
> static const char *ftrace_jmp_replace(unsigned long ip, unsigned long addr)
> {
> @@ -610,18 +608,7 @@ int ftrace_disable_ftrace_graph_caller(void)
>
> return ftrace_mod_jmp(ip, &ftrace_stub);
> }
> -#else /* !CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS */
> -int ftrace_enable_ftrace_graph_caller(void)
> -{
> - return 0;
> -}
> -
> -int ftrace_disable_ftrace_graph_caller(void)
> -{
> - return 0;
> -}
> -#endif /* CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS */
> -#endif /* !CONFIG_DYNAMIC_FTRACE */
> +#endif /* CONFIG_DYNAMIC_FTRACE && !CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS */
>
> /*
> * Hook the return address and push it in the stack of return addrs
> diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
> index 8f4fb328133a..289311680c29 100644
> --- a/kernel/trace/fgraph.c
> +++ b/kernel/trace/fgraph.c
> @@ -30,6 +30,24 @@ int ftrace_graph_active;
> /* Both enabled by default (can be cleared by function_graph tracer flags */
> static bool fgraph_sleep_time = true;
>
> +/*
> + * archs can override this function if they must do something
> + * to enable hook for graph tracer.
> + */
> +int __weak ftrace_enable_ftrace_graph_caller(void)
> +{
> + return 0;
> +}
> +
> +/*
> + * archs can override this function if they must do something
> + * to disable hook for graph tracer.
> + */
> +int __weak ftrace_disable_ftrace_graph_caller(void)
> +{
> + return 0;
> +}
> +
> /**
> * ftrace_graph_stop - set to permanently disable function graph tracing
> *
> --
> 2.35.2
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v5 1/2] ftrace: cleanup ftrace_graph_caller enable and disable
2022-04-20 16:00 [PATCH v5 1/2] ftrace: cleanup ftrace_graph_caller enable and disable Chengming Zhou
2022-04-20 16:00 ` [PATCH v5 2/2] arm64/ftrace: Make function graph use ftrace directly Chengming Zhou
2022-04-25 9:51 ` [PATCH v5 1/2] ftrace: cleanup ftrace_graph_caller enable and disable Mark Rutland
@ 2022-04-29 19:19 ` Catalin Marinas
2 siblings, 0 replies; 6+ messages in thread
From: Catalin Marinas @ 2022-04-29 19:19 UTC (permalink / raw)
To: rostedt, mingo, tglx, will, mark.rutland, Chengming Zhou, ardb,
broonie, dave.hansen
Cc: songmuchun, linux-arm-kernel, duanxiongchun, zhengqi.arch,
linux-kernel, x86
On Thu, 21 Apr 2022 00:00:05 +0800, Chengming Zhou wrote:
> The ftrace_[enable,disable]_ftrace_graph_caller() are used to do
> special hooks for graph tracer, which are not needed on some ARCHs
> that use graph_ops:func function to install return_hooker.
>
> So introduce the weak version in ftrace core code to cleanup
> in x86.
>
> [...]
Applied to arm64 (for-next/ftrace), thanks!
[1/2] ftrace: cleanup ftrace_graph_caller enable and disable
https://git.kernel.org/arm64/c/e999995c84c3
[2/2] arm64/ftrace: Make function graph use ftrace directly
https://git.kernel.org/arm64/c/c4a0ebf87ceb
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread