BPF Archive mirror
 help / color / mirror / Atom feed
From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	 Florent Revest <revest@chromium.org>,
	linux-trace-kernel@vger.kernel.org,
	 LKML <linux-kernel@vger.kernel.org>,
	Martin KaFai Lau <martin.lau@linux.dev>,
	 bpf <bpf@vger.kernel.org>, Sven Schnelle <svens@linux.ibm.com>,
	 Alexei Starovoitov <ast@kernel.org>,
	Jiri Olsa <jolsa@kernel.org>,
	 Arnaldo Carvalho de Melo <acme@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	 Alan Maguire <alan.maguire@oracle.com>,
	Mark Rutland <mark.rutland@arm.com>,
	 Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>, Guo Ren <guoren@kernel.org>
Subject: Re: [PATCH v9 00/36] tracing: fprobe: function_graph: Multi-function graph and fprobe on fgraph
Date: Tue, 7 May 2024 14:04:08 -0700	[thread overview]
Message-ID: <CAEf4BzYb0LUKo_BUnd72qrBOtnbbHRS8SaDz0XcTx-DTgb2mVA@mail.gmail.com> (raw)
In-Reply-To: <20240502110610.412d54a0cf194293b82ee787@kernel.org>

On Wed, May 1, 2024 at 7:06 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> On Tue, 30 Apr 2024 09:29:40 -0700
> Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> > On Tue, Apr 30, 2024 at 6:32 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > >
> > > On Mon, 29 Apr 2024 13:25:04 -0700
> > > Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> > >
> > > > On Mon, Apr 29, 2024 at 6:51 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > > > >
> > > > > Hi Andrii,
> > > > >
> > > > > On Thu, 25 Apr 2024 13:31:53 -0700
> > > > > Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> > > > >
> > > > > > Hey Masami,
> > > > > >
> > > > > > I can't really review most of that code as I'm completely unfamiliar
> > > > > > with all those inner workings of fprobe/ftrace/function_graph. I left
> > > > > > a few comments where there were somewhat more obvious BPF-related
> > > > > > pieces.
> > > > > >
> > > > > > But I also did run our BPF benchmarks on probes/for-next as a baseline
> > > > > > and then with your series applied on top. Just to see if there are any
> > > > > > regressions. I think it will be a useful data point for you.
> > > > >
> > > > > Thanks for testing!
> > > > >
> > > > > >
> > > > > > You should be already familiar with the bench tool we have in BPF
> > > > > > selftests (I used it on some other patches for your tree).
> > > > >
> > > > > What patches we need?
> > > > >
> > > >
> > > > You mean for this `bench` tool? They are part of BPF selftests (under
> > > > tools/testing/selftests/bpf), you can build them by running:
> > > >
> > > > $ make RELEASE=1 -j$(nproc) bench
> > > >
> > > > After that you'll get a self-container `bench` binary, which has all
> > > > the self-contained benchmarks.
> > > >
> > > > You might also find a small script (benchs/run_bench_trigger.sh inside
> > > > BPF selftests directory) helpful, it collects final summary of the
> > > > benchmark run and optionally accepts a specific set of benchmarks. So
> > > > you can use it like this:
> > > >
> > > > $ benchs/run_bench_trigger.sh kprobe kprobe-multi
> > > > kprobe         :   18.731 ± 0.639M/s
> > > > kprobe-multi   :   23.938 ± 0.612M/s
> > > >
> > > > By default it will run a wider set of benchmarks (no uprobes, but a
> > > > bunch of extra fentry/fexit tests and stuff like this).
> > >
> > > origin:
> > > # benchs/run_bench_trigger.sh
> > > kretprobe :    1.329 ± 0.007M/s
> > > kretprobe-multi:    1.341 ± 0.004M/s
> > > # benchs/run_bench_trigger.sh
> > > kretprobe :    1.288 ± 0.014M/s
> > > kretprobe-multi:    1.365 ± 0.002M/s
> > > # benchs/run_bench_trigger.sh
> > > kretprobe :    1.329 ± 0.002M/s
> > > kretprobe-multi:    1.331 ± 0.011M/s
> > > # benchs/run_bench_trigger.sh
> > > kretprobe :    1.311 ± 0.003M/s
> > > kretprobe-multi:    1.318 ± 0.002M/s s
> > >
> > > patched:
> > >
> > > # benchs/run_bench_trigger.sh
> > > kretprobe :    1.274 ± 0.003M/s
> > > kretprobe-multi:    1.397 ± 0.002M/s
> > > # benchs/run_bench_trigger.sh
> > > kretprobe :    1.307 ± 0.002M/s
> > > kretprobe-multi:    1.406 ± 0.004M/s
> > > # benchs/run_bench_trigger.sh
> > > kretprobe :    1.279 ± 0.004M/s
> > > kretprobe-multi:    1.330 ± 0.014M/s
> > > # benchs/run_bench_trigger.sh
> > > kretprobe :    1.256 ± 0.010M/s
> > > kretprobe-multi:    1.412 ± 0.003M/s
> > >
> > > Hmm, in my case, it seems smaller differences (~3%?).
> > > I attached perf report results for those, but I don't see large difference.
> >
> > I ran my benchmarks on bare metal machine (and quite powerful at that,
> > you can see my numbers are almost 10x of yours), with mitigations
> > disabled, no retpolines, etc. If you have any of those mitigations it
> > might result in smaller differences, probably. If you are running
> > inside QEMU/VM, the results might differ significantly as well.
>
> I ran it on my bare metal machines again, but could not find any difference
> between them. But I think I enabled intel mitigations on, so it might make
> a difference from your result.
>
> Can you run the benchmark with perf record? If there is such differences,
> there should be recorded.

