| Message ID | 169181859570.505132.10136520092011157898.stgit@devnote2 (mailing list archive) |
|---|---|
| Headers | show |
| Series | bpf: fprobe: rethook: Use ftrace_regs instead of pt_regs | expand |
On Sat, Aug 12, 2023 at 7:36 AM Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote: > > Hi, > > Here is the 3rd version of RFC series to use ftrace_regs instead of pt_regs. > The previous version is here; > > https://lore.kernel.org/all/169139090386.324433.6412259486776991296.stgit@devnote2/ > > This also includes the generic part and minimum modifications of arch > dependent code. (e.g. not including rethook for arm64.) I think that one aspect that's missing from the discussion (and maybe the series) so far is plans to actually save partial registers in the existing rethook trampolines. For now the series makes everything called by the rethook trampolines handle the possibility of having a sparse ftrace_regs but the rethook trampolines still save full ftrace_regs. I think that to rip the full benefits of this series, we should have the rethook trampolines save the equivalent ftrace_regs as the light "args" version of the ftrace trampoline.
On Thu, 17 Aug 2023 10:57:13 +0200 Florent Revest <revest@chromium.org> wrote: > On Sat, Aug 12, 2023 at 7:36 AM Masami Hiramatsu (Google) > <mhiramat@kernel.org> wrote: > > > > Hi, > > > > Here is the 3rd version of RFC series to use ftrace_regs instead of pt_regs. > > The previous version is here; > > > > https://lore.kernel.org/all/169139090386.324433.6412259486776991296.stgit@devnote2/ > > > > This also includes the generic part and minimum modifications of arch > > dependent code. (e.g. not including rethook for arm64.) > > I think that one aspect that's missing from the discussion (and maybe > the series) so far is plans to actually save partial registers in the > existing rethook trampolines. Yes, it is arch-dependent part. We have to recheck what registers are required for the rethook, and that is saved correctly on partial pt_regs on each architecture. > For now the series makes everything called by the rethook trampolines > handle the possibility of having a sparse ftrace_regs but the rethook > trampolines still save full ftrace_regs. I think that to rip the full > benefits of this series, we should have the rethook trampolines save > the equivalent ftrace_regs as the light "args" version of the ftrace > trampoline. I think this part depends on the architecture implementation, but yes. Arm64 can *add* the rethook implementation but not enable KRETPROBE_ON_RETHOOK. (do not remove kretprobe trampoline) For this perpose, we need HAVE_RETHOOK_WITH_REGS; config KRETPROBE_ON_RETHOOK def_bool y - depends on HAVE_RETHOOK + depends on HAVE_RETHOOK_WITH_REGS depends on KRETPROBES select RETHOOK So there will be pt_regs rethook and ftrace_regs (partial regs) rethook. I would like to replace rethook's pt_regs with ftrace_regs too. However the most problematic part is kretprobe. If CONFIG_KRETPROBE_ON_RETHOOK=y, the rethook must use pt_regs instead of ftrace_regs for API compatibility. But it makes hard to integrate the rethook and function-graph trace return hook. (I will discuss this in LPC) Thank you,
Hi, Here is the 3rd version of RFC series to use ftrace_regs instead of pt_regs. The previous version is here; https://lore.kernel.org/all/169139090386.324433.6412259486776991296.stgit@devnote2/ This also includes the generic part and minimum modifications of arch dependent code. (e.g. not including rethook for arm64.) This series is based on the discussion at https://lore.kernel.org/all/20230801112036.0d4ee60d@gandalf.local.home/ This version added some documentation updates, fix typos, fix some build errors on arm64 and s390, rename config name to HAVE_PT_REGS_TO_FTRACE_REGS_CAST, Use FTRACE_OPS_FL_SAVE_ARGS in fprobe, add ftrace_regs_get_kernel_stack_nth() and add check HAVE_REGS_AND_STACK_ACCESS_API=y for ftrace_regs APIs. - Document fix for the current fprobe callback prototype - Simply replace pt_regs in fprobe_entry_handler with ftrace_regs. - Expose ftrace_regs even if CONFIG_FUNCTION_TRACER=n. - Replace pt_regs in rethook and fprobe_exit_handler with ftrace_regs. This introduce a new HAVE_PT_REGS_TO_FTRACE_REGS_CAST which means ftrace_regs is just a wrapper of pt_regs (except for arm64, other architectures do this) - Update fprobe-events to use ftrace_regs natively. - Introduce ftrace_partial_regs(). (This changes ARM64 which needs a custom implementation) - Update bpf multi-kprobe handler use ftrace_partial_regs(). - Update document to new fprobe callbacks. This series can also be found below branch. https://git.kernel.org/pub/scm/linux/kernel/git/mhiramat/linux.git/log/?h=topic/fprobe-ftrace-regs Thank you, --- Masami Hiramatsu (Google) (8): Documentation: probes: Add a new ret_ip callback parameter fprobe: Use fprobe_regs in fprobe entry handler tracing: Expose ftrace_regs regardless of CONFIG_FUNCTION_TRACER fprobe: rethook: Use ftrace_regs in fprobe exit handler and rethook tracing/fprobe: Enable fprobe events with CONFIG_DYNAMIC_FTRACE_WITH_ARGS ftrace: Add ftrace_partial_regs() for converting ftrace_regs to pt_regs bpf: Enable kprobe_multi feature if CONFIG_FPROBE is enabled Documentations: probes: Update fprobe document to use ftrace_regs Documentation/trace/fprobe.rst | 18 +++++--- arch/Kconfig | 1 arch/arm64/include/asm/ftrace.h | 11 +++++ arch/loongarch/Kconfig | 1 arch/s390/Kconfig | 1 arch/s390/include/asm/ftrace.h | 4 ++ arch/x86/Kconfig | 1 arch/x86/kernel/rethook.c | 13 +++--- include/linux/fprobe.h | 4 +- include/linux/ftrace.h | 83 +++++++++++++++++++++++++++++---------- include/linux/rethook.h | 11 +++-- kernel/kprobes.c | 10 ++++- kernel/trace/Kconfig | 9 ++++ kernel/trace/bpf_trace.c | 14 ++++--- kernel/trace/fprobe.c | 10 ++--- kernel/trace/rethook.c | 16 ++++---- kernel/trace/trace_fprobe.c | 69 +++++++++++++++++++------------- kernel/trace/trace_probe_tmpl.h | 2 - lib/test_fprobe.c | 10 ++--- samples/fprobe/fprobe_example.c | 4 +- 20 files changed, 191 insertions(+), 101 deletions(-) -- Masami Hiramatsu (Google) <mhiramat@kernel.org>