On Thu, 16 May 2024 09:46:46 -0400
Abhishek Dubey <adu...@linux.ibm.com> wrote:
> This is an adaptation of commit f3a112c0c40d ("x86,rethook,kprobes:
> Replace kretprobe with rethook on x86") to Power.
>
> Replaces the kretprobe code with rethook on Power. With this patch,
> kretprobe on Power uses the rethook instead of kretprobe specific
> trampoline code.
>
> Reference to other archs:
> commit b57c2f124098 ("riscv: add riscv rethook implementation")
> commit 7b0a096436c2 ("LoongArch: Replace kretprobe with rethook")
>
Hi Abhishek,
Thanks for applying rethook, it looks good. I have comments below.
> diff --git a/arch/powerpc/kernel/stacktrace.c
> b/arch/powerpc/kernel/stacktrace.c
> index e6a958a5da27..6de912cf198c 100644
> --- a/arch/powerpc/kernel/stacktrace.c
> +++ b/arch/powerpc/kernel/stacktrace.c
> @@ -21,6 +21,7 @@
> #include <asm/processor.h>
> #include <linux/ftrace.h>
> #include <asm/kprobes.h>
> +#include <linux/rethook.h>
>
> #include <asm/paca.h>
>
> @@ -133,14 +134,13 @@ int __no_sanitize_address
> arch_stack_walk_reliable(stack_trace_consume_fn consum
> * arch-dependent code, they are generic.
> */
> ip = ftrace_graph_ret_addr(task, &graph_idx, ip, stack);
> -#ifdef CONFIG_KPROBES
This still needs to check CONFIG_RETHOOK.
> +
> /*
> * Mark stacktraces with kretprobed functions on them
> * as unreliable.
> */
> - if (ip == (unsigned long)__kretprobe_trampoline)
> + if (ip == (unsigned long)arch_rethook_trampoline)
> return -EINVAL;
> -#endif
>
> if (!consume_entry(cookie, ip))
> return -EINVAL;
> diff --git a/include/linux/rethook.h b/include/linux/rethook.h
> index ba60962805f6..9f2fb6abdc60 100644
> --- a/include/linux/rethook.h
> +++ b/include/linux/rethook.h
> @@ -65,7 +65,6 @@ void rethook_recycle(struct rethook_node *node);
> void rethook_hook(struct rethook_node *node, struct pt_regs *regs, bool
> mcount);
> unsigned long rethook_find_ret_addr(struct task_struct *tsk, unsigned long
> frame,
> struct llist_node **cur);
> -
nit: removed unrelated line.
> /* Arch dependent code must implement arch_* and trampoline code */
> void arch_rethook_prepare(struct rethook_node *node, struct pt_regs *regs,
> bool mcount);
> void arch_rethook_trampoline(void);
> --
> 2.44.0
>
Thank you,
--
Masami Hiramatsu (Google) <mhira...@kernel.org>
