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?
> 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);
>
>
> [...]
