Thread (26 messages) 26 messages, 3 authors, 2023-08-18

Re: [PATCH v3 0/8] bpf: fprobe: rethook: Use ftrace_regs instead of pt_regs

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Date: 2023-08-18 13:41:02
Also in: bpf, lkml

On Thu, 17 Aug 2023 10:57:13 +0200
Florent Revest [off-list ref] wrote:
On Sat, Aug 12, 2023 at 7:36 AM Masami Hiramatsu (Google)
[off-list ref] wrote:
quoted
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/ (local)

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) [off-list ref]
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help