All of lore.kernel.org
 help / color / mirror / Atom feed
From: Menglong Dong <menglong.dong@linux.dev>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Menglong Dong <menglong8.dong@gmail.com>,
	Andrii Nakryiko <andrii.nakryiko@gmail.com>,
	Alexei Starovoitov <ast@kernel.org>, Eduard <eddyz87@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	David Ahern <dsahern@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	Martin KaFai Lau <martin.lau@linux.dev>,
	Song Liu <song@kernel.org>,
	Yonghong Song <yonghong.song@linux.dev>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@kernel.org>,
	Stanislav Fomichev <sdf@fomichev.me>, Hao Luo <haoluo@google.com>,
	Jiri Olsa <jolsa@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	X86 ML <x86@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>,
	Network Development <netdev@vger.kernel.org>,
	bpf <bpf@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH bpf-next v6 1/2] bpf, x86: inline bpf_get_current_task() for x86_64
Date: Wed, 21 Jan 2026 11:37:28 +0800	[thread overview]
Message-ID: <3672461.iIbC2pHGDl@7940hx> (raw)
In-Reply-To: <CAADnVQ+Om14RF1GvA0vkhKdsCtVGzbacftEZarLJ-LKdYXUZ+g@mail.gmail.com>

On 2026/1/21 11:10 Alexei Starovoitov <alexei.starovoitov@gmail.com> write:
> On Tue, Jan 20, 2026 at 5:58 PM Menglong Dong <menglong.dong@linux.dev> wrote:
> >
> > On 2026/1/21 09:23 Andrii Nakryiko <andrii.nakryiko@gmail.com> write:
> > > On Mon, Jan 19, 2026 at 11:06 PM Menglong Dong <menglong8.dong@gmail.com> wrote:
> > > >
> > > > Inline bpf_get_current_task() and bpf_get_current_task_btf() for x86_64
> > > > to obtain better performance.
> > > >
> > > > Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
> > > > Acked-by: Eduard Zingerman <eddyz87@gmail.com>
> > > > ---
> > > > v5:
> > > > - don't support the !CONFIG_SMP case
> > > >
> > > > v4:
> > > > - handle the !CONFIG_SMP case
> > > >
> > > > v3:
> > > > - implement it in the verifier with BPF_MOV64_PERCPU_REG() instead of in
> > > >   x86_64 JIT.
> > > > ---
> > > >  kernel/bpf/verifier.c | 22 ++++++++++++++++++++++
> > > >  1 file changed, 22 insertions(+)
> > > >
> > > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > > > index 9de0ec0c3ed9..c4e2ffadfb1f 100644
> > > > --- a/kernel/bpf/verifier.c
> > > > +++ b/kernel/bpf/verifier.c
> > > > @@ -17739,6 +17739,10 @@ static bool verifier_inlines_helper_call(struct bpf_verifier_env *env, s32 imm)
> > > >         switch (imm) {
> > > >  #ifdef CONFIG_X86_64
> > > >         case BPF_FUNC_get_smp_processor_id:
> > > > +#ifdef CONFIG_SMP
> > > > +       case BPF_FUNC_get_current_task_btf:
> > > > +       case BPF_FUNC_get_current_task:
> > > > +#endif
> > >
> > > Does this have to be x86-64 specific inlining? With verifier inlining
> > > and per_cpu instruction support it should theoretically work across
> > > all architectures that do support per-cpu instruction, no?
> > >
> > > Eduard pointed out [0] to me for why we have that x86-64 specific
> > > check. But looking at do_misc_fixups(), we have that early
> > > bpf_jit_inlines_helper_call(insn->imm)) check, so if some JIT has more
> > > performant inlining implementation, we will just do that.
> > >
> > > So it seems like we can just drop all that x86-64 specific logic and
> > > claim all three of these functions as inlinable, no?
> > >
> > > And even more. We can drop rather confusing
> > > verifier_inlines_helper_call() that duplicates the decision of which
> > > helpers can be inlined or not, and have:
> >
> > The verifier_inlines_helper_call() is confusing, but I think we can't
> > remove the x86-64 checking. For example, some architecture
> > don't support BPF_FUNC_get_current_task both in
> > bpf_jit_inlines_helper_call() and verifier_inlines_helper_call(), which
> > means it can't be inline.
> >
> > >
> > > if (env->prog->jit_requested && bpf_jit_supports_percpu_insn() {
> > >     switch (insn->imm) {
> > >     case BPF_FUNC_get_smp_processor_id:
> > >         ...
> > >         break;
> > >     case BPF_FUNC_get_current_task_btf:
> > >     case BPF_FUNC_get_current_task_btf:
> > >         ...
> > >         break;
> > >     default:
> > > }
> > >
> > > And the decision about inlining will live in one place.
> > >
> > > Or am I missing some complications?
> >
> > As Alexei said, the implement of "current" is architecture specific,
> > and the per-cpu variable "current_task" only exist on x86_64.
> >
> > >
> > > And with all that, should we mark get_current_task and
> > > get_current_task_btf as __bpf_fastcall?
> >
> > I think it make sense, and the I saw bpf_get_smp_processor_id does
> > such operation:
> >
> > const struct bpf_func_proto bpf_get_smp_processor_id_proto = {
> >         [...]
> >         .allow_fastcall = true,
> > };
> >
> > PS: I'm a little confused about the fast call. We inline many helper,
> > but it seems that bpf_get_smp_processor_id is the only one that
> > use the "allow_fastcall". Why? I'd better study harder.
> 
> It's
> static __bpf_fastcall __u32 (* const bpf_get_smp_processor_id)(void) =
> (void *) 8;
> 
> and
> #define __bpf_fastcall __attribute__((bpf_fastcall))

Ah, I see. It seems that the bpf_doc.py does the trick.

> 
> which makes LLVM use more registers at the callsite (less spill/fill).
> 
> Looking at the patch again. I think it's fine as-is.
> fastcall can be a follow up.

Okay!

Thanks!
Menglong Dong






  reply	other threads:[~2026-01-21  3:37 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-20  7:05 [PATCH bpf-next v6 0/2] bpf, x86: inline bpf_get_current_task() for x86_64 Menglong Dong
2026-01-20  7:05 ` [PATCH bpf-next v6 1/2] " Menglong Dong
2026-01-21  1:23   ` Andrii Nakryiko
2026-01-21  1:43     ` Alexei Starovoitov
2026-01-21  1:58     ` Menglong Dong
2026-01-21  3:10       ` Alexei Starovoitov
2026-01-21  3:37         ` Menglong Dong [this message]
2026-01-21  4:12         ` Andrii Nakryiko
2026-01-21  4:46           ` Alexei Starovoitov
2026-01-21  6:35             ` Andrii Nakryiko
2026-01-20  7:05 ` [PATCH bpf-next v6 2/2] selftests/bpf: test the jited inline of bpf_get_current_task Menglong Dong
2026-01-20 17:52   ` Eduard Zingerman
2026-01-21  1:05   ` Andrii Nakryiko
2026-01-21  1:28     ` Menglong Dong
2026-01-21  1:32       ` Eduard Zingerman
2026-01-21  3:03         ` Menglong Dong
2026-01-21  4:50 ` [PATCH bpf-next v6 0/2] bpf, x86: inline bpf_get_current_task() for x86_64 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=3672461.iIbC2pHGDl@7940hx \
    --to=menglong.dong@linux.dev \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bp@alien8.de \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=dave.hansen@linux.intel.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=eddyz87@gmail.com \
    --cc=haoluo@google.com \
    --cc=hpa@zytor.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=menglong8.dong@gmail.com \
    --cc=mingo@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=sdf@fomichev.me \
    --cc=song@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.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.