Messages in this thread | | | From | Andy Lutomirski <> | Date | Sun, 22 May 2016 14:07:28 -0700 | Subject | Re: [PATCH 3/4] x86: Rewrite switch_to() code |
| |
On Sun, May 22, 2016 at 12:31 PM, Brian Gerst <brgerst@gmail.com> wrote: > On Sun, May 22, 2016 at 1:59 PM, Andy Lutomirski <luto@amacapital.net> wrote: >> 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: > > What is so magical about the standard C calling convention (regarm(3) > in the 32-bit case)? This just passes them right though to > __switch_to(). >
I guess it's not highly magical, just a bit different from what I expected. I guess it's okay.
--Andy
>>> + >>> + jmp __switch_to >> >> leaves prev in eax. This works, but it might be worth a comment. > > Not quite, __switch_to() returns 'last', not 'prev'. The previous > task when this is called is not the same task when the thread wakes > up.
Right.
I wish switch_to were a normal function instead of a silly macro.
--Andy
| |