[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEf4BzbxjRwxhJTLUgJNwR-vEbDybBpawNsRb+y+PiDsxzT=eA@mail.gmail.com>
Date: Thu, 4 Sep 2025 11:06:31 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: Oleg Nesterov <oleg@...hat.com>, Jiri Olsa <olsajiri@...il.com>,
Masami Hiramatsu <mhiramat@...nel.org>, Peter Zijlstra <peterz@...radead.org>,
Andrii Nakryiko <andrii@...nel.org>, bpf <bpf@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>,
linux-trace-kernel <linux-trace-kernel@...r.kernel.org>, X86 ML <x86@...nel.org>,
Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
John Fastabend <john.fastabend@...il.com>, Hao Luo <haoluo@...gle.com>,
Steven Rostedt <rostedt@...dmis.org>, Ingo Molnar <mingo@...nel.org>
Subject: Re: [PATCH perf/core 02/11] uprobes: Skip emulate/sstep on unique
uprobe when ip is changed
On Thu, Sep 4, 2025 at 8:02 AM Alexei Starovoitov
<alexei.starovoitov@...il.com> wrote:
>
> On Thu, Sep 4, 2025 at 4:26 AM Oleg Nesterov <oleg@...hat.com> wrote:
> >
> > On 09/04, Jiri Olsa wrote:
> > >
> > > On Thu, Sep 04, 2025 at 10:49:50AM +0200, Oleg Nesterov wrote:
> > > > On 09/03, Jiri Olsa wrote:
> > > > >
> > > > > On Wed, Sep 03, 2025 at 01:26:48PM +0200, Oleg Nesterov wrote:
> > > > > > On 09/02, Jiri Olsa wrote:
> > > > > > >
> > > > > > > If user decided to take execution elsewhere, it makes little sense
> > > > > > > to execute the original instruction, so let's skip it.
> > > > > >
> > > > > > Exactly.
> > > > > >
> > > > > > So why do we need all these "is_unique" complications? Only a single
> > > > > > is_unique/exclusive consumer can change regs->ip, so I guess handle_swbp()
> > > > > > can just do
> > > > > >
> > > > > > handler_chain(uprobe, regs);
> > > > > > if (instruction_pointer(regs) != bp_vaddr)
> > > > > > goto out;
> > > > >
> > > > > hum, that's what I did in rfc [1] but I thought you did not like that [2]
> > > > >
> > > > > [1] https://lore.kernel.org/bpf/20250801210238.2207429-2-jolsa@kernel.org/
> > > > > [2] https://lore.kernel.org/bpf/20250802103426.GC31711@redhat.com/
> > > > >
> > > > > I guess I misunderstood your reply [2], I'd be happy to drop the
> > > > > unique/exclusive flag
> > > >
> > > > Well, but that rfc didn't introduce the exclusive consumers, and I think
> > > > we agree that even with these changes the non-exclusive consumers must
> > > > never change regs->ip?
> > >
> > > ok, got excited too soon.. so you meant getting rid of is_unique
> > > check only for this patch and have just change below.. but keep
> > > the unique/exclusive flag from patch#1
> >
> > Yes, this is what I meant,
> >
> > > IIUC Andrii would remove the unique flag completely?
> >
> > Lets wait for Andrii...
>
> Not Andrii, but I see only negatives in this extra flag.
> It doesn't add any safety or guardrails.
> No need to pollute uapi with pointless flags.
+1. I think it's fine to just have something like
if (unlikely(instruction_pointer(regs) != bp_vaddr))
goto out;
after all uprobe callbacks were processed. Even if every single one of
them modify IP, the last one that did that wins. Others (if they care)
can detect this.
Generally speaking, this is a very specialized use case (which is why
the opposition to complicating UAPI for all of that), and I'd expect
to have at most 1 such uprobe callbacks at any attach point, while all
others (if there are any "others") are read-only and won't care about
return IP.
Powered by blists - more mailing lists