All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <olsajiri@gmail.com>
To: Andrey Grodzovsky <andrey.grodzovsky@crowdstrike.com>
Cc: bot+bpf-ci@kernel.org, yonghong.song@linux.dev,
	bpf@vger.kernel.org, linux-open-source@crowdstrike.com,
	ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
	rostedt@goodmis.org, linux-trace-kernel@vger.kernel.org,
	martin.lau@kernel.org, eddyz87@gmail.com, clm@meta.com,
	ihor.solodrai@linux.dev
Subject: Re: [External] Re: [RFC PATCH bpf-next v3 1/3] libbpf: Optimize kprobe.session attachment for exact function names
Date: Mon, 2 Mar 2026 11:58:50 +0100	[thread overview]
Message-ID: <aaVtarHiOYPfe9Tu@krava> (raw)
In-Reply-To: <CAOu3gNgRpMd9poZYibhGg+UfQXiHpB3ucnhvghoT0TF1rFfmPQ@mail.gmail.com>

On Fri, Feb 27, 2026 at 05:28:05PM -0500, Andrey Grodzovsky wrote:
> On Fri, Feb 27, 2026 at 4:18 PM <bot+bpf-ci@kernel.org> wrote:
> >
> > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > > --- a/tools/lib/bpf/libbpf.c
> > > +++ b/tools/lib/bpf/libbpf.c
> > > @@ -12041,7 +12041,15 @@ struct kprobe_multi_resolve res = {
> > >       if (addrs && syms)
> > >               return libbpf_err_ptr(-EINVAL);
> > >
> > > -     if (pattern) {
> > > +     /*
> > > +      * Exact function name (no wildcards): bypass kallsyms parsing
> > > +      * and pass the symbol directly to the kernel via syms[] array.
> > > +      * The kernel's ftrace_lookup_symbols() resolves it efficiently.
> > > +      */
> > > +     if (pattern && !strpbrk(pattern, "*?")) {
> > > +             syms = &pattern;
> > > +             cnt = 1;
> > > +     } else if (pattern) {
> >
> > When the fast path is taken here, unique_match is never checked.
> > The unique_match validation is inside the "else if (pattern)"
> > branch:
> >
> >     if (unique_match && res.cnt != 1) {
> >         pr_warn("prog '%s': failed to find a unique match ...");
> >         err = -EINVAL;
> >         goto error;
> >     }
> >
> > Before this change, a caller passing an exact function name with
> > unique_match = true would go through kallsyms parsing, which could
> > find multiple addresses for same-named static functions across
> > different translation units, and the unique_match check would
> > correctly return an error.
> >
> > With the fast path, the name goes directly to the kernel's
> > ftrace_lookup_symbols(), which resolves only the first match via
> > kallsyms_callback() skipping duplicates:
> >
> >     if (args->addrs[idx])
> >         return 0;
> >
> > Should the fast path condition also check unique_match, for
> > example:
> >
> >     if (pattern && !strpbrk(pattern, "*?") && !unique_match) {
> >
> > This would preserve the optimization for the common case while
> > falling back to the slow path when unique_match is explicitly
> > requested.
> >
> 
> I am not sure this makes sense, according to the original patchset [1]
> this flag was specifically tailored for patterns with wildcards where
> this indeed
> makes sense. in our case, cnt == 1 from the get go since no wildcards so this
> check can't ever fail.

I think the problem is that it could now pass for cases where
function name has multiple instances in kallsyms .. while before
it would be caught in libbpf_available_kallsyms_parse and fail

jirka


> 
> Andrii, Yonghong - any suggestions ?
> 
> [1] - https://lore.kernel.org/bpf/20241218225246.3170300-1-yonghong.song@linux.dev/
> 
> Andrey
> 
> 
> > > [ ... ]
> >
> >
> > ---
> > AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> > See: https://urldefense.com/v3/__https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md__;!!BmdzS3_lV9HdKG8!z-aIXCz8YRZcraMmGI2bmb4YrDgW0brRTcX_BaJCWYwj7xfmkZL6qka6aqqIwzDPUjR1TxUU-Mc50s9AAYQf-vQMuPuGlVKW$
> >
> > CI run summary: https://urldefense.com/v3/__https://github.com/kernel-patches/bpf/actions/runs/22503275616__;!!BmdzS3_lV9HdKG8!z-aIXCz8YRZcraMmGI2bmb4YrDgW0brRTcX_BaJCWYwj7xfmkZL6qka6aqqIwzDPUjR1TxUU-Mc50s9AAYQf-vQMuGeekJPd$
> >
> > AI-authorship-score: medium
> > AI-authorship-explanation: Comments are unusually verbose for a simple optimization, and the commit message is well-structured with distinct sections, but the iterative v1-v3 refinement following reviewer feedback is typical of human development.
> > issues-found: 1
> > issue-severity-score: low
> > issue-severity-explanation: The fast path bypasses the unique_match check, which could silently attach to the wrong function among same-named statics, but requires the uncommon combination of unique_match=true with an exact name matching multiple kernel functions.

  reply	other threads:[~2026-03-02 10:58 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-27 20:40 [RFC PATCH bpf-next v3 0/3] Optimize kprobe.session attachment for exact function names Andrey Grodzovsky
2026-02-27 20:40 ` [RFC PATCH bpf-next v3 1/3] libbpf: " Andrey Grodzovsky
2026-02-27 21:17   ` bot+bpf-ci
2026-02-27 22:28     ` [External] " Andrey Grodzovsky
2026-03-02 10:58       ` Jiri Olsa [this message]
2026-03-02 17:37         ` Andrey Grodzovsky
2026-02-27 20:40 ` [RFC PATCH bpf-next v3 2/3] ftrace: Use kallsyms binary search for single-symbol lookup Andrey Grodzovsky
2026-02-27 20:40 ` [RFC PATCH bpf-next v3 3/3] selftests/bpf: add tests for kprobe.session optimization Andrey Grodzovsky
2026-02-27 20:43 ` [RFC PATCH bpf-next v3 0/3] Optimize kprobe.session attachment for exact function names Alexei Starovoitov
2026-02-27 21:15   ` [External] " Andrey Grodzovsky
2026-03-02 19:25     ` Jakub Sitnicki
2026-03-02 19:47       ` Andrey Grodzovsky
2026-03-02 12:47 ` Jakub Sitnicki
2026-03-02 18:00   ` [External] " Andrey Grodzovsky

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=aaVtarHiOYPfe9Tu@krava \
    --to=olsajiri@gmail.com \
    --cc=andrey.grodzovsky@crowdstrike.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bot+bpf-ci@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=clm@meta.com \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=ihor.solodrai@linux.dev \
    --cc=linux-open-source@crowdstrike.com \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=martin.lau@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=yonghong.song@linux.dev \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.