On Fri, Jun 23, 2023 at 9:24 AM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> > > > +
> > > > +static int uprobe_prog_run(struct bpf_uprobe *uprobe,
> > > > + unsigned long entry_ip,
> > > > + struct pt_regs *regs)
> > > > +{
> > > > + struct bpf_uprobe_multi_link *link = uprobe->link;
> > > > + struct bpf_uprobe_multi_run_ctx run_ctx = {
> > > > + .entry_ip = entry_ip,
> > > > + };
> > > > + struct bpf_prog *prog = link->link.prog;
> > > > + struct bpf_run_ctx *old_run_ctx;
> > > > + int err = 0;
> > > > +
> > > > + might_fault();
> > > > +
> > > > + rcu_read_lock_trace();
> > >
> > > we don't need this if uprobe is not sleepable, right? why unconditional then?
> >
> > I won't pretend I understand what rcu_read_lock_trace does ;-)
> >
> > I tried to follow bpf_prog_run_array_sleepable where it's called
> > unconditionally for both sleepable and non-sleepable progs
> >
> > there are conditional rcu_read_un/lock calls later on
> >
> > I will check
>
> hm... Alexei can chime in here, but given here we actually are trying
> to run one BPF program (not entire array of them), we do know whether
> it's going to be sleepable or not. So we can avoid unnecessary
> rcu_read_{lock,unlock}_trace() calls. rcu_read_lock_trace() is used
> when there is going to be sleepable BPF program executed to protect
> BPF maps and other resources from being freed too soon. But if we know
> that we don't need sleepable, we can avoid that.
We can add more checks and bool flags to avoid rcu_read_{lock,unlock}_trace(),
but it will likely be slower. These calls are very fast.
Simpler and faster to do it unconditionally even when the array doesn't
have sleepable progs.
rcu_read_lock() we have to do conditionally, because it won't be ok
if sleepable progs are in the array.
