[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <Zw9gGHzQNY9BLJCZ@krava>
Date: Wed, 16 Oct 2024 08:41:28 +0200
From: Jiri Olsa <olsajiri@...il.com>
To: Masami Hiramatsu <mhiramat@...nel.org>,
Peter Zijlstra <peterz@...radead.org>
Cc: Oleg Nesterov <oleg@...hat.com>, Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>, bpf@...r.kernel.org,
Martin KaFai Lau <kafai@...com>, Song Liu <songliubraving@...com>,
Yonghong Song <yhs@...com>,
John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...omium.org>,
Stanislav Fomichev <sdf@...ichev.me>, Hao Luo <haoluo@...gle.com>,
Steven Rostedt <rostedt@...dmis.org>, linux-kernel@...r.kernel.org,
linux-trace-kernel@...r.kernel.org
Subject: Re: [PATCHv6 perf/core 01/16] uprobe: Add data pointer to consumer
handlers
On Wed, Oct 16, 2024 at 07:24:26AM +0900, Masami Hiramatsu wrote:
> On Thu, 10 Oct 2024 22:09:42 +0200
> Jiri Olsa <jolsa@...nel.org> wrote:
>
> > Adding data pointer to both entry and exit consumer handlers and all
> > its users. The functionality itself is coming in following change.
> >
>
> Looks good to me.
>
> Acked-by: Masami Hiramatsu (Google) <mhiramat@...nel.org>
>
> Alexei, please merge this series via bpf tree, since most of the patches
> in this series are for bpf.
hi,
so the uprobe changes are based on Peter's perf/core and bpf changes
on bpf-next/master.. I merged them locally as a base for this patchset
Andrii has an idea how we could proceed with merging:
> I think uprobe parts should stay in tip/perf/core (if that's where all
> uprobe code goes in), as we have a bunch of ongoing work that all will
> conflict a bit with each other, if it lands across multiple trees.
>
> So that means that patches #1 and #2 ideally land in tip/perf/core.
> But you have a lot of BPF-specific things that would be inconvenient
> to route through tip, so I'd say those should go through bpf-next.
>
> What we can do, if Ingo and Peter are OK with that, is to create a
> stable (non-rebaseable) branch off of your first two patches (applied
> in tip/perf/core), which we'll merge into bpf-next/master and land the
> rest of your patch set there. We've done that with recent struct fd
> changes, and there were few other similar cases in the past, and that
> all worked well.
>
> Peter, Ingo, are you guys OK with that approach?
jirka
>
> Thank you,
>
> > Acked-by: Oleg Nesterov <oleg@...hat.com>
> > Acked-by: Andrii Nakryiko <andrii@...nel.org>
> > Signed-off-by: Jiri Olsa <jolsa@...nel.org>
> > ---
> > include/linux/uprobes.h | 4 ++--
> > kernel/events/uprobes.c | 4 ++--
> > kernel/trace/bpf_trace.c | 6 ++++--
> > kernel/trace/trace_uprobe.c | 12 ++++++++----
> > .../testing/selftests/bpf/bpf_testmod/bpf_testmod.c | 2 +-
> > 5 files changed, 17 insertions(+), 11 deletions(-)
> >
> > diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
> > index 2b294bf1881f..bb265a632b91 100644
> > --- a/include/linux/uprobes.h
> > +++ b/include/linux/uprobes.h
> > @@ -37,10 +37,10 @@ struct uprobe_consumer {
> > * for the current process. If filter() is omitted or returns true,
> > * UPROBE_HANDLER_REMOVE is effectively ignored.
> > */
> > - int (*handler)(struct uprobe_consumer *self, struct pt_regs *regs);
> > + int (*handler)(struct uprobe_consumer *self, struct pt_regs *regs, __u64 *data);
> > int (*ret_handler)(struct uprobe_consumer *self,
> > unsigned long func,
> > - struct pt_regs *regs);
> > + struct pt_regs *regs, __u64 *data);
> > bool (*filter)(struct uprobe_consumer *self, struct mm_struct *mm);
> >
> > struct list_head cons_node;
> > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> > index 2a0059464383..6b44c386a5df 100644
> > --- a/kernel/events/uprobes.c
> > +++ b/kernel/events/uprobes.c
> > @@ -2090,7 +2090,7 @@ static void handler_chain(struct uprobe *uprobe, struct pt_regs *regs)
> > int rc = 0;
> >
> > if (uc->handler) {
> > - rc = uc->handler(uc, regs);
> > + rc = uc->handler(uc, regs, NULL);
> > WARN(rc & ~UPROBE_HANDLER_MASK,
> > "bad rc=0x%x from %ps()\n", rc, uc->handler);
> > }
> > @@ -2128,7 +2128,7 @@ handle_uretprobe_chain(struct return_instance *ri, struct pt_regs *regs)
> > rcu_read_lock_trace();
> > list_for_each_entry_rcu(uc, &uprobe->consumers, cons_node, rcu_read_lock_trace_held()) {
> > if (uc->ret_handler)
> > - uc->ret_handler(uc, ri->func, regs);
> > + uc->ret_handler(uc, ri->func, regs, NULL);
> > }
> > rcu_read_unlock_trace();
> > }
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index a582cd25ca87..fdab7ecd8dfa 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -3244,7 +3244,8 @@ uprobe_multi_link_filter(struct uprobe_consumer *con, struct mm_struct *mm)
> > }
> >
> > static int
> > -uprobe_multi_link_handler(struct uprobe_consumer *con, struct pt_regs *regs)
> > +uprobe_multi_link_handler(struct uprobe_consumer *con, struct pt_regs *regs,
> > + __u64 *data)
> > {
> > struct bpf_uprobe *uprobe;
> >
> > @@ -3253,7 +3254,8 @@ uprobe_multi_link_handler(struct uprobe_consumer *con, struct pt_regs *regs)
> > }
> >
> > static int
> > -uprobe_multi_link_ret_handler(struct uprobe_consumer *con, unsigned long func, struct pt_regs *regs)
> > +uprobe_multi_link_ret_handler(struct uprobe_consumer *con, unsigned long func, struct pt_regs *regs,
> > + __u64 *data)
> > {
> > struct bpf_uprobe *uprobe;
> >
> > diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> > index c40531d2cbad..5895eabe3581 100644
> > --- a/kernel/trace/trace_uprobe.c
> > +++ b/kernel/trace/trace_uprobe.c
> > @@ -89,9 +89,11 @@ static struct trace_uprobe *to_trace_uprobe(struct dyn_event *ev)
> > static int register_uprobe_event(struct trace_uprobe *tu);
> > static int unregister_uprobe_event(struct trace_uprobe *tu);
> >
> > -static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs *regs);
> > +static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs *regs,
> > + __u64 *data);
> > static int uretprobe_dispatcher(struct uprobe_consumer *con,
> > - unsigned long func, struct pt_regs *regs);
> > + unsigned long func, struct pt_regs *regs,
> > + __u64 *data);
> >
> > #ifdef CONFIG_STACK_GROWSUP
> > static unsigned long adjust_stack_addr(unsigned long addr, unsigned int n)
> > @@ -1517,7 +1519,8 @@ trace_uprobe_register(struct trace_event_call *event, enum trace_reg type,
> > }
> > }
> >
> > -static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs *regs)
> > +static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs *regs,
> > + __u64 *data)
> > {
> > struct trace_uprobe *tu;
> > struct uprobe_dispatch_data udd;
> > @@ -1548,7 +1551,8 @@ static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs *regs)
> > }
> >
> > static int uretprobe_dispatcher(struct uprobe_consumer *con,
> > - unsigned long func, struct pt_regs *regs)
> > + unsigned long func, struct pt_regs *regs,
> > + __u64 *data)
> > {
> > struct trace_uprobe *tu;
> > struct uprobe_dispatch_data udd;
> > diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> > index 8835761d9a12..12005e3dc3e4 100644
> > --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> > +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> > @@ -461,7 +461,7 @@ static struct bin_attribute bin_attr_bpf_testmod_file __ro_after_init = {
> >
> > static int
> > uprobe_ret_handler(struct uprobe_consumer *self, unsigned long func,
> > - struct pt_regs *regs)
> > + struct pt_regs *regs, __u64 *data)
> >
> > {
> > regs->ax = 0x12345678deadbeef;
> > --
> > 2.46.2
> >
>
>
> --
> Masami Hiramatsu (Google) <mhiramat@...nel.org>
Powered by blists - more mailing lists