BPF List
 help / color / mirror / Atom feed
From: Jiri Olsa <olsajiri@gmail.com>
To: Jiri Olsa <olsajiri@gmail.com>
Cc: Oleg Nesterov <oleg@redhat.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	bpf@vger.kernel.org, Song Liu <songliubraving@fb.com>,
	Yonghong Song <yhs@fb.com>,
	John Fastabend <john.fastabend@gmail.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	"Borislav Petkov (AMD)" <bp@alien8.de>,
	x86@kernel.org
Subject: Re: [PATCH RFC bpf-next 4/3] uprobe: ensure sys_uretprobe uses sysret
Date: Tue, 19 Mar 2024 20:31:55 +0100	[thread overview]
Message-ID: <ZfnoK8aQDJMgOYfY@krava> (raw)
In-Reply-To: <ZflyM5U67c47i4Oo@krava>

On Tue, Mar 19, 2024 at 12:08:35PM +0100, Jiri Olsa wrote:
> On Tue, Mar 19, 2024 at 11:25:24AM +0100, Oleg Nesterov wrote:
> > Obviously not for inclusion yet ;) untested, lacks the comments, and I am not
> > sure it makes sense.
> > 
> > But I am wondering if this change can speedup uretprobes a bit more. Any chance
> > you can test it?
> > 
> > With 1/3 sys_uretprobe() changes regs->r11/cx, this is correct but implies iret.
> > See the /* SYSRET requires RCX == RIP and R11 == EFLAGS */ code in do_syscall_64().
> 
> nice idea, looks like sysexit should be faster
> 
> > 
> > With this patch uretprobe_syscall_entry restores rcx/r11 itself and does retq,
> > sys_uretprobe() needs to hijack regs->ip after uprobe_handle_trampoline() to
> > make it possible.
> > 
> > Comments?
> > 
> > Oleg.
> > ---
> > 
> > diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
> > index 069371e86180..b99f1d80a8c8 100644
> > --- a/arch/x86/kernel/uprobes.c
> > +++ b/arch/x86/kernel/uprobes.c
> > @@ -319,6 +319,9 @@ asm (
> >  	"pushq %r11\n"
> >  	"movq $462, %rax\n"
> >  	"syscall\n"
> > +	"popq %r11\n"
> > +	"popq %rcx\n"
> > +	"retq\n"
> 
> using rax space on stack for return pointer, cool :)
> 
> I'll run the test with this change

I got bigger speed up on intel, amd stays the same (I'll double check that)

current:
  base           :   16.133 ± 0.035M/s
  uprobe-nop     :    3.003 ± 0.002M/s
  uprobe-push    :    2.829 ± 0.001M/s
  uprobe-ret     :    1.101 ± 0.001M/s
  uretprobe-nop  :    1.485 ± 0.001M/s
  uretprobe-push :    1.447 ± 0.000M/s
  uretprobe-ret  :    0.812 ± 0.000M/s

fix:
  base           :   16.522 ± 0.103M/s
  uprobe-nop     :    2.920 ± 0.034M/s
  uprobe-push    :    2.749 ± 0.047M/s
  uprobe-ret     :    1.094 ± 0.003M/s
  uretprobe-nop  :    2.004 ± 0.006M/s  < ~34% speed up
  uretprobe-push :    1.940 ± 0.003M/s  < ~34% speed up
  uretprobe-ret  :    0.921 ± 0.050M/s  < ~13% speed up

original fix:
  base           :   15.704 ± 0.076M/s
  uprobe-nop     :    2.841 ± 0.008M/s
  uprobe-push    :    2.666 ± 0.029M/s
  uprobe-ret     :    1.037 ± 0.008M/s
  uretprobe-nop  :    1.718 ± 0.010M/s  < ~25% speed up
  uretprobe-push :    1.658 ± 0.008M/s  < ~23% speed up
  uretprobe-ret  :    0.853 ± 0.004M/s  < ~14% speed up


jirka

  parent reply	other threads:[~2024-03-19 19:31 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-18  9:31 [PATCH RFC bpf-next 0/3] uprobe: uretprobe speed up Jiri Olsa
2024-03-18  9:31 ` [PATCH RFC bpf-next 1/3] uprobe: Add uretprobe syscall to speed up return probe Jiri Olsa
2024-03-18 14:22   ` Oleg Nesterov
2024-03-19  1:11   ` Andrii Nakryiko
2024-03-19  6:32     ` Oleg Nesterov
2024-03-19 16:20       ` Andrii Nakryiko
2024-03-19 10:54     ` Jiri Olsa
2024-03-18  9:31 ` [PATCH RFC bpf-next 2/3] selftests/bpf: Add uretprobe syscall test Jiri Olsa
2024-03-19  1:16   ` Andrii Nakryiko
2024-03-19 11:09     ` Jiri Olsa
2024-03-18  9:31 ` [PATCH RFC bpf-next 3/3] selftests/bpf: Mark uprobe trigger functions with nocf_check attribute Jiri Olsa
2024-03-19  1:22   ` Andrii Nakryiko
2024-03-19 11:11     ` Jiri Olsa
2024-03-22 13:40       ` Jiri Olsa
2024-03-19 10:25 ` [PATCH RFC bpf-next 4/3] uprobe: ensure sys_uretprobe uses sysret Oleg Nesterov
2024-03-19 11:08   ` Jiri Olsa
2024-03-19 16:25     ` Andrii Nakryiko
2024-03-19 16:38       ` Oleg Nesterov
2024-03-19 19:35       ` Jiri Olsa
2024-03-19 19:31     ` Jiri Olsa [this message]
2024-03-19 20:13       ` Andrii Nakryiko
2024-03-20 11:04       ` Jiri Olsa
2024-03-20 14:37         ` Oleg Nesterov
2024-03-20 15:28           ` Oleg Nesterov
2024-03-20 17:44             ` Andrii Nakryiko
2024-03-20 19:08               ` Jiri Olsa
2024-03-21 10:10                 ` Oleg Nesterov
2024-03-21  9:59             ` Jiri Olsa
2024-03-21 10:17               ` Oleg Nesterov
2024-03-21 10:52                 ` Jiri Olsa
2024-03-21 12:14                   ` Oleg Nesterov
2024-03-21 20:29                     ` Jiri Olsa
2024-03-22  8:48                       ` Oleg Nesterov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZfnoK8aQDJMgOYfY@krava \
    --to=olsajiri@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bp@alien8.de \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=john.fastabend@gmail.com \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=songliubraving@fb.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox