On Wed, Jul 31, 2024 at 9:56 AM Peter Zijlstra <pet...@infradead.org> wrote:
>
> On Wed, Jul 31, 2024 at 09:18:00AM -0700, Andrii Nakryiko wrote:
> > On Mon, Jul 29, 2024 at 6:45 AM Oleg Nesterov <o...@redhat.com> wrote:
> > >
> > > This way uprobe_unregister() and uprobe_apply() can use "struct uprobe *"
> > > rather than inode + offset. This simplifies the code and allows to avoid
> > > the unnecessary find_uprobe() + put_uprobe() in these functions.
> > >
> > > TODO: uprobe_unregister() still needs get_uprobe/put_uprobe to ensure that
> > > this uprobe can't be freed before up_write(&uprobe->register_rwsem).
> > >
> > > Signed-off-by: Oleg Nesterov <o...@redhat.com>
> > > Acked-by: Andrii Nakryiko <and...@kernel.org>
> > > ---
> > > include/linux/uprobes.h | 15 +++++-----
> > > kernel/events/uprobes.c | 56 +++++++++++++++----------------------
> > > kernel/trace/bpf_trace.c | 25 ++++++++---------
> > > kernel/trace/trace_uprobe.c | 26 ++++++++---------
> > > 4 files changed, 55 insertions(+), 67 deletions(-)
> > >
> >
> > You'll need something like below to not break our bpf_testmod. And
> > please send pull patch sets, not individually updated patches, it's a
> > PITA to deal with. Thanks!
>
> Do I stuff this on top of Oleg's patch or do you want me to fold it in
> one of them?
Please fold so we have better (potential) bisectability of BPF
selftests, thanks!
>
> > commit 9f739a9997ab833394196459fa7e6dd4d13dd48b (HEAD ->
> > uprobes-oleg-cleanups)
> > Author: Andrii Nakryiko <and...@kernel.org>
> > Date: Wed Jul 31 09:15:46 2024 -0700
> >
> > uprobes: fix bpf_testmod after uprobe_register/uprobe_unregister API
> > change
> >
> > Signed-off-by: Andrii Nakryiko <and...@kernel.org>
> >
> > diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> > b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> > index 5f152afdec2f..73a6b041bcce 100644
> > --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> > +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> > @@ -431,6 +431,7 @@ uprobe_ret_handler(struct uprobe_consumer *self,
> > unsigned long func,
> > }
> >
> > struct testmod_uprobe {
> > + struct uprobe *uprobe;
> > struct path path;
> > loff_t offset;
> > struct uprobe_consumer consumer;
> > @@ -458,12 +459,14 @@ static int testmod_register_uprobe(loff_t offset)
> > if (err)
> > goto out;
> >
> > - err = uprobe_register(d_real_inode(uprobe.path.dentry),
> > - offset, 0, &uprobe.consumer);
> > - if (err)
> > + uprobe.uprobe = uprobe_register(d_real_inode(uprobe.path.dentry),
> > + offset, 0, &uprobe.consumer);
> > + if (IS_ERR(uprobe.uprobe)) {
> > path_put(&uprobe.path);
> > - else
> > + uprobe.uprobe = NULL;
> > + } else {
> > uprobe.offset = offset;
> > + }
> >
> > out:
> > mutex_unlock(&testmod_uprobe_mutex);
> > @@ -474,10 +477,10 @@ static void testmod_unregister_uprobe(void)
> > {
> > mutex_lock(&testmod_uprobe_mutex);
> >
> > - if (uprobe.offset) {
> > - uprobe_unregister(d_real_inode(uprobe.path.dentry),
> > - uprobe.offset, &uprobe.consumer);
> > + if (uprobe.uprobe) {
> > + uprobe_unregister(uprobe.uprobe, &uprobe.consumer);
> > uprobe.offset = 0;
> > + uprobe.uprobe = NULL;
> > }
> >
> > mutex_unlock(&testmod_uprobe_mutex);
> >
> >
> > [...]
