Re: FAILED: patch "[PATCH] bpf: support deferring bpf_link dealloc to after RCU grace" failed to apply to 6.8-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Apr 5, 2024 at 9:15 AM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Fri, Apr 5, 2024 at 2:50 AM <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> >
> >
> > The patch below does not apply to the 6.8-stable tree.
> > If someone wants it applied there, or to any other stable or longterm
> > tree, then please email the backport, including the original git commit
> > id to <stable@xxxxxxxxxxxxxxx>.
> >
>
> This is strange, I get a clean auto-merged cherry-pick:
>
> $ git co linux-6.8.y
> Updating files: 100% (10694/10694), done.
> branch 'linux-6.8.y' set up to track 'stable/linux-6.8.y'.
> Switched to a new branch 'linux-6.8.y'
> $ git cp -x 1a80dbcb2dbaf6e4c216e62e30fa7d3daa8001ce
> Auto-merging include/linux/bpf.h
> Auto-merging kernel/bpf/syscall.c
> Auto-merging kernel/trace/bpf_trace.c
> [linux-6.8.y fd74c60792f5] bpf: support deferring bpf_link dealloc to
> after RCU grace period
>  Date: Wed Mar 27 22:24:26 2024 -0700
>  3 files changed, 49 insertions(+), 6 deletions(-)
>
> Note that e9c856cabefb ("bpf: put uprobe link's path and task in
> release callback") has to be backported at the same time. I'll
> cherry-pick both and will send it just in case.
>

Ah, so it doesn't build (trivial link->prog->sleepable ->
link->prog->aux->sleepable change, will fix up). Not sure if possible,
but it would be nice to distinguish between patch not applying vs it
causing build (or test) failures, but no big deal.