I can, yes, will try to do this week, I'm just trying to keep up with
the rest of the stuff on my plate and haven't found yet time to do
this. I'll get back to you (and I'll use the latest version of your
patch set, of course).

> e.g.
>
> # perf record -g -o perf.data-kretprobe-nopatch-raw-bpf -- bench -w2 -d5 -a trig-kretprobe
> # perf report -G -i perf.data-kretprobe-nopatch-raw-bpf -k $VMLINUX --stdio > perf-out-kretprobe-nopatch-raw-bpf
>
> I attached the results in my side.
> The interesting point is, the functions int the result are not touched by
> this series. Thus there may be another reason if you see the kretprobe
> regression.
>
> Thank you,
> --
> Masami Hiramatsu (Google) <mhiramat@kernel.org>

      reply	other threads:[~2024-05-07 21:04 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-15 12:48 [PATCH v9 00/36] tracing: fprobe: function_graph: Multi-function graph and fprobe on fgraph Masami Hiramatsu (Google)
2024-04-15 12:49 ` [PATCH v9 01/36] tracing: Add a comment about ftrace_regs definition Masami Hiramatsu (Google)
2024-04-24 12:23   ` Florent Revest
2024-04-24 13:19     ` Florent Revest
2024-04-24 14:31       ` Masami Hiramatsu
2024-04-15 12:49 ` [PATCH v9 02/36] tracing: Rename ftrace_regs_return_value to ftrace_regs_get_return_value Masami Hiramatsu (Google)
2024-04-15 12:49 ` [PATCH v9 03/36] x86: tracing: Add ftrace_regs definition in the header Masami Hiramatsu (Google)
2024-04-15 12:49 ` [PATCH v9 04/36] function_graph: Convert ret_stack to a series of longs Masami Hiramatsu (Google)
2024-04-15 12:49 ` [PATCH v9 05/36] fgraph: Use BUILD_BUG_ON() to make sure we have structures divisible by long Masami Hiramatsu (Google)
2024-04-15 12:50 ` [PATCH v9 06/36] function_graph: Add an array structure that will allow multiple callbacks Masami Hiramatsu (Google)
2024-04-15 12:50 ` [PATCH v9 07/36] function_graph: Allow multiple users to attach to function graph Masami Hiramatsu (Google)
2024-04-20  3:52   ` Steven Rostedt
2024-04-20  8:56     ` Masami Hiramatsu
2024-04-15 12:50 ` [PATCH v9 08/36] function_graph: Remove logic around ftrace_graph_entry and return Masami Hiramatsu (Google)
2024-04-15 12:50 ` [PATCH v9 09/36] ftrace/function_graph: Pass fgraph_ops to function graph callbacks Masami Hiramatsu (Google)
2024-04-15 12:50 ` [PATCH v9 10/36] ftrace: Allow function_graph tracer to be enabled in instances Masami Hiramatsu (Google)
2024-04-15 12:51 ` [PATCH v9 11/36] ftrace: Allow ftrace startup flags exist without dynamic ftrace Masami Hiramatsu (Google)
2024-04-15 12:51 ` [PATCH v9 12/36] function_graph: Have the instances use their own ftrace_ops for filtering Masami Hiramatsu (Google)
2024-04-15 12:51 ` [PATCH v9 13/36] function_graph: Use a simple LRU for fgraph_array index number Masami Hiramatsu (Google)
2024-04-15 12:51 ` [PATCH v9 14/36] function_graph: Add "task variables" per task for fgraph_ops Masami Hiramatsu (Google)
2024-04-15 12:51 ` [PATCH v9 15/36] function_graph: Move set_graph_function tests to shadow stack global var Masami Hiramatsu (Google)
2024-04-15 12:52 ` [PATCH v9 16/36] function_graph: Move graph depth stored data " Masami Hiramatsu (Google)
2024-04-15 12:52 ` [PATCH v9 17/36] function_graph: Move graph notrace bit " Masami Hiramatsu (Google)
2024-04-15 12:52 ` [PATCH v9 18/36] function_graph: Implement fgraph_reserve_data() and fgraph_retrieve_data() Masami Hiramatsu (Google)
2024-04-15 12:52 ` [PATCH v9 19/36] function_graph: Add selftest for passing local variables Masami Hiramatsu (Google)
2024-04-15 12:52 ` [PATCH v9 20/36] ftrace: Add multiple fgraph storage selftest Masami Hiramatsu (Google)
2024-04-15 12:53 ` [PATCH v9 21/36] function_graph: Pass ftrace_regs to entryfunc Masami Hiramatsu (Google)
2024-04-15 12:53 ` [PATCH v9 22/36] function_graph: Replace fgraph_ret_regs with ftrace_regs Masami Hiramatsu (Google)
2024-04-15 12:53 ` [PATCH v9 23/36] function_graph: Pass ftrace_regs to retfunc Masami Hiramatsu (Google)
2024-04-15 12:53 ` [PATCH v9 24/36] fprobe: Use ftrace_regs in fprobe entry handler Masami Hiramatsu (Google)
2024-04-15 12:53 ` [PATCH v9 25/36] fprobe: Use ftrace_regs in fprobe exit handler Masami Hiramatsu (Google)
2024-04-15 12:54 ` [PATCH v9 26/36] tracing: Add ftrace_partial_regs() for converting ftrace_regs to pt_regs Masami Hiramatsu (Google)
2024-04-15 12:54 ` [PATCH v9 27/36] tracing: Add ftrace_fill_perf_regs() for perf event Masami Hiramatsu (Google)
2024-04-15 12:54 ` [PATCH v9 28/36] tracing/fprobe: Enable fprobe events with CONFIG_DYNAMIC_FTRACE_WITH_ARGS Masami Hiramatsu (Google)
2024-04-15 12:54 ` [PATCH v9 29/36] bpf: Enable kprobe_multi feature if CONFIG_FPROBE is enabled Masami Hiramatsu (Google)
2024-04-25 20:09   ` Andrii Nakryiko
2024-04-29 14:57     ` Masami Hiramatsu
2024-04-15 12:54 ` [PATCH v9 30/36] ftrace: Add CONFIG_HAVE_FTRACE_GRAPH_FUNC Masami Hiramatsu (Google)
2024-04-15 12:55 ` [PATCH v9 31/36] fprobe: Rewrite fprobe on function-graph tracer Masami Hiramatsu (Google)
2024-04-15 12:55 ` [PATCH v9 32/36] tracing/fprobe: Remove nr_maxactive from fprobe Masami Hiramatsu (Google)
2024-04-15 12:55 ` [PATCH v9 33/36] selftests: ftrace: Remove obsolate maxactive syntax check Masami Hiramatsu (Google)
2024-04-15 12:55 ` [PATCH v9 34/36] selftests/ftrace: Add a test case for repeating register/unregister fprobe Masami Hiramatsu (Google)
2024-04-15 12:55 ` [PATCH v9 35/36] Documentation: probes: Update fprobe on function-graph tracer Masami Hiramatsu (Google)
2024-04-15 12:55 ` [PATCH v9 36/36] fgraph: Skip recording calltime/rettime if it is not nneeded Masami Hiramatsu (Google)
2024-04-25 20:15   ` Andrii Nakryiko
2024-04-29 14:56     ` Masami Hiramatsu
2024-04-19  5:36 ` [PATCH v9 00/36] tracing: fprobe: function_graph: Multi-function graph and fprobe on fgraph Masami Hiramatsu
2024-04-19  8:01   ` Steven Rostedt
2024-04-24 13:35 ` Florent Revest
2024-04-25 15:10   ` Masami Hiramatsu
2024-04-25 20:31 ` Andrii Nakryiko
2024-04-28 23:25   ` Steven Rostedt
2024-04-29 20:28     ` Andrii Nakryiko
2024-04-29 13:51   ` Masami Hiramatsu
2024-04-29 20:25     ` Andrii Nakryiko
2024-04-30 13:32       ` Masami Hiramatsu
2024-04-30 16:29         ` Andrii Nakryiko
2024-05-02  2:06           ` Masami Hiramatsu
2024-05-07 21:04             ` Andrii Nakryiko [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAEf4BzYb0LUKo_BUnd72qrBOtnbbHRS8SaDz0XcTx-DTgb2mVA@mail.gmail.com \
    --to=andrii.nakryiko@gmail.com \
    --cc=acme@kernel.org \
    --cc=alan.maguire@oracle.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=guoren@kernel.org \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=martin.lau@linux.dev \
    --cc=mhiramat@kernel.org \
    --cc=peterz@infradead.org \
    --cc=revest@chromium.org \
    --cc=rostedt@goodmis.org \
    --cc=svens@linux.ibm.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).