Re: [RFC/PATCH bpf-next 01/20] bpf: Add multi uprobe link

[Jiri Olsa <olsajiri@gmail.com>]

On Wed, Apr 26, 2023 at 12:01:47PM -0700, Andrii Nakryiko wrote:
> On Tue, Apr 25, 2023 at 2:55 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Mon, Apr 24, 2023 at 03:11:20PM -0700, Alexei Starovoitov wrote:
> > > On Mon, Apr 24, 2023 at 06:04:28PM +0200, Jiri Olsa 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_run_ctx *old_run_ctx;
> > > > +   int err;
> > > > +
> > > > +   preempt_disable();
> > >
> > > preempt_disable? Which year is this? :)
> > > Let's allow sleepable from the start.
> > > See bpf_prog_run_array_sleepable.
> >
> > ok, we should probably add also 'multi.uprobe.s' section so the program
> > gets loaded with the BPF_F_SLEEPABLE flag.. or maybe we can enable that
> > by default for 'multi.uprobe' section
> 
> "uprobe.multi.s" rather, to follow "kprobe.multi.s". But we can't make
> it sleepable always/by default. Sleepable BPF programs are not just
> better types of programs, they have their own limitations, so it has
> to be the user's choice to go with sleepable or non-sleepable.

ok, will add uprobe.multi.s

thanks,
jirka

> 
> >
> > >
> > > Other than this the set looks great.
> > >
> > > > +
> > > > +   if (unlikely(__this_cpu_inc_return(bpf_prog_active) != 1)) {
> > > > +           err = 0;
> > > > +           goto out;
> > > > +   }
> > > > +
> > > > +   rcu_read_lock();
> > > > +   old_run_ctx = bpf_set_run_ctx(&run_ctx.run_ctx);
> > > > +   err = bpf_prog_run(link->link.prog, regs);
> > > > +   bpf_reset_run_ctx(old_run_ctx);
> > > > +   rcu_read_unlock();
> > > > +
> 
> [...]