[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20250112134510.eb9be6b8d9ec70693e4a9aa5@kernel.org>
Date: Sun, 12 Jan 2025 13:45:10 +0900
From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
To: Jiri Olsa <olsajiri@...il.com>
Cc: Alexei Starovoitov <alexei.starovoitov@...il.com>, Steven Rostedt
<rostedt@...dmis.org>, Florent Revest <revest@...omium.org>,
linux-trace-kernel@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
Martin KaFai Lau <martin.lau@...ux.dev>, bpf <bpf@...r.kernel.org>, Alexei
Starovoitov <ast@...nel.org>, Alan Maguire <alan.maguire@...cle.com>, Mark
Rutland <mark.rutland@....com>, linux-arch@...r.kernel.org
Subject: Re: [PATCH v22 00/20] tracing: fprobe: function_graph:
Multi-function graph and fprobe on fgraph
On Thu, 2 Jan 2025 14:20:58 +0100
Jiri Olsa <olsajiri@...il.com> wrote:
> On Thu, Dec 26, 2024 at 02:11:16PM +0900, Masami Hiramatsu (Google) wrote:
> > Hi,
> >
> > Here is the 22nd version of the series to re-implement the fprobe on
> > function-graph tracer. The previous version is;
> >
> > https://lore.kernel.org/all/173379652547.973433.2311391879173461183.stgit@devnote2/
> >
> > This version is rebased on v6.13-rc4 with fixes on [3/20] for x86-32 and
> > [5/20] for build error.
>
>
> hi,
> I ran the bench and I'm seeing native_sched_clock being used
> again kretprobe_multi bench:
>
> 5.85% bench [kernel.kallsyms] [k] native_sched_clock
> |
> ---native_sched_clock
> sched_clock
> |
> --5.83%--trace_clock_local
> ftrace_return_to_handler
> return_to_handler
> syscall
> bpf_prog_test_run_opts
> trigger_producer_batch
> start_thread
> __GI___clone3
>
> I recall we tried to fix that before with [1] change, but that replaced
> later with [2] changes
>
> When I remove the trace_clock_local call in __ftrace_return_to_handler
> than the kretprobe-multi gets much faster (see last block below), so it
> seems worth to make it optional
>
> there's some decrease in kprobe_multi benchmark compared to base numbers,
> which I'm not sure yet why, but other than that it seems ok
>
> base:
> kprobe : 12.873 ± 0.011M/s
> kprobe-multi : 13.088 ± 0.052M/s
> kretprobe : 6.339 ± 0.003M/s
> kretprobe-multi: 7.240 ± 0.002M/s
>
> fprobe_on_fgraph:
> kprobe : 12.816 ± 0.002M/s
> kprobe-multi : 12.126 ± 0.004M/s
> kretprobe : 6.305 ± 0.018M/s
> kretprobe-multi: 7.740 ± 0.003M/s
>
> removed native_sched_clock call:
> kprobe : 12.850 ± 0.006M/s
> kprobe-multi : 12.115 ± 0.006M/s
> kretprobe : 6.270 ± 0.017M/s
> kretprobe-multi: 9.190 ± 0.005M/s
Hmm, this still look good. Ok, let me move the trace_clock_local() to
the function_graph.
Thanks!
>
>
> happy new year ;-) thanks,
>
> jirka
>
>
> [1] https://lore.kernel.org/bpf/172615389864.133222.14452329708227900626.stgit@devnote2/
> [2] https://lore.kernel.org/all/20240914214805.779822616@goodmis.org/
>
> >
> > Overview
> > --------
> > This series rewrites the fprobe on this function-graph.
> > The purposes of this change are;
> >
> > 1) Remove dependency of the rethook from fprobe so that we can reduce
> > the return hook code and shadow stack.
> >
> > 2) Make 'ftrace_regs' the common trace interface for the function
> > boundary.
> >
> > 1) Currently we have 2(or 3) different function return hook codes,
> > the function-graph tracer and rethook (and legacy kretprobe).
> > But since this is redundant and needs double maintenance cost,
> > I would like to unify those. From the user's viewpoint, function-
> > graph tracer is very useful to grasp the execution path. For this
> > purpose, it is hard to use the rethook in the function-graph
> > tracer, but the opposite is possible. (Strictly speaking, kretprobe
> > can not use it because it requires 'pt_regs' for historical reasons.)
> >
> > 2) Now the fprobe provides the 'pt_regs' for its handler, but that is
> > wrong for the function entry and exit. Moreover, depending on the
> > architecture, there is no way to accurately reproduce 'pt_regs'
> > outside of interrupt or exception handlers. This means fprobe should
> > not use 'pt_regs' because it does not use such exceptions.
> > (Conversely, kprobe should use 'pt_regs' because it is an abstract
> > interface of the software breakpoint exception.)
> >
> > This series changes fprobe to use function-graph tracer for tracing
> > function entry and exit, instead of mixture of ftrace and rethook.
> > Unlike the rethook which is a per-task list of system-wide allocated
> > nodes, the function graph's ret_stack is a per-task shadow stack.
> > Thus it does not need to set 'nr_maxactive' (which is the number of
> > pre-allocated nodes).
> > Also the handlers will get the 'ftrace_regs' instead of 'pt_regs'.
> > Since eBPF mulit_kprobe/multi_kretprobe events still use 'pt_regs' as
> > their register interface, this changes it to convert 'ftrace_regs' to
> > 'pt_regs'. Of course this conversion makes an incomplete 'pt_regs',
> > so users must access only registers for function parameters or
> > return value.
> >
> > Design
> > ------
> > Instead of using ftrace's function entry hook directly, the new fprobe
> > is built on top of the function-graph's entry and return callbacks
> > with 'ftrace_regs'.
> >
> > Since the fprobe requires access to 'ftrace_regs', the architecture
> > must support CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS and
> > CONFIG_HAVE_FTRACE_GRAPH_FUNC, which enables to call function-graph
> > entry callback with 'ftrace_regs', and also
> > CONFIG_HAVE_FUNCTION_GRAPH_FREGS, which passes the ftrace_regs to
> > return_to_handler.
> >
> > All fprobes share a single function-graph ops (means shares a common
> > ftrace filter) similar to the kprobe-on-ftrace. This needs another
> > layer to find corresponding fprobe in the common function-graph
> > callbacks, but has much better scalability, since the number of
> > registered function-graph ops is limited.
> >
> > In the entry callback, the fprobe runs its entry_handler and saves the
> > address of 'fprobe' on the function-graph's shadow stack as data. The
> > return callback decodes the data to get the 'fprobe' address, and runs
> > the exit_handler.
> >
> > The fprobe introduces two hash-tables, one is for entry callback which
> > searches fprobes related to the given function address passed by entry
> > callback. The other is for a return callback which checks if the given
> > 'fprobe' data structure pointer is still valid. Note that it is
> > possible to unregister fprobe before the return callback runs. Thus
> > the address validation must be done before using it in the return
> > callback.
> >
> > Download
> > --------
> > This series can be applied against the v6.13-rc2 kernel.
> >
> > This series can also be found below branch.
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/mhiramat/linux.git/log/?h=topic/fprobe-on-fgraph
> >
> > Thank you,
> >
> > ---
> >
> > Masami Hiramatsu (Google) (19):
> > fgraph: Get ftrace recursion lock in function_graph_enter
> > fgraph: Pass ftrace_regs to entryfunc
> > fgraph: Replace fgraph_ret_regs with ftrace_regs
> > fgraph: Pass ftrace_regs to retfunc
> > fprobe: Use ftrace_regs in fprobe entry handler
> > fprobe: Use ftrace_regs in fprobe exit handler
> > tracing: Add ftrace_partial_regs() for converting ftrace_regs to pt_regs
> > tracing: Add ftrace_fill_perf_regs() for perf event
> > tracing/fprobe: Enable fprobe events with CONFIG_DYNAMIC_FTRACE_WITH_ARGS
> > bpf: Enable kprobe_multi feature if CONFIG_FPROBE is enabled
> > ftrace: Add CONFIG_HAVE_FTRACE_GRAPH_FUNC
> > fprobe: Rewrite fprobe on function-graph tracer
> > fprobe: Add fprobe_header encoding feature
> > tracing/fprobe: Remove nr_maxactive from fprobe
> > selftests: ftrace: Remove obsolate maxactive syntax check
> > selftests/ftrace: Add a test case for repeating register/unregister fprobe
> > Documentation: probes: Update fprobe on function-graph tracer
> > ftrace: Add ftrace_get_symaddr to convert fentry_ip to symaddr
> > bpf: Use ftrace_get_symaddr() for kprobe_multi probes
> >
> > Sven Schnelle (1):
> > s390/tracing: Enable HAVE_FTRACE_GRAPH_FUNC
> >
> >
> > Documentation/trace/fprobe.rst | 42 +
> > arch/arm64/Kconfig | 2
> > arch/arm64/include/asm/Kbuild | 1
> > arch/arm64/include/asm/ftrace.h | 51 +-
> > arch/arm64/kernel/asm-offsets.c | 12
> > arch/arm64/kernel/entry-ftrace.S | 32 +
> > arch/arm64/kernel/ftrace.c | 78 ++
> > arch/loongarch/Kconfig | 4
> > arch/loongarch/include/asm/fprobe.h | 12
> > arch/loongarch/include/asm/ftrace.h | 32 -
> > arch/loongarch/kernel/asm-offsets.c | 12
> > arch/loongarch/kernel/ftrace_dyn.c | 10
> > arch/loongarch/kernel/mcount.S | 17 -
> > arch/loongarch/kernel/mcount_dyn.S | 14
> > arch/powerpc/Kconfig | 1
> > arch/powerpc/include/asm/ftrace.h | 13
> > arch/powerpc/kernel/trace/ftrace.c | 8
> > arch/powerpc/kernel/trace/ftrace_64_pg.c | 16
> > arch/riscv/Kconfig | 3
> > arch/riscv/include/asm/Kbuild | 1
> > arch/riscv/include/asm/ftrace.h | 45 +
> > arch/riscv/kernel/ftrace.c | 17 -
> > arch/riscv/kernel/mcount.S | 24 -
> > arch/s390/Kconfig | 4
> > arch/s390/include/asm/fprobe.h | 10
> > arch/s390/include/asm/ftrace.h | 37 +
> > arch/s390/kernel/asm-offsets.c | 6
> > arch/s390/kernel/entry.h | 1
> > arch/s390/kernel/ftrace.c | 48 -
> > arch/s390/kernel/mcount.S | 23 -
> > arch/x86/Kconfig | 4
> > arch/x86/include/asm/Kbuild | 1
> > arch/x86/include/asm/ftrace.h | 54 +-
> > arch/x86/kernel/ftrace.c | 47 +
> > arch/x86/kernel/ftrace_32.S | 13
> > arch/x86/kernel/ftrace_64.S | 17 -
> > include/asm-generic/fprobe.h | 46 +
> > include/linux/fprobe.h | 62 +-
> > include/linux/ftrace.h | 116 +++
> > include/linux/ftrace_regs.h | 2
> > kernel/trace/Kconfig | 22 -
> > kernel/trace/bpf_trace.c | 28 +
> > kernel/trace/fgraph.c | 65 +-
> > kernel/trace/fprobe.c | 664 +++++++++++++++-----
> > kernel/trace/ftrace.c | 6
> > kernel/trace/trace.h | 6
> > kernel/trace/trace_fprobe.c | 146 ++--
> > kernel/trace/trace_functions_graph.c | 10
> > kernel/trace/trace_irqsoff.c | 6
> > kernel/trace/trace_probe_tmpl.h | 2
> > kernel/trace/trace_sched_wakeup.c | 6
> > kernel/trace/trace_selftest.c | 11
> > lib/test_fprobe.c | 51 --
> > samples/fprobe/fprobe_example.c | 4
> > .../test.d/dynevent/add_remove_fprobe_repeat.tc | 19 +
> > .../ftrace/test.d/dynevent/fprobe_syntax_errors.tc | 4
> > 56 files changed, 1318 insertions(+), 670 deletions(-)
> > create mode 100644 arch/loongarch/include/asm/fprobe.h
> > create mode 100644 arch/s390/include/asm/fprobe.h
> > create mode 100644 include/asm-generic/fprobe.h
> > create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/add_remove_fprobe_repeat.tc
> >
> > --
> > Masami Hiramatsu (Google) <mhiramat@...nel.org>
--
Masami Hiramatsu (Google) <mhiramat@...nel.org>
Powered by blists - more mailing lists