Re: [PATCH v5 07/13] bpf, x86: Take a Tasks Trace reader in the trampoline around its call-outs

["Alexei Starovoitov" <alexei.starovoitov@gmail.com>]

On Tue Sep 22, 2026 at 2:23 AM UTC, Josef Bacik wrote:
> On HAVE_RCU_TRAMPOLINE_READERS kernels Tasks RCU keeps a BPF trampoline
> image allocated only while a task using it is a Tasks Trace RCU reader
> or is executing text that rcu_tasks_trampoline_text() recognises.  The
> image itself is such text, but the C glue and the programs it calls are
> not, and only sleepable programs take rcu_read_lock_trace() today.
>
> Have the x86-64 JIT open-code rcu_read_lock_trace() and
> rcu_read_unlock_trace() in the trampoline, as ftrace_64.S does for
> ftrace_caller: one reader from just after the frame is set up to just
> before the original function is called, covering __bpf_tramp_enter()
> and the fentry and fmod_ret programs, and a second one from just after
> the original function returns to just before the final register
> restore, covering the fexit programs and __bpf_tramp_exit().  The
> original function itself runs outside both, since it may run for a long
> time and the image is pinned by im->pcref across it.  Trampolines that
> do not call the original function get a single reader around all their
> programs.  The second reader is entered before ip_after_call, so the
> ip_after_call -> ip_epilogue jump that bpf_tramp_image_put() patches in
> is inside it, and the fmod_ret early-exit branch lands after that point
> still holding the first reader, so exactly one is held on every path.
>
> The sequence uses r10 and r11, which are scratch at each emission point,
> and references current_task and rcu_tasks_trace_srcu_struct by absolute
> sign-extended address, the form the JIT already relies on for
> this_cpu_off.  Sleepable programs' own rcu_read_lock_trace() simply
> nests.  Nothing is emitted on other configurations.
>
> Suggested-by: Alexei Starovoitov <ast@kernel.org>
> Assisted-by: LLM
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  arch/x86/net/bpf_jit_comp.c | 107 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 107 insertions(+)
>
> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index 2853e87797a7..b77e29c9599d 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -14,6 +14,7 @@
>  #include <linux/memory.h>
>  #include <linux/sort.h>
>  #include <linux/execmem.h>
> +#include <linux/rcupdate_trace.h>
>  #include <asm/extable.h>
>  #include <asm/ftrace.h>
>  #include <asm/set_memory.h>
> @@ -722,6 +723,91 @@ static void emit_indirect_jump(u8 **pprog, int bpf_reg, u8 *ip)
>  	*pprog = prog;
>  }
>  
> +/*
> + * Open-coded rcu_read_lock_trace() / rcu_read_unlock_trace() for the
> + * trampoline, see CONFIG_HAVE_RCU_TRAMPOLINE_READERS and the equivalent
> + * macros in arch/x86/kernel/ftrace_64.S.  The image is not relocated, so
> + * current_task and rcu_tasks_trace_srcu_struct are referenced by absolute
> + * (sign-extended 32-bit) address, the form the JIT already relies on for
> + * this_cpu_off.  Uses r10 and r11, which are scratch at every emission
> + * point, and clobbers flags.
> + *
> + * lock:                                  unlock:
> + *   mov  r11, gs:[current_task]            mov  r11, gs:[current_task]
> + *   mov  r10d, [r11+nesting_off]           mov  r10d, [r11+nesting_off]
> + *   inc  dword ptr [r11+nesting_off]       sub  r10d, 1
> + *   test r10d, r10d                        jnz  2f
> + *   jnz  1f                                mov  r10, [r11+scp_off]
> + *   mov  r10, [&srcu.srcu_ctrp]            mov  dword ptr [r11+nesting_off], 0
> + *   inc  qword ptr gs:[r10+locks_off]      (smp_mb)
> + *   mov  [r11+scp_off], r10                inc  qword ptr gs:[r10+unlocks_off]
> + *   (smp_mb)                               jmp  3f
> + * 1:                                     2: mov [r11+nesting_off], r10d
> + *                                        3:
> + */
> +static void emit_trace_rcu_reader(u8 **pprog, bool lock)
> +{
> +#ifdef CONFIG_TASKS_RCU_TRAMPOLINE_READERS
> +	const u32 nesting_off = offsetof(struct task_struct, trc_reader_nesting);
> +	const u32 scp_off = offsetof(struct task_struct, trc_reader_scp);
> +	const u32 locks_off = offsetof(struct srcu_ctr, srcu_locks);
> +	const u32 unlocks_off = offsetof(struct srcu_ctr, srcu_unlocks);
> +	const bool mb = !IS_ENABLED(CONFIG_TASKS_TRACE_RCU_NO_MB);
> +	u8 *prog = *pprog;
> +
> +	/* The plain this_cpu_inc() form of __srcu_read_lock_fast(). */
> +	BUILD_BUG_ON(IS_ENABLED(CONFIG_NEED_SRCU_NMI_SAFE));
> +
> +	/* mov r11, gs:[abs32 current_task] */
> +	EMIT2(0x65, 0x4C);
> +	EMIT3(0x8B, 0x1C, 0x25);
> +	EMIT((u32)(unsigned long)&current_task, 4);
> +	/* mov r10d, dword ptr [r11 + nesting_off] */
> +	EMIT3_off32(0x45, 0x8B, 0x93, nesting_off);
> +
> +	if (lock) {
> +		/* inc dword ptr [r11 + nesting_off] */
> +		EMIT3_off32(0x41, 0xFF, 0x83, nesting_off);
> +		/* test r10d, r10d */
> +		EMIT3(0x45, 0x85, 0xD2);
> +		/* jnz 1f */
> +		EMIT2(X86_JNE, 8 + 8 + 7 + (mb ? 6 : 0));
> +		/* mov r10, qword ptr [abs32 &rcu_tasks_trace_srcu_struct.srcu_ctrp] */
> +		EMIT4(0x4C, 0x8B, 0x14, 0x25);
> +		EMIT((u32)(unsigned long)&rcu_tasks_trace_srcu_struct.srcu_ctrp, 4);
> +		/* inc qword ptr gs:[r10 + locks_off] */
> +		EMIT4_off32(0x65, 0x49, 0xFF, 0x82, locks_off);
> +		/* mov qword ptr [r11 + scp_off], r10 */
> +		EMIT3_off32(0x4D, 0x89, 0x93, scp_off);
> +		/* smp_mb(): lock add dword ptr [rsp - 4], 0 */
> +		if (mb)
> +			EMIT2_off32(0xF0, 0x83, 0x00FC2444);
> +		/* 1: */
> +	} else {
> +		/* sub r10d, 1 */
> +		EMIT4(0x41, 0x83, 0xEA, 0x01);
> +		/* jnz 2f */
> +		EMIT2(X86_JNE, 7 + 11 + (mb ? 6 : 0) + 8 + 2);
> +		/* mov r10, qword ptr [r11 + scp_off] */
> +		EMIT3_off32(0x4D, 0x8B, 0x93, scp_off);
> +		/* mov dword ptr [r11 + nesting_off], 0 */
> +		EMIT3_off32(0x41, 0xC7, 0x83, nesting_off);
> +		EMIT(0, 4);
> +		if (mb)
> +			EMIT2_off32(0xF0, 0x83, 0x00FC2444);
> +		/* inc qword ptr gs:[r10 + unlocks_off] */
> +		EMIT4_off32(0x65, 0x49, 0xFF, 0x82, unlocks_off);
> +		/* jmp 3f */
> +		EMIT2(0xEB, 7);
> +		/* 2: mov dword ptr [r11 + nesting_off], r10d */
> +		EMIT3_off32(0x45, 0x89, 0x93, nesting_off);
> +		/* 3: */
> +	}

I think lockdep is still broken due to inlining.
lockdep enabled kernel will miss this rcu tasks CS.

Instead let's add rcu_read_lock_trace() to __bpf_tramp_enter()
and remove it from __bpf_prog_enter_sleepable*().

Also add __bpf_tramp_before_call_orig()
and __bpf_tramp_after_call_orig(), so that orig call
can run outside for RCU tasks CS.

Better ideas?