>
> > To reproduce the conflict and resubmit, you may use the following commands:
> >
> > git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.8.y
> > git checkout FETCH_HEAD
> > git cherry-pick -x 1a80dbcb2dbaf6e4c216e62e30fa7d3daa8001ce
> > # <resolve conflicts, build, test, etc.>
> > git commit -s
> > git send-email --to '<stable@xxxxxxxxxxxxxxx>' --in-reply-to '2024040527-propeller-immovably-a6d8@gregkh' --subject-prefix 'PATCH 6.8.y' HEAD^..
> >
> > Possible dependencies:
> >
> >
> >
> > thanks,
> >
> > greg k-h
> >
> > ------------------ original commit in Linus's tree ------------------
> >
> > From 1a80dbcb2dbaf6e4c216e62e30fa7d3daa8001ce Mon Sep 17 00:00:00 2001
> > From: Andrii Nakryiko <andrii@xxxxxxxxxx>
> > Date: Wed, 27 Mar 2024 22:24:26 -0700
> > Subject: [PATCH] bpf: support deferring bpf_link dealloc to after RCU grace
> >  period
> >
> > BPF link for some program types is passed as a "context" which can be
> > used by those BPF programs to look up additional information. E.g., for
> > multi-kprobes and multi-uprobes, link is used to fetch BPF cookie values.
> >
> > Because of this runtime dependency, when bpf_link refcnt drops to zero
> > there could still be active BPF programs running accessing link data.
> >
> > This patch adds generic support to defer bpf_link dealloc callback to
> > after RCU GP, if requested. This is done by exposing two different
> > deallocation callbacks, one synchronous and one deferred. If deferred
> > one is provided, bpf_link_free() will schedule dealloc_deferred()
> > callback to happen after RCU GP.
> >
> > BPF is using two flavors of RCU: "classic" non-sleepable one and RCU
> > tasks trace one. The latter is used when sleepable BPF programs are
> > used. bpf_link_free() accommodates that by checking underlying BPF
> > program's sleepable flag, and goes either through normal RCU GP only for
> > non-sleepable, or through RCU tasks trace GP *and* then normal RCU GP
> > (taking into account rcu_trace_implies_rcu_gp() optimization), if BPF
> > program is sleepable.
> >
> > We use this for multi-kprobe and multi-uprobe links, which dereference
> > link during program run. We also preventively switch raw_tp link to use
> > deferred dealloc callback, as upcoming changes in bpf-next tree expose
> > raw_tp link data (specifically, cookie value) to BPF program at runtime
> > as well.
> >
> > Fixes: 0dcac2725406 ("bpf: Add multi kprobe link")
> > Fixes: 89ae89f53d20 ("bpf: Add multi uprobe link")
> > Reported-by: syzbot+981935d9485a560bfbcb@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Reported-by: syzbot+2cb5a6c573e98db598cc@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Reported-by: syzbot+62d8b26793e8a2bd0516@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> > Acked-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> > Link: https://lore.kernel.org/r/20240328052426.3042617-2-andrii@xxxxxxxxxx
> > Signed-off-by: Alexei Starovoitov <ast@xxxxxxxxxx>
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index 4f20f62f9d63..890e152d553e 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -1574,12 +1574,26 @@ struct bpf_link {
> >         enum bpf_link_type type;
> >         const struct bpf_link_ops *ops;
> >         struct bpf_prog *prog;
> > -       struct work_struct work;
> > +       /* rcu is used before freeing, work can be used to schedule that
> > +        * RCU-based freeing before that, so they never overlap
> > +        */
> > +       union {
> > +               struct rcu_head rcu;
> > +               struct work_struct work;
> > +       };
> >  };
> >
> >  struct bpf_link_ops {
> >         void (*release)(struct bpf_link *link);
> > +       /* deallocate link resources callback, called without RCU grace period
> > +        * waiting
> > +        */
> >         void (*dealloc)(struct bpf_link *link);
> > +       /* deallocate link resources callback, called after RCU grace period;
> > +        * if underlying BPF program is sleepable we go through tasks trace
> > +        * RCU GP and then "classic" RCU GP
> > +        */
> > +       void (*dealloc_deferred)(struct bpf_link *link);
> >         int (*detach)(struct bpf_link *link);
> >         int (*update_prog)(struct bpf_link *link, struct bpf_prog *new_prog,
> >                            struct bpf_prog *old_prog);
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index ae2ff73bde7e..c287925471f6 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -3024,17 +3024,46 @@ void bpf_link_inc(struct bpf_link *link)
> >         atomic64_inc(&link->refcnt);
> >  }
> >
> > +static void bpf_link_defer_dealloc_rcu_gp(struct rcu_head *rcu)
> > +{
> > +       struct bpf_link *link = container_of(rcu, struct bpf_link, rcu);
> > +
> > +       /* free bpf_link and its containing memory */
> > +       link->ops->dealloc_deferred(link);
> > +}
> > +
> > +static void bpf_link_defer_dealloc_mult_rcu_gp(struct rcu_head *rcu)
> > +{
> > +       if (rcu_trace_implies_rcu_gp())
> > +               bpf_link_defer_dealloc_rcu_gp(rcu);
> > +       else
> > +               call_rcu(rcu, bpf_link_defer_dealloc_rcu_gp);
> > +}
> > +
> >  /* bpf_link_free is guaranteed to be called from process context */
> >  static void bpf_link_free(struct bpf_link *link)
> >  {
> > +       bool sleepable = false;
> > +
> >         bpf_link_free_id(link->id);
> >         if (link->prog) {
> > +               sleepable = link->prog->sleepable;
> >                 /* detach BPF program, clean up used resources */
> >                 link->ops->release(link);
> >                 bpf_prog_put(link->prog);
> >         }
> > -       /* free bpf_link and its containing memory */
> > -       link->ops->dealloc(link);
> > +       if (link->ops->dealloc_deferred) {
> > +               /* schedule BPF link deallocation; if underlying BPF program
> > +                * is sleepable, we need to first wait for RCU tasks trace
> > +                * sync, then go through "classic" RCU grace period
> > +                */
> > +               if (sleepable)
> > +                       call_rcu_tasks_trace(&link->rcu, bpf_link_defer_dealloc_mult_rcu_gp);
> > +               else
> > +                       call_rcu(&link->rcu, bpf_link_defer_dealloc_rcu_gp);
> > +       }
> > +       if (link->ops->dealloc)
> > +               link->ops->dealloc(link);
> >  }
> >
> >  static void bpf_link_put_deferred(struct work_struct *work)
> > @@ -3544,7 +3573,7 @@ static int bpf_raw_tp_link_fill_link_info(const struct bpf_link *link,
> >
> >  static const struct bpf_link_ops bpf_raw_tp_link_lops = {
> >         .release = bpf_raw_tp_link_release,
> > -       .dealloc = bpf_raw_tp_link_dealloc,
> > +       .dealloc_deferred = bpf_raw_tp_link_dealloc,
> >         .show_fdinfo = bpf_raw_tp_link_show_fdinfo,
> >         .fill_link_info = bpf_raw_tp_link_fill_link_info,
> >  };
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index 0b73fe5f7206..9dc605f08a23 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -2728,7 +2728,7 @@ static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link,
> >
> >  static const struct bpf_link_ops bpf_kprobe_multi_link_lops = {
> >         .release = bpf_kprobe_multi_link_release,
> > -       .dealloc = bpf_kprobe_multi_link_dealloc,
> > +       .dealloc_deferred = bpf_kprobe_multi_link_dealloc,
> >         .fill_link_info = bpf_kprobe_multi_link_fill_link_info,
> >  };
> >
> > @@ -3242,7 +3242,7 @@ static int bpf_uprobe_multi_link_fill_link_info(const struct bpf_link *link,
> >
> >  static const struct bpf_link_ops bpf_uprobe_multi_link_lops = {
> >         .release = bpf_uprobe_multi_link_release,
> > -       .dealloc = bpf_uprobe_multi_link_dealloc,
> > +       .dealloc_deferred = bpf_uprobe_multi_link_dealloc,
> >         .fill_link_info = bpf_uprobe_multi_link_fill_link_info,
> >  };
> >
> >





[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux