Messages in this thread | ![/](/images/icornerl.gif) | | Date | Fri, 18 Aug 2023 22:40:06 +0900 | From | Masami Hiramatsu (Google) <> | Subject | Re: [PATCH v3 0/8] bpf: fprobe: rethook: Use ftrace_regs instead of pt_regs |
| |
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,
-- Masami Hiramatsu (Google) <mhiramat@kernel.org>
| ![\](/images/icornerr.gif) |