From: Andy Lutomirski <luto@amacapital.net>
To: Brian Gerst <brgerst@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
Denys Vlasenko <dvlasenk@redhat.com>,
Ingo Molnar <mingo@kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Borislav Petkov <bp@suse.de>, "H. Peter Anvin" <hpa@zytor.com>,
X86 ML <x86@kernel.org>, Josh Poimboeuf <jpoimboe@redhat.com>
Subject: Re: [PATCH 3/4] x86: Rewrite switch_to() code
Date: Sun, 22 May 2016 10:59:38 -0700 [thread overview]
Message-ID: <CALCETrV2gtaGeX72tuczYfU+C6SAPL5uDOy-_EgbH+-KR-D_jA@mail.gmail.com> (raw)
In-Reply-To: <1463846691-18498-4-git-send-email-brgerst@gmail.com>
cc: Josh Poimboeuf: do you care about the exact stack layout of the
bottom of the stack of an inactive task?
On May 21, 2016 9:05 AM, "Brian Gerst" <brgerst@gmail.com> wrote:
>
> Move the low-level context switch code to an out-of-line asm stub instead of
> using complex inline asm. This allows constructing a new stack frame for the
> child process to make it seamlessly flow to ret_from_fork without an extra
> test and branch in __switch_to(). It also improves code generation for
> __schedule() by using the C calling convention instead of clobbering all
> registers.
I like the concept a lot.
>
> Signed-off-by: Brian Gerst <brgerst@gmail.com>
> ---
> arch/x86/entry/entry_32.S | 38 ++++++++++
> arch/x86/entry/entry_64.S | 42 +++++++++++-
> arch/x86/include/asm/processor.h | 3 -
> arch/x86/include/asm/switch_to.h | 137 ++++++-------------------------------
> arch/x86/include/asm/thread_info.h | 2 -
> arch/x86/kernel/asm-offsets.c | 6 ++
> arch/x86/kernel/asm-offsets_32.c | 5 ++
> arch/x86/kernel/asm-offsets_64.c | 5 ++
> arch/x86/kernel/process_32.c | 8 ++-
> arch/x86/kernel/process_64.c | 7 +-
> arch/x86/kernel/smpboot.c | 1 -
> 11 files changed, 124 insertions(+), 130 deletions(-)
>
> diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
> index ee6fea0..05e5340 100644
> --- a/arch/x86/entry/entry_32.S
> +++ b/arch/x86/entry/entry_32.S
> @@ -204,6 +204,44 @@
> POP_GS_EX
> .endm
>
> +/*
> + * %eax: prev task
> + * %edx: next task
> + */
> +ENTRY(__switch_to_asm)
> + /*
> + * Save callee-saved registers
> + * This must match the order in struct fork_frame
> + * Frame pointer must be last for get_wchan
> + */
> + pushl %ebx
> + pushl %edi
> + pushl %esi
> + pushl %ebp
> +
> + /* switch stack */
> + movl %esp, TASK_threadsp(%eax)
> + movl TASK_threadsp(%edx), %esp
> +
> +#ifdef CONFIG_CC_STACKPROTECTOR
> + movl TASK_stack_canary(%edx), %ebx
> + movl %ebx, PER_CPU_VAR(stack_canary)+stack_canary_offset
> +#endif
> +
> + /* restore callee-saved registers */
> + popl %ebp
> + popl %esi
> + popl %edi
> + popl %ebx
This is highly, highly magical. eax and edx are prev and next, and:
> +
> + jmp __switch_to
leaves prev in eax. This works, but it might be worth a comment.
> +END(__switch_to_asm)
> /*
> + * %rdi: prev task
> + * %rsi: next task
> + */
> +ENTRY(__switch_to_asm)
> + /*
> + * Save callee-saved registers
> + * This must match the order in struct fork_frame
> + * Frame pointer must be last for get_wchan
> + */
> + pushq %rbx
> + pushq %r12
> + pushq %r13
> + pushq %r14
> + pushq %r15
> + pushq %rbp
> +
> + /* switch stack */
> + movq %rsp, TASK_threadsp(%rdi)
> + movq TASK_threadsp(%rsi), %rsp
> +
> +#ifdef CONFIG_CC_STACKPROTECTOR
> + movq TASK_stack_canary(%rsi), %rbx
> + movq %rbx, PER_CPU_VAR(irq_stack_union)+stack_canary_offset
> +#endif
> +
> + /* restore callee-saved registers */
> + popq %rbp
> + popq %r15
> + popq %r14
> + popq %r13
> + popq %r12
> + popq %rbx
> +
> + jmp __switch_to
Ditto with the magic here.
> +struct fork_frame {
> + unsigned long bp;
> +#ifdef CONFIG_X86_64
> + unsigned long r15;
> + unsigned long r14;
> + unsigned long r13;
> + unsigned long r12;
> +#else
> + unsigned long si;
> + unsigned long di;
> +#endif
> + unsigned long bx;
> + unsigned long ret_addr;
> + struct pt_regs regs;
> +};
This, like the old implementation, is very much geared to the current
implementation of fork. Can you split it up:
struct inactive_task_frame {
unsigned long bp;
...
unsigned long ret_addr;
};
/* fork works by setting up the child stack so that switch_to will
land at ret_from_fork with sp pointing at pt_regs */
struct fork_frame {
struct inactive_task_frame switch_frame;
struct pt_regs regs;
};
Then, if and when someone wants to fork into a different type of
context, they can reuse this. Also, a future improved unwinder can
use inactive_task_frame directly to kick off its unwind.
--Andy
next prev parent reply other threads:[~2016-05-22 18:00 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-21 16:04 [PATCH 0/4] x86: Rewrite switch_to() Brian Gerst
2016-05-21 16:04 ` [PATCH 1/4] x86: Save return value from kernel_thread Brian Gerst
2016-05-22 1:44 ` Andy Lutomirski
2016-05-22 2:19 ` Brian Gerst
2016-05-21 16:04 ` [PATCH 2/4] x86-32, kgdb: Don't use thread.ip in sleeping_thread_to_gdb_regs() Brian Gerst
2016-05-23 17:05 ` Andy Lutomirski
2016-05-21 16:04 ` [PATCH 3/4] x86: Rewrite switch_to() code Brian Gerst
2016-05-22 17:59 ` Andy Lutomirski [this message]
2016-05-22 19:31 ` Brian Gerst
2016-05-22 21:07 ` Andy Lutomirski
2016-05-23 2:34 ` Josh Poimboeuf
2016-05-23 4:47 ` Andy Lutomirski
2016-05-23 11:40 ` Josh Poimboeuf
2016-05-23 11:49 ` Brian Gerst
2016-05-23 12:05 ` Josh Poimboeuf
2016-05-23 11:14 ` Brian Gerst
2016-05-23 11:47 ` Josh Poimboeuf
2016-05-23 11:49 ` Josh Poimboeuf
2016-05-23 16:46 ` Josh Poimboeuf
2016-05-23 17:03 ` Andy Lutomirski
2016-05-23 18:44 ` Josh Poimboeuf
2016-07-12 14:16 ` Josh Poimboeuf
2016-06-15 1:31 ` Andy Lutomirski
2016-06-15 8:03 ` Ingo Molnar
2016-06-15 11:52 ` Brian Gerst
2016-05-21 16:04 ` [PATCH 4/4] x86: Pass kernel thread parameters in fork_frame Brian Gerst
2016-05-22 18:01 ` Andy Lutomirski
2016-05-22 19:21 ` Brian Gerst
2016-05-23 15:23 ` Josh Poimboeuf
2016-05-23 15:36 ` Andy Lutomirski
2016-05-23 21:04 ` Brian Gerst
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=CALCETrV2gtaGeX72tuczYfU+C6SAPL5uDOy-_EgbH+-KR-D_jA@mail.gmail.com \
--to=luto@amacapital.net \
--cc=bp@suse.de \
--cc=brgerst@gmail.com \
--cc=dvlasenk@redhat.com \
--cc=hpa@zytor.com \
--cc=jpoimboe@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/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;
as well as URLs for NNTP newsgroup(s).