LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Andy Lutomirski <luto@amacapital.net>
To: Ingo Molnar <mingo@kernel.org>
Cc: Denys Vlasenko <dvlasenk@redhat.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Borislav Petkov <bp@alien8.de>, "H. Peter Anvin" <hpa@zytor.com>,
	Oleg Nesterov <oleg@redhat.com>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Alexei Starovoitov <ast@plumgrid.com>,
	Will Drewry <wad@chromium.org>, Kees Cook <keescook@chromium.org>,
	X86 ML <x86@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 3/4] x86: save user rsp in pt_regs->sp on SYSCALL64 fastpath
Date: Tue, 10 Mar 2015 06:10:29 -0700	[thread overview]
Message-ID: <CALCETrUFbbjeUrZEuUvGQLefrKBTH=RTdVJ0vXgAGBEEuzEMKg@mail.gmail.com> (raw)
In-Reply-To: <20150310125151.GB21686@gmail.com>

On Tue, Mar 10, 2015 at 5:51 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Denys Vlasenko <dvlasenk@redhat.com> wrote:
>
>> PER_CPU(old_rsp) usage is simplified - now it is used only
>> as temp storage, and userspace stack pointer is immediately stored
>> in pt_regs->sp on syscall entry, instead of being used later,
>> on syscall exit.
>>
>> Instead of PER_CPU(old_rsp) and task->thread.usersp, C code
>> uses pt_regs->sp now.
>>
>> FIXUP/RESTORE_TOP_OF_STACK are simplified.
>
> Just trying to judge the performance impact:
>
>> --- a/arch/x86/kernel/entry_64.S
>> +++ b/arch/x86/kernel/entry_64.S
>> @@ -128,8 +128,6 @@ ENDPROC(native_usergs_sysret64)
>>   * manipulation.
>>   */
>>       .macro FIXUP_TOP_OF_STACK tmp offset=0
>> -     movq PER_CPU_VAR(old_rsp),\tmp
>> -     movq \tmp,RSP+\offset(%rsp)
>>       movq $__USER_DS,SS+\offset(%rsp)
>>       movq $__USER_CS,CS+\offset(%rsp)
>>       movq RIP+\offset(%rsp),\tmp  /* get rip */
>> @@ -139,8 +137,7 @@ ENDPROC(native_usergs_sysret64)
>>       .endm
>>
>>       .macro RESTORE_TOP_OF_STACK tmp offset=0
>> -     movq RSP+\offset(%rsp),\tmp
>> -     movq \tmp,PER_CPU_VAR(old_rsp)
>> +     /* nothing to do */
>>       .endm
>>
>>  /*
>> @@ -253,11 +247,13 @@ GLOBAL(system_call_after_swapgs)
>>        */
>>       ENABLE_INTERRUPTS(CLBR_NONE)
>>       ALLOC_PT_GPREGS_ON_STACK 8              /* +8: space for orig_ax */
>> +     movq    %rcx,RIP(%rsp)
>> +     movq    PER_CPU_VAR(old_rsp),%rcx
>> +     movq    %r11,EFLAGS(%rsp)
>> +     movq    %rcx,RSP(%rsp)
>> +     movq_cfi rax,ORIG_RAX
>>       SAVE_C_REGS_EXCEPT_RAX_RCX_R11
>>       movq    $-ENOSYS,RAX(%rsp)
>> -     movq_cfi rax,ORIG_RAX
>> -     movq    %r11,EFLAGS(%rsp)
>> -     movq    %rcx,RIP(%rsp)
>>       CFI_REL_OFFSET rip,RIP
>>       testl $_TIF_WORK_SYSCALL_ENTRY,TI_flags+THREAD_INFO(%rsp,RIP)
>>       jnz tracesys
>
> So there are now +2 instructions (5 instead of 3) in the system_call
> path, but there are -2 instructions in the SYSRETQ path, so combined
> it's a wash performance-wise, right?

I think that the SYSRETQ path is the same number of instructions --
RESTORE_TOP_OF_STACK is only used in somewhat unusual circumstances.

On SYSRETQ, I think we touch one fewer cache line, although that cache
line will be in L1 unless the syscall pushed it out.  The added
instructions on entry are probably nearly free, though, as one of them
is a load from a forwarded address and the other is a store to a hot
cacheline.

--And

  reply	other threads:[~2015-03-10 13:10 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-09 18:39 [PATCH 0/4 v2] x86: entry_64.S: steps towards simpler iret frame handling Denys Vlasenko
2015-03-09 18:39 ` [PATCH 1/4] x86: save r11 into pt_regs->flags on SYSCALL64 fastpath Denys Vlasenko
2015-03-09 20:02   ` Andy Lutomirski
2015-03-16 12:04   ` [tip:x86/asm] x86/asm/entry/64: Save R11 into pt_regs-> flags " tip-bot for Denys Vlasenko
2015-03-09 18:39 ` [PATCH 3/4] x86: save user rsp in pt_regs->sp " Denys Vlasenko
2015-03-09 20:11   ` Andy Lutomirski
2015-03-09 20:32     ` Denys Vlasenko
2015-03-09 20:43       ` Andy Lutomirski
2015-03-10 12:51   ` Ingo Molnar
2015-03-10 13:10     ` Andy Lutomirski [this message]
2015-03-10 13:18     ` Denys Vlasenko
2015-03-10 13:20       ` Andy Lutomirski
2015-03-10 13:26         ` Ingo Molnar
2015-03-10 13:21       ` Ingo Molnar
2015-03-10 13:26         ` Andy Lutomirski
2015-03-10 14:00           ` Denys Vlasenko
2015-03-10 14:02             ` Andy Lutomirski
2015-03-10 14:09               ` Denys Vlasenko
2015-03-10 13:28         ` Ingo Molnar
2015-03-10 13:50         ` Denys Vlasenko
2015-03-16  9:44           ` Ingo Molnar
2015-03-16 12:05   ` [tip:x86/asm] x86/asm/entry/64: Save user RSP in pt_regs-> sp " tip-bot for Denys Vlasenko
2015-03-10  6:00 ` [PATCH 0/4 v2] x86: entry_64.S: steps towards simpler iret frame handling Ingo Molnar

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='CALCETrUFbbjeUrZEuUvGQLefrKBTH=RTdVJ0vXgAGBEEuzEMKg@mail.gmail.com' \
    --to=luto@amacapital.net \
    --cc=ast@plumgrid.com \
    --cc=bp@alien8.de \
    --cc=dvlasenk@redhat.com \
    --cc=fweisbec@gmail.com \
    --cc=hpa@zytor.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=torvalds@linux-foundation.org \
    --cc=wad@chromium.org \
    --cc=x86@kernel.org \
    --subject='Re: [PATCH 3/4] x86: save user rsp in pt_regs->sp on SYSCALL64 fastpath' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).