| Message ID | 164937903547.1272679.7244379141135199176.stgit@devnote2 (mailing list archive) |
|---|---|
| Headers | show |
| Series | kprobes: rethook, ARM, arm64: Replace kretprobe trampoline with rethook | expand |
On Fri, Apr 08, 2022 at 09:50:35AM +0900, Masami Hiramatsu wrote: > Hi, Hi, > Here is the 2nd version of the series for replacing kretprobe trampoline > with rethook on ARM/arm64. I fixed some compiler warnings in this version. What tree is this based on? It doesn't cleanly apply atop v5.18-rc1: | [mark@lakrids:~/src/linux]% git am v2_20220408_mhiramat_kprobes_rethook_arm_arm64_replace_kretprobe_trampoline_with_rethook.mbx | Applying: ARM: unwind: Initialize the lr_addr field of unwind_ctrl_block | Applying: rethook,fprobe,kprobes: Check a failure in the rethook_hook() | Applying: ARM: rethook: Replace kretprobe trampoline with rethook | error: patch failed: arch/arm/kernel/stacktrace.c:66 | error: arch/arm/kernel/stacktrace.c: patch does not apply | Patch failed at 0003 ARM: rethook: Replace kretprobe trampoline with rethook | hint: Use 'git am --show-current-patch=diff' to see the failed patch | When you have resolved this problem, run "git am --continue". | If you prefer to skip this patch, run "git am --skip" instead. | To restore the original branch and stop patching, run "git am --abort". I've done a `git am -3` locally to make that work for now. > The previous version is here[1]; > > [1] https://lore.kernel.org/all/164915121498.982637.12787715964983738566.stgit@devnote2/T/#u > > This series includes a trivial bugfix for the arm unwinder to initialize > an internal data structure([1/4]). This is not critical for stack trace, > but required for rethook to find the LR register from the stack. > This also have an update for the rethook interface, which allows us to > check the rethook_hook() failure ([2/4]). This is also required for the > rethook on arm because unwinder is able to fail. > The rest of patches are replacing kretprobe trampoline with rethook on > ARM ([3/4]) and arm64 ([4/4]). Generally, the arm and arm64 bits go via different trees, and for unwinding the two are quite different. IIUC the dependency between the two is just because patch 2 changes the prototypes of some functions. Is that right? > Background: > > This rethook came from Jiri's request of multiple kprobe for bpf[2]. > He tried to solve an issue that starting bpf with multiple kprobe will > take a long time because bpf-kprobe will wait for RCU grace period for > sync rcu events. > > Jiri wanted to attach a single bpf handler to multiple kprobes and > he tried to introduce multiple-probe interface to kprobe. So I asked > him to use ftrace and kretprobe-like hook if it is only for the > function entry and exit, instead of adding ad-hoc interface > to kprobes. > For this purpose, I introduced the fprobe (kprobe like interface for > ftrace) with the rethook (this is a generic return hook feature for > fprobe exit handler)[3]. > > [2] https://lore.kernel.org/all/20220104080943.113249-1-jolsa@kernel.org/T/#u > [3] https://lore.kernel.org/all/164191321766.806991.7930388561276940676.stgit@devnote2/T/#u > > The rethook is basically same as the kretprobe trampoline. I just made > it decoupled from kprobes. Eventually, the all arch dependent kretprobe > trampolines will be replaced with the rethook trampoline instead of > cloning the code. > > When I port the rethook for all arch which supports kretprobe, the > legacy kretprobe specific code (which is for CONFIG_KRETPROBE_ON_RETHOOK=n) > will be removed eventually. > > BTW, the arm Clang support for rethook is for kretprobes only. fprobe > and ftrace seems not working with Clang yet. Do you mean that's an existing issue? Thanks, Mark. > > Thank you, > > --- > > Masami Hiramatsu (4): > ARM: unwind: Initialize the lr_addr field of unwind_ctrl_block > rethook,fprobe,kprobes: Check a failure in the rethook_hook() > ARM: rethook: Replace kretprobe trampoline with rethook > arm64: rethook: Replace kretprobe trampoline with rethook > > > arch/arm/Kconfig | 1 > arch/arm/include/asm/stacktrace.h | 5 + > arch/arm/kernel/stacktrace.c | 13 +-- > arch/arm/kernel/unwind.c | 1 > arch/arm/probes/Makefile | 1 > arch/arm/probes/kprobes/core.c | 62 ------------ > arch/arm/probes/rethook.c | 127 +++++++++++++++++++++++++ > arch/arm64/Kconfig | 1 > arch/arm64/include/asm/kprobes.h | 2 > arch/arm64/include/asm/stacktrace.h | 2 > arch/arm64/kernel/Makefile | 1 > arch/arm64/kernel/probes/Makefile | 1 > arch/arm64/kernel/probes/kprobes.c | 15 --- > arch/arm64/kernel/probes/kprobes_trampoline.S | 86 ----------------- > arch/arm64/kernel/rethook.c | 28 ++++++ > arch/arm64/kernel/rethook_trampoline.S | 87 +++++++++++++++++ > arch/arm64/kernel/stacktrace.c | 9 +- > arch/x86/kernel/rethook.c | 4 + > include/linux/rethook.h | 4 - > kernel/kprobes.c | 8 +- > kernel/trace/fprobe.c | 5 + > kernel/trace/rethook.c | 12 ++ > 22 files changed, 287 insertions(+), 188 deletions(-) > create mode 100644 arch/arm/probes/rethook.c > delete mode 100644 arch/arm64/kernel/probes/kprobes_trampoline.S > create mode 100644 arch/arm64/kernel/rethook.c > create mode 100644 arch/arm64/kernel/rethook_trampoline.S > > -- > Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
Hi Mark, Sorry for replying this late. On Tue, 12 Apr 2022 19:03:53 +0100 Mark Rutland <mark.rutland@arm.com> wrote: > On Fri, Apr 08, 2022 at 09:50:35AM +0900, Masami Hiramatsu wrote: > > Hi, > > Hi, > > > Here is the 2nd version of the series for replacing kretprobe trampoline > > with rethook on ARM/arm64. I fixed some compiler warnings in this version. > > What tree is this based on? It doesn't cleanly apply atop v5.18-rc1: I worked on bpf tree, but I'll try rebasing on the latest linus tree (or arm/arm64 tree) in the next version. > > | [mark@lakrids:~/src/linux]% git am v2_20220408_mhiramat_kprobes_rethook_arm_arm64_replace_kretprobe_trampoline_with_rethook.mbx > | Applying: ARM: unwind: Initialize the lr_addr field of unwind_ctrl_block > | Applying: rethook,fprobe,kprobes: Check a failure in the rethook_hook() > | Applying: ARM: rethook: Replace kretprobe trampoline with rethook > | error: patch failed: arch/arm/kernel/stacktrace.c:66 > | error: arch/arm/kernel/stacktrace.c: patch does not apply > | Patch failed at 0003 ARM: rethook: Replace kretprobe trampoline with rethook > | hint: Use 'git am --show-current-patch=diff' to see the failed patch > | When you have resolved this problem, run "git am --continue". > | If you prefer to skip this patch, run "git am --skip" instead. > | To restore the original branch and stop patching, run "git am --abort". > > I've done a `git am -3` locally to make that work for now. > > > The previous version is here[1]; > > > > [1] https://lore.kernel.org/all/164915121498.982637.12787715964983738566.stgit@devnote2/T/#u > > > > This series includes a trivial bugfix for the arm unwinder to initialize > > an internal data structure([1/4]). This is not critical for stack trace, > > but required for rethook to find the LR register from the stack. > > This also have an update for the rethook interface, which allows us to > > check the rethook_hook() failure ([2/4]). This is also required for the > > rethook on arm because unwinder is able to fail. > > The rest of patches are replacing kretprobe trampoline with rethook on > > ARM ([3/4]) and arm64 ([4/4]). > > Generally, the arm and arm64 bits go via different trees, and for unwinding the > two are quite different. OK, Should I split this into 2 series? Anyway, I'll send the first one for arm tree because this is a real bug. > > IIUC the dependency between the two is just because patch 2 changes the > prototypes of some functions. Is that right? Yes. I can push it one by one. What is the best way, would you think? If I split this into some series, I will; - Send [1/4] to arm tree as a bugfix. - Send [2/4] and [3/4] to arm tree as an enhancement. - Send [4/4] to arm64 when above 2 series are merged. Is that good for you? > > > > Background: > > > > This rethook came from Jiri's request of multiple kprobe for bpf[2]. > > He tried to solve an issue that starting bpf with multiple kprobe will > > take a long time because bpf-kprobe will wait for RCU grace period for > > sync rcu events. > > > > Jiri wanted to attach a single bpf handler to multiple kprobes and > > he tried to introduce multiple-probe interface to kprobe. So I asked > > him to use ftrace and kretprobe-like hook if it is only for the > > function entry and exit, instead of adding ad-hoc interface > > to kprobes. > > For this purpose, I introduced the fprobe (kprobe like interface for > > ftrace) with the rethook (this is a generic return hook feature for > > fprobe exit handler)[3]. > > > > [2] https://lore.kernel.org/all/20220104080943.113249-1-jolsa@kernel.org/T/#u > > [3] https://lore.kernel.org/all/164191321766.806991.7930388561276940676.stgit@devnote2/T/#u > > > > The rethook is basically same as the kretprobe trampoline. I just made > > it decoupled from kprobes. Eventually, the all arch dependent kretprobe > > trampolines will be replaced with the rethook trampoline instead of > > cloning the code. > > > > When I port the rethook for all arch which supports kretprobe, the > > legacy kretprobe specific code (which is for CONFIG_KRETPROBE_ON_RETHOOK=n) > > will be removed eventually. > > > > BTW, the arm Clang support for rethook is for kretprobes only. fprobe > > and ftrace seems not working with Clang yet. > > Do you mean that's an existing issue? Yes, but the above is not enough, Clang on arm and dynamic ftrace doesn't work. Perhaps, Clang on arm doesn't support -mrecord-mcount? Thank you, > > Thanks, > Mark. > > > > > Thank you, > > > > --- > > > > Masami Hiramatsu (4): > > ARM: unwind: Initialize the lr_addr field of unwind_ctrl_block > > rethook,fprobe,kprobes: Check a failure in the rethook_hook() > > ARM: rethook: Replace kretprobe trampoline with rethook > > arm64: rethook: Replace kretprobe trampoline with rethook > > > > > > arch/arm/Kconfig | 1 > > arch/arm/include/asm/stacktrace.h | 5 + > > arch/arm/kernel/stacktrace.c | 13 +-- > > arch/arm/kernel/unwind.c | 1 > > arch/arm/probes/Makefile | 1 > > arch/arm/probes/kprobes/core.c | 62 ------------ > > arch/arm/probes/rethook.c | 127 +++++++++++++++++++++++++ > > arch/arm64/Kconfig | 1 > > arch/arm64/include/asm/kprobes.h | 2 > > arch/arm64/include/asm/stacktrace.h | 2 > > arch/arm64/kernel/Makefile | 1 > > arch/arm64/kernel/probes/Makefile | 1 > > arch/arm64/kernel/probes/kprobes.c | 15 --- > > arch/arm64/kernel/probes/kprobes_trampoline.S | 86 ----------------- > > arch/arm64/kernel/rethook.c | 28 ++++++ > > arch/arm64/kernel/rethook_trampoline.S | 87 +++++++++++++++++ > > arch/arm64/kernel/stacktrace.c | 9 +- > > arch/x86/kernel/rethook.c | 4 + > > include/linux/rethook.h | 4 - > > kernel/kprobes.c | 8 +- > > kernel/trace/fprobe.c | 5 + > > kernel/trace/rethook.c | 12 ++ > > 22 files changed, 287 insertions(+), 188 deletions(-) > > create mode 100644 arch/arm/probes/rethook.c > > delete mode 100644 arch/arm64/kernel/probes/kprobes_trampoline.S > > create mode 100644 arch/arm64/kernel/rethook.c > > create mode 100644 arch/arm64/kernel/rethook_trampoline.S > > > > -- > > Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
Hi, Here is the 2nd version of the series for replacing kretprobe trampoline with rethook on ARM/arm64. I fixed some compiler warnings in this version. The previous version is here[1]; [1] https://lore.kernel.org/all/164915121498.982637.12787715964983738566.stgit@devnote2/T/#u This series includes a trivial bugfix for the arm unwinder to initialize an internal data structure([1/4]). This is not critical for stack trace, but required for rethook to find the LR register from the stack. This also have an update for the rethook interface, which allows us to check the rethook_hook() failure ([2/4]). This is also required for the rethook on arm because unwinder is able to fail. The rest of patches are replacing kretprobe trampoline with rethook on ARM ([3/4]) and arm64 ([4/4]). Background: This rethook came from Jiri's request of multiple kprobe for bpf[2]. He tried to solve an issue that starting bpf with multiple kprobe will take a long time because bpf-kprobe will wait for RCU grace period for sync rcu events. Jiri wanted to attach a single bpf handler to multiple kprobes and he tried to introduce multiple-probe interface to kprobe. So I asked him to use ftrace and kretprobe-like hook if it is only for the function entry and exit, instead of adding ad-hoc interface to kprobes. For this purpose, I introduced the fprobe (kprobe like interface for ftrace) with the rethook (this is a generic return hook feature for fprobe exit handler)[3]. [2] https://lore.kernel.org/all/20220104080943.113249-1-jolsa@kernel.org/T/#u [3] https://lore.kernel.org/all/164191321766.806991.7930388561276940676.stgit@devnote2/T/#u The rethook is basically same as the kretprobe trampoline. I just made it decoupled from kprobes. Eventually, the all arch dependent kretprobe trampolines will be replaced with the rethook trampoline instead of cloning the code. When I port the rethook for all arch which supports kretprobe, the legacy kretprobe specific code (which is for CONFIG_KRETPROBE_ON_RETHOOK=n) will be removed eventually. BTW, the arm Clang support for rethook is for kretprobes only. fprobe and ftrace seems not working with Clang yet. Thank you, --- Masami Hiramatsu (4): ARM: unwind: Initialize the lr_addr field of unwind_ctrl_block rethook,fprobe,kprobes: Check a failure in the rethook_hook() ARM: rethook: Replace kretprobe trampoline with rethook arm64: rethook: Replace kretprobe trampoline with rethook arch/arm/Kconfig | 1 arch/arm/include/asm/stacktrace.h | 5 + arch/arm/kernel/stacktrace.c | 13 +-- arch/arm/kernel/unwind.c | 1 arch/arm/probes/Makefile | 1 arch/arm/probes/kprobes/core.c | 62 ------------ arch/arm/probes/rethook.c | 127 +++++++++++++++++++++++++ arch/arm64/Kconfig | 1 arch/arm64/include/asm/kprobes.h | 2 arch/arm64/include/asm/stacktrace.h | 2 arch/arm64/kernel/Makefile | 1 arch/arm64/kernel/probes/Makefile | 1 arch/arm64/kernel/probes/kprobes.c | 15 --- arch/arm64/kernel/probes/kprobes_trampoline.S | 86 ----------------- arch/arm64/kernel/rethook.c | 28 ++++++ arch/arm64/kernel/rethook_trampoline.S | 87 +++++++++++++++++ arch/arm64/kernel/stacktrace.c | 9 +- arch/x86/kernel/rethook.c | 4 + include/linux/rethook.h | 4 - kernel/kprobes.c | 8 +- kernel/trace/fprobe.c | 5 + kernel/trace/rethook.c | 12 ++ 22 files changed, 287 insertions(+), 188 deletions(-) create mode 100644 arch/arm/probes/rethook.c delete mode 100644 arch/arm64/kernel/probes/kprobes_trampoline.S create mode 100644 arch/arm64/kernel/rethook.c create mode 100644 arch/arm64/kernel/rethook_trampoline.S -- Masami Hiramatsu (Linaro) <mhiramat@kernel.org>