Linux Trace Kernel
 help / color / mirror / Atom feed
From: Jiri Olsa <olsajiri@gmail.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	bpf@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
	Martin KaFai Lau <martin.lau@linux.dev>,
	Eduard Zingerman <eddyz87@gmail.com>,
	Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
	Menglong Dong <menglong8.dong@gmail.com>,
	Steven Rostedt <rostedt@kernel.org>
Subject: Re: [PATCHv8 bpf-next 28/29] selftests/bpf: Add tracing multi attach benchmark test
Date: Mon, 8 Jun 2026 10:25:30 +0200	[thread overview]
Message-ID: <aiZ8euKIuw_9qaw6@krava> (raw)
In-Reply-To: <DJ30RPYRXESZ.2P3LP98X55VMG@gmail.com>

On Sun, Jun 07, 2026 at 11:13:32AM -0700, Alexei Starovoitov wrote:
> On Sat Jun 6, 2026 at 5:39 AM PDT, Jiri Olsa wrote:
> > Adding benchmark test that attaches to (almost) all allowed tracing
> > functions and display attach/detach times.
> >
> >   # ./test_progs -t tracing_multi_bench_attach -v
> >   bpf_testmod.ko is already unloaded.
> >   Loading bpf_testmod.ko...
> >   Successfully loaded bpf_testmod.ko.
> >   serial_test_tracing_multi_bench_attach:PASS:btf__load_vmlinux_btf 0 nsec
> >   serial_test_tracing_multi_bench_attach:PASS:tracing_multi_bench__open_and_load 0 nsec
> >   serial_test_tracing_multi_bench_attach:PASS:get_syms 0 nsec
> >   serial_test_tracing_multi_bench_attach:PASS:bpf_program__attach_tracing_multi 0 nsec
> >   serial_test_tracing_multi_bench_attach: found 51186 functions
> >   serial_test_tracing_multi_bench_attach: attached in   1.295s
> >   serial_test_tracing_multi_bench_attach: detached in   0.243s
> 
> ...
> 
> > +	if (!ASSERT_OK(bpf_get_ksyms(&ksyms, true), "get_syms"))
> > +		goto cleanup;
> > +
> > +	/* Get all ftrace 'safe' symbols.. */
> > +	for (i = 0; i < ksyms->filtered_cnt; i++) {
> > +		if (!tsearch(&ksyms->filtered_syms[i], &root, compare)) {
> > +			ASSERT_FAIL("tsearch failed");
> > +			goto cleanup;
> > +		}
> > +	}
> > +
> > +	/* ..and filter them through BTF and btf_type_is_traceable_func. */
> > +	nr = btf__type_cnt(btf);
> > +	for (type_id = 1; type_id < nr; type_id++) {
> > +		const struct btf_type *type;
> > +		const char *str;
> > +
> > +		type = btf__type_by_id(btf, type_id);
> > +		if (!type)
> > +			break;
> > +
> > +		if (BTF_INFO_KIND(type->info) != BTF_KIND_FUNC)
> > +			continue;
> > +
> > +		str = btf__name_by_offset(btf, type->name_off);
> > +		if (!str)
> > +			break;
> > +
> > +		if (!tfind(&str, &root, compare))
> > +			continue;
> > +
> > +		if (!btf_type_is_traceable_func(btf, type))
> > +			continue;
> > +
> > +		err = libbpf_ensure_mem((void **) &ids, &cap, sizeof(*ids), cnt + 1);
> > +		if (err)
> > +			goto cleanup;
> > +
> > +		ids[cnt++] = type_id;
> > +	}
> 
> This filtering wasn't enough.
> I've added removal of duplicates here while applying:
> 
> +       /*
> +        * Collect names that are not unique in kallsyms. The kernel resolves a
> +        * tracing-multi BTF id to an address with kallsyms_lookup_name(), which
> +        * returns the first symbol of that name. For a duplicate name that may
> +        * be a different (non-ftrace-able) instance than the ftrace-able one in
> +        * available_filter_functions, so attaching to it by BTF id fails with
> +        * -ENOENT (e.g. t_start/t_next/t_stop). ksyms->syms is sorted by name,
> +        * so equal names are adjacent.
> +        */
> +       for (i = 1; i < ksyms->sym_cnt; i++) {
> +               if (strcmp(ksyms->syms[i].name, ksyms->syms[i - 1].name))
> +                       continue;
> +               if (!tsearch(&ksyms->syms[i].name, &dups, compare)) {
> +                       ASSERT_FAIL("tsearch failed");
> +                       goto cleanup;
> +               }
> +       }
> 
> 
> +               /* Skip names that are not unique in kallsyms, see above. */
> +               if (tfind(&str, &dups, compare))
> +                       continue;
> 
> 
> As claude explains it:
> ----
> 1. The kernel attaches tracing_multi by BTF id. To get an address it resolves
> the BTF function name via kallsyms_lookup_name(tname) and requires
> ftrace_location(addr) — kernel/bpf/verifier.c:19380:
> addr = kallsyms_lookup_name(tname);
> ...
> if (!addr || !ftrace_location(addr))
>         return -ENOENT;
> 2. t_start/t_next/t_stop each have 5 instances in this kernel. Only one is
> ftrace-able — the copies in kernel/trace/* are built notrace (ftrace's Makefile
> strips -pg), so only the unrelated copy is in available_filter_functions:
> 3. kallsyms_lookup_name() returns the lowest-address instance among equal names
> (exact strcmp, lowest seq). That instance has no fentry → ftrace_location()
> returns 0 → -ENOENT, which aborts the whole all-or-nothing bench attach.
> 
> Why the bench includes them: it intersects BTF FUNC names with
> available_filter_functions names. Since some t_start is ftrace-able, the name
> passes the filter — but the kernel resolves the wrong (non-ftrace-able)
> t_start. The author's kernel apparently had the ftrace-able copy at the lowest
> address, so it passed there.
> 
> This is a pre-existing limitation, not multi-specific: single fentry attach by
> BTF id uses the same kallsyms_lookup_name(tname) path (verifier.c:19120) — you
> can't reliably fentry-attach to any duplicate-named function on this kernel
> either.
> ----

strange I never triggered that.. but makes sense

> 
> Maybe we should adjust bpf_get_ksyms() instead. Not sure.

the only other user is test_kprobe_multi_bench_attach which does
not care about this, so I think at this point keeping this in the
serial_test_tracing_multi_bench_attach is enough for now

thanks,
jirka

  reply	other threads:[~2026-06-08  8:25 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-06 12:39 [PATCHv8 bpf-next 00/29] bpf: tracing_multi link Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 01/29] ftrace: Add ftrace_hash_count function Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 02/29] ftrace: Add ftrace_hash_remove function Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 03/29] ftrace: Add add_ftrace_hash_entry function Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 04/29] bpf: Use mutex lock pool for bpf trampolines Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 05/29] bpf: Add struct bpf_trampoline_ops object Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 06/29] bpf: Move trampoline image setup into bpf_trampoline_ops callbacks Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 07/29] bpf: Add bpf_trampoline_add/remove_prog functions Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 08/29] bpf: Add struct bpf_tramp_node object Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 09/29] bpf: Factor fsession link to use struct bpf_tramp_node Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 10/29] bpf: Add multi tracing attach types Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 11/29] bpf: Move sleepable verification code to btf_id_allow_sleepable Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 12/29] bpf: Add bpf_trampoline_multi_attach/detach functions Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 13/29] bpf: Add support for tracing multi link Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 14/29] bpf: Add support for tracing_multi link cookies Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 15/29] bpf: Add support for tracing_multi link session Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 16/29] bpf: Add support for tracing_multi link fdinfo Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 17/29] libbpf: Add bpf_object_cleanup_btf function Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 18/29] libbpf: Add bpf_link_create support for tracing_multi link Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 19/29] libbpf: Add btf_type_is_traceable_func function Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 20/29] libbpf: Add support to create tracing multi link Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 21/29] selftests/bpf: Add tracing multi skel/pattern/ids attach tests Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 22/29] selftests/bpf: Add tracing multi skel/pattern/ids module " Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 23/29] selftests/bpf: Add tracing multi intersect tests Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 24/29] selftests/bpf: Add tracing multi cookies test Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 25/29] selftests/bpf: Add tracing multi session test Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 26/29] selftests/bpf: Add tracing multi attach fails test Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 27/29] selftests/bpf: Add tracing multi verifier " Jiri Olsa
2026-06-06 12:39 ` [PATCHv8 bpf-next 28/29] selftests/bpf: Add tracing multi attach benchmark test Jiri Olsa
2026-06-07 18:13   ` Alexei Starovoitov
2026-06-08  8:25     ` Jiri Olsa [this message]
2026-06-06 12:39 ` [PATCHv8 bpf-next 29/29] selftests/bpf: Add tracing multi attach rollback tests Jiri Olsa
2026-06-07 18:20 ` [PATCHv8 bpf-next 00/29] bpf: tracing_multi link patchwork-bot+netdevbpf

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=aiZ8euKIuw_9qaw6@krava \
    --to=olsajiri@gmail.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=menglong8.dong@gmail.com \
    --cc=rostedt@kernel.org \
    --cc=songliubraving@fb.com \
    --cc=yhs@fb.com \
    /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