|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH RFC v2 08/15] bpf, x86: Maintain Tasks RCU trampoline nesting in the BPF trampoline
On Sat, Sep 12, 2026 at 10:18:48AM -0700, Alexei Starovoitov wrote:
> On Fri Sep 11, 2026 at 10:10 PM PDT, Paul E. McKenney wrote:
> > On Fri, Sep 11, 2026 at 08:27:56PM -0700, Alexei Starovoitov wrote:
> >> On Fri Sep 11, 2026 at 7:08 AM PDT, Josef Bacik wrote:
> >> > Emit an increment of current->rcu_tramp_nesting once the trampoline's
> >> > frame is set up and a decrement before the final register restore, so
> >> > that a task preempted while running fentry/fexit/fmod_ret/LSM programs
> >> > or the __bpf_tramp_enter()/__bpf_tramp_exit() glue is not treated as
> >> > Tasks-RCU quiescent. Drop the count around the call to the original
> >> > function: that may run arbitrarily long without sleeping and must not pin
> >> > a Tasks RCU grace period, and the trampoline frame above it is held by
> >> > im->pcref rather than by Tasks RCU (see bpf_tramp_image_put()). The
> >> > fmod_ret early-exit branch and the ip_after_call -> ip_epilogue poke both
> >> > skip the decrement/increment pair around the original call, so the count
> >> > stays balanced on every path.
> >> >
> >> > The sequence is "mov r11, gs:[current_task]; inc/dec dword [r11 + off]";
> >> > r11 is scratch at every emission point and (u32)¤t_task is a valid
> >> > sign-extended %gs-absolute with the current per-CPU layout, the same form
> >> > the JIT already uses for this_cpu_off. The image is dynamically
> >> > allocated text, so the instructions outside the bracketed region are
> >> > covered by the irq-exit IP check.
> >> >
> >> > Assisted-by: LLM
> >> > Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> >> > ---
> >> > arch/x86/net/bpf_jit_comp.c | 43
> >> > +++++++++++++++++++++++++++++++++++++++++++
> >> > 1 file changed, 43 insertions(+)
> >> >
> >> > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> >> > index 2853e87797a7..a375c1b7bd50 100644
> >> > --- a/arch/x86/net/bpf_jit_comp.c
> >> > +++ b/arch/x86/net/bpf_jit_comp.c
> >> > @@ -722,6 +722,31 @@ static void emit_indirect_jump(u8 **pprog, int
> >> > bpf_reg, u8 *ip)
> >> > *pprog = prog;
> >> > }
> >> >
> >> > +/*
> >> > + * Tasks RCU trampoline nesting, see rcu_tasks_trampoline_enter().
> >> > + *
> >> > + * mov r11, QWORD PTR gs:[current_task]
> >> > + * inc/dec DWORD PTR [r11 + offsetof(struct task_struct,
> >> > rcu_tramp_nesting)]
> >> > + *
> >> > + * r11 (AUX_REG) is scratch in the trampoline at every point this is
> >> > emitted.
> >> > + */
> >> > +static void emit_rcu_tasks_tramp_nesting(u8 **pprog, bool enter)
> >> > +{
> >> > +#ifdef CONFIG_TASKS_RCU
> >> > + u8 *prog = *pprog;
> >> > +
> >> > + /* mov r11, gs:[abs32] */
> >> > + EMIT2(0x65, 0x4C);
> >> > + EMIT3(0x8B, 0x1C, 0x25);
> >> > + EMIT((u32)(unsigned long)¤t_task, 4);
> >> > + /* inc/dec dword ptr [r11 + disp32] */
> >> > + EMIT3(0x41, 0xFF, enter ? 0x83 : 0x8B);
> >> > + EMIT(offsetof(struct task_struct, rcu_tramp_nesting), 4);
> >> > +
> >> > + *pprog = prog;
> >> > +#endif
> >>
> >> It's not a lot of overhead, but I feel it will be the death by thousand
> >> cuts.
> >> rcu_read_lock_trace() in bpf_prog_enter_sleepable is doing the same
> >> thing...
> >> increamenting a variable inside current.
> >> Can they be combined? Like treat current->trc_reader_nesting > 0 as
> >> current->rcu_tramp_nesting > 0 ?
> >> Or replace one with the other?
> >> Two current->foo++ operations look redundant.
> >>
> >> bpf trampoline is already quite heavy. I'd like to find ways to reduce
> >> its overhead instead of adding more.
> >
> > Replace rcu_read_lock_trace() with Josef's rcu_tasks_trampoline_enter)?
>
> If necessary...
> what I don't understand why we need another rcu_tasks_trampoline_enter-like
> counter.
> Can existing rcu_read_lock_trace() current be used ?
> It's already doing current->trc_reader_nesting++
> so use that as a signal ?
In the old kernels, yes, we have current->trc_reader_nesting++.
In the newer kernels, Tasks Trace RCU is instead implemented in terms
of SRCU-fast, which instead increments per-CPU counters. Which among
other thins is a bit faster and does not need to hook into the scheduler.
So we have several ways forward:
1. Revert the implementation of RCU Tasks Trace in terms of
SRCU-fast, and use the existing current->trc_reader_nesting++,
as you suggest.
2. Deprecate RCU Tasks Trace entirely in favor of RCU Tasks
augmented by rcu_tasks_trampoline_enter() and friends, as
I was suggesting.
3. Implement rcu_tasks_trampoline_enter() in terms of SRCU-fast,
keeping the speedup, and put a synchronize_srcu() in the
RCU Tasks grace-period mechanism. This again deprecates
RCU Tasks Trace entirely in favor of the augmented RCU Tasks.
4. It is always good to explicitly state the apparent status quo,
which involves redundant trampoline entry/exit overhead.
5. As always, your additional ideas here!
Thanx, Paul
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |