[RFC,riscv/for-next,4/4] riscv: ftrace: do not use stop_machine to update code

Commit Message

Andy Chiu May 29, 2022, 2:33 p.m. UTC

Now it is safe to remove dependency from stop_machine() to patch code in
ftrace.

Signed-off-by: Andy Chiu <[e-mail]>
Reviewed-by: Greentime Hu <[e-mail]>
Reviewed-by: Zong Li <[e-mail]>
---
 arch/riscv/kernel/ftrace.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Steven Rostedt June 5, 2022, 4:14 p.m. UTC | #1

On Sun, 29 May 2022 22:33:15 +0800
Andy Chiu <[e-mail]> wrote:

> Now it is safe to remove dependency from stop_machine() to patch code in
> ftrace.
> 

I wasn't able to test it, and I don't know the intrinsic details of
risc-v, but from the change logs and looking at the code from what I
could understand, it all seems reasonable to me.

For the entire patch set:

Acked-by: Steven Rostedt (Google) <[e-mail]>

-- Steve


> Signed-off-by: Andy Chiu <[e-mail]>
> Reviewed-by: Greentime Hu <[e-mail]>
> Reviewed-by: Zong Li <[e-mail]>
> ---
>  arch/riscv/kernel/ftrace.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c
> index d4bf0e5255f6..e7b8bf0a699b 100644
> --- a/arch/riscv/kernel/ftrace.c
> +++ b/arch/riscv/kernel/ftrace.c
> @@ -12,6 +12,11 @@
>  #include <asm/patch.h>
>  
>  #ifdef CONFIG_DYNAMIC_FTRACE
> +void arch_ftrace_update_code(int command)
> +{
> +	ftrace_modify_all_code(command);
> +}
> +
>  int ftrace_arch_code_modify_prepare(void) __acquires(&text_mutex)
>  {
>  	mutex_lock(&text_mutex);

Palmer Dabbelt July 21, 2022, 5:40 a.m. UTC | #2

On Sun, 29 May 2022 07:33:15 PDT (-0700), [e-mail] wrote:
> Now it is safe to remove dependency from stop_machine() to patch code in
> ftrace.
>
> Signed-off-by: Andy Chiu <[e-mail]>
> Reviewed-by: Greentime Hu <[e-mail]>
> Reviewed-by: Zong Li <[e-mail]>
> ---
>  arch/riscv/kernel/ftrace.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c
> index d4bf0e5255f6..e7b8bf0a699b 100644
> --- a/arch/riscv/kernel/ftrace.c
> +++ b/arch/riscv/kernel/ftrace.c
> @@ -12,6 +12,11 @@
>  #include <asm/patch.h>
>
>  #ifdef CONFIG_DYNAMIC_FTRACE
> +void arch_ftrace_update_code(int command)
> +{
> +	ftrace_modify_all_code(command);
> +}
> +
>  int ftrace_arch_code_modify_prepare(void) __acquires(&text_mutex)
>  {
>  	mutex_lock(&text_mutex);

Reviewed-by: Palmer Dabbelt <[e-mail]>

Assuming we sort out the issues in #3.  IIUC we don't need to ensure the 
command is fully visible before returning, as the concurrent 
modification scheme doesn't flush the remote icaches and thus may skip 
instrumentation.

Patch

diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c
index d4bf0e5255f6..e7b8bf0a699b 100644
--- a/arch/riscv/kernel/ftrace.c
+++ b/arch/riscv/kernel/ftrace.c
@@ -12,6 +12,11 @@ 
 #include <asm/patch.h>
 
 #ifdef CONFIG_DYNAMIC_FTRACE
+void arch_ftrace_update_code(int command)
+{
+	ftrace_modify_all_code(command);
+}
+
 int ftrace_arch_code_modify_prepare(void) __acquires(&text_mutex)
 {
 	mutex_lock(&text_mutex);