Re: [PATCH bpf-next] bpf, x86: inline bpf_get_current_task() for x86_64

[Menglong Dong <menglong.dong@linux.dev>]

On 2025/12/30 03:58 Eduard Zingerman <eddyz87@gmail.com> write:
> On Thu, 2025-12-25 at 18:44 +0800, Menglong Dong wrote:
> > Inline bpf_get_current_task() and bpf_get_current_task_btf() for x86_64
> > to obtain better performance. The instruction we use here is:
> > 
> >   65 48 8B 04 25 [offset] // mov rax, gs:[offset]
> > 
> > Not sure if there is any side effect here.
> > 
> > Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
> > ---
> 
> The change makes sense to me.
> Could you please address the compilation error reported by kernel test robot?

Yeah, I'll send a V2 later.

> Could you please also add a tests case using __jited annotation like
> in verifier_ldsx.c?

OK, sounds nice.

> 
> >  arch/x86/net/bpf_jit_comp.c | 33 +++++++++++++++++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> > 
> > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> > index b69dc7194e2c..7f38481816f0 100644
> > --- a/arch/x86/net/bpf_jit_comp.c
> > +++ b/arch/x86/net/bpf_jit_comp.c
> > @@ -1300,6 +1300,19 @@ static void emit_st_r12(u8 **pprog, u32 size, u32 dst_reg, int off, int imm)
> >  	emit_st_index(pprog, size, dst_reg, X86_REG_R12, off, imm);
> >  }
> >  
> > +static void emit_ldx_percpu_r0(u8 **pprog, const void __percpu *ptr)
> > +{
> > +	u8 *prog = *pprog;
> > +
> > +	/* mov rax, gs:[offset] */
> > +	EMIT2(0x65, 0x48);
> > +	EMIT2(0x8B, 0x04);
> > +	EMIT1(0x25);
> > +	EMIT((u32)(unsigned long)ptr, 4);
> > +
> > +	*pprog = prog;
> > +}
> > +
> >  static int emit_atomic_rmw(u8 **pprog, u32 atomic_op,
> >  			   u32 dst_reg, u32 src_reg, s16 off, u8 bpf_size)
> >  {
> > @@ -2435,6 +2448,15 @@ st:			if (is_imm8(insn->off))
> >  		case BPF_JMP | BPF_CALL: {
> >  			u8 *ip = image + addrs[i - 1];
> >  
> > +			if (insn->src_reg == 0 && (insn->imm == BPF_FUNC_get_current_task ||
> > +						   insn->imm == BPF_FUNC_get_current_task_btf)) {
> > +				if (IS_ENABLED(CONFIG_USE_X86_SEG_SUPPORT))
> > +					emit_ldx_percpu_r0(&prog, &const_current_task);
> > +				else
> > +					emit_ldx_percpu_r0(&prog, &current_task);
> 
> Nit: arch/x86/include/asm/current.h says that current_task and const_current_task are aliases.
>      In that case, why would we need two branches here?

It's not need here. I were not familiar with the per-cpu variable
before, and didn't realize it.

And it seems that the gs register is only used in the
CONFIG_USE_X86_SEG_SUPPORT case, which is the common case.
So maybe we can support it for this case only. For the
!CONFIG_USE_X86_SEG_SUPPORT case, let me do more analysis to
see if we can support it easily.

Thanks!
Menglong Dong

> 
> > +				break;
> > +			}
> > +
> >  			func = (u8 *) __bpf_call_base + imm32;
> >  			if (src_reg == BPF_PSEUDO_CALL && tail_call_reachable) {
> >  				LOAD_TAIL_CALL_CNT_PTR(stack_depth);
> > @@ -4067,3 +4089,14 @@ bool bpf_jit_supports_timed_may_goto(void)
> >  {
> >  	return true;
> >  }
> > +
> > +bool bpf_jit_inlines_helper_call(s32 imm)
> > +{
> > +	switch (imm) {
> > +	case BPF_FUNC_get_current_task:
> > +	case BPF_FUNC_get_current_task_btf:
> > +		return true;
> > +	default:
> > +		return false;
> > +	}
> > +}
>