x86: Rewrite switch_to()
Subsystems (3)
v3 cover x86: Rewrite switch_to()
This patch set simplifies the switch_to() code, by moving the stack switch code out of line into an asm stub before calling __switch_to(). This ends up being more readable, and using the C calling convention instead of clobbering all registers improves code generation. It also allows newly forked processes to construct a special stack frame to seamlessly flow to ret_from_fork, instead of using a test and branch, or an unbalanced call/ret.
Changes from v2:
- Updated comments around kernel threads being uncommon for fork, etc.
- Removed STACK_FRAME_NON_STANDARD annotation from __schedule() per Josh Poimboeuf
- A few minor cleanups added
Changes from v1:
- Added struct inactive_task_frame
- Added comments about kernel threads returning to userspace
- Cleaned up some incorrect uses of thread.sp
- Rearranged inactive stack frame so that BP (frame pointer) is in the natural position right below the return address. This should take care of unwinding issues Josh raised.
Brian Gerst (7):
x86-32, kgdb: Don't use thread.ip in sleeping_thread_to_gdb_regs() x86-64, kgdb: clear GDB_PS on 64-bit
x86: Add struct inactive_task_frame
x86: Rewrite switch_to() code
x86: Pass kernel thread parameters in fork_frame
x86: Fix thread_saved_pc()
Revert "sched: Mark __schedule() stack frame as non-standard"
arch/x86/entry/entry_32.S | 68 +++++++++++++----- arch/x86/entry/entry_64.S | 78 ++++++++++++++------ arch/x86/include/asm/processor.h | 13 +--- arch/x86/include/asm/stacktrace.h | 4 +- arch/x86/include/asm/switch_to.h | 144 ++++++++----------------------------- 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/kgdb.c | 8 +-- arch/x86/kernel/process.c | 14 +++- arch/x86/kernel/process_32.c | 31 +++----- arch/x86/kernel/process_64.c | 21 +++--- arch/x86/kernel/smpboot.c | 1 - kernel/sched/core.c | 1 - 15 files changed, 190 insertions(+), 211 deletions(-)
Signature · 1 line
2.5.5
PATCH v3 1/7 x86-32, kgdb: Don't use thread.ip in sleeping_thread_to_gdb_regs()
Match 64-bit and set gdb_regs[GDB_PC] to zero. thread.ip is always the same point in the scheduler (except for newly forked processes), and will be removed in a future patch.
--- arch/x86/kernel/kgdb.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
arch/x86/kernel/kgdb.c +1 -2
@@ -172,7 +172,6 @@ void sleeping_thread_to_gdb_regs(unsigned long *gdb_regs, struct task_struct *p) gdb_regs[GDB_ES ] = __KERNEL_DS; gdb_regs[GDB_PS ] =0 ; gdb_regs[GDB_CS ] = __KERNEL_CS; - gdb_regs[GDB_PC ] = p->thread.ip; gdb_regs[GDB_SS ] = __KERNEL_DS; gdb_regs[GDB_FS ] =0xFFFF ; gdb_regs[GDB_GS ] =0xFFFF ;
@@ -180,7 +179,6 @@ void sleeping_thread_to_gdb_regs(unsigned long *gdb_regs, struct task_struct *p) gdb_regs32[GDB_PS ] = *(unsigned long *)(p->thread.sp +8 ); gdb_regs32[GDB_CS ] = __KERNEL_CS; gdb_regs32[GDB_SS ] = __KERNEL_DS; - gdb_regs[GDB_PC ] =0 ; gdb_regs[GDB_R8 ] =0 ; gdb_regs[GDB_R9 ] =0 ; gdb_regs[GDB_R10 ] =0 ;
@@ -190,6 +188,7 @@ void sleeping_thread_to_gdb_regs(unsigned long *gdb_regs, struct task_struct *p) gdb_regs[GDB_R14 ] =0 ; gdb_regs[GDB_R15 ] =0 ;#endif + gdb_regs[GDB_PC ] =0 ; gdb_regs[GDB_SP ] = p->thread.sp; }
Signature · 1 line
2.5.5
tip-bot for Brian Gerst
merge bot[tip:x86/asm] sched/x86/32, kgdb: Don't use thread.ip in sleeping_thread_to_gdb_regs()
sched/x86/32, kgdb: Don't use thread.ip in sleeping_thread_to_gdb_regs()
Commit-ID: 4e047aa7f267c3449b6d323510d35864829aca70 Gitweb: http://git.kernel.org/tip/4e047aa7f267c3449b6d323510d35864829aca70 Author: Brian Gerst <[email protected]> AuthorDate: Sat, 13 Aug 2016 12:38:16 -0400 Committer: Ingo Molnar <[email protected]> CommitDate: Wed, 24 Aug 2016 12:27:40 +0200
sched/x86/32, kgdb: Don't use thread.ip in sleeping_thread_to_gdb_regs()
Match 64-bit and set gdb_regs[GDB_PC] to zero. thread.ip is always the same point in the scheduler (except for newly forked processes), and will be removed in a future patch.
--- arch/x86/kernel/kgdb.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
arch/x86/kernel/kgdb.c +1 -2
@@ -172,7 +172,6 @@ void sleeping_thread_to_gdb_regs(unsigned long *gdb_regs, struct task_struct *p) gdb_regs[GDB_ES ] = __KERNEL_DS; gdb_regs[GDB_PS ] =0 ; gdb_regs[GDB_CS ] = __KERNEL_CS; - gdb_regs[GDB_PC ] = p->thread.ip; gdb_regs[GDB_SS ] = __KERNEL_DS; gdb_regs[GDB_FS ] =0xFFFF ; gdb_regs[GDB_GS ] =0xFFFF ;
@@ -180,7 +179,6 @@ void sleeping_thread_to_gdb_regs(unsigned long *gdb_regs, struct task_struct *p) gdb_regs32[GDB_PS ] = *(unsigned long *)(p->thread.sp +8 ); gdb_regs32[GDB_CS ] = __KERNEL_CS; gdb_regs32[GDB_SS ] = __KERNEL_DS; - gdb_regs[GDB_PC ] =0 ; gdb_regs[GDB_R8 ] =0 ; gdb_regs[GDB_R9 ] =0 ; gdb_regs[GDB_R10 ] =0 ;
@@ -190,6 +188,7 @@ void sleeping_thread_to_gdb_regs(unsigned long *gdb_regs, struct task_struct *p) gdb_regs[GDB_R14 ] =0 ; gdb_regs[GDB_R15 ] =0 ;#endif + gdb_regs[GDB_PC ] =0 ; gdb_regs[GDB_SP ] = p->thread.sp; }
PATCH v3 2/7 x86-64, kgdb: clear GDB_PS on 64-bit
In reply to Brian Gerstswitch_to() no longer saves EFLAGS, so it's bogus to look for it on the stack. Set it to zero like 32-bit.
--- arch/x86/kernel/kgdb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/kernel/kgdb.c +1 -1
@@ -176,7 +176,7 @@ void sleeping_thread_to_gdb_regs(unsigned long *gdb_regs, struct task_struct *p) gdb_regs[GDB_FS ] =0xFFFF ; gdb_regs[GDB_GS ] =0xFFFF ;#else - gdb_regs32[GDB_PS ] = *(unsigned long *)(p->thread.sp +8 ); + gdb_regs32[GDB_PS ] =0 ; gdb_regs32[GDB_CS ] = __KERNEL_CS; gdb_regs32[GDB_SS ] = __KERNEL_DS; gdb_regs[GDB_R8 ] =0 ;
Signature · 1 line
2.5.5
tip-bot for Brian Gerst
merge bot[tip:x86/asm] sched/x86/64, kgdb: Clear GDB_PS on 64-bit
sched/x86/64, kgdb: Clear GDB_PS on 64-bit
Commit-ID: 163630191ecb0dd9e4146d3c910045aba1cfeec1 Gitweb: http://git.kernel.org/tip/163630191ecb0dd9e4146d3c910045aba1cfeec1 Author: Brian Gerst <[email protected]> AuthorDate: Sat, 13 Aug 2016 12:38:17 -0400 Committer: Ingo Molnar <[email protected]> CommitDate: Wed, 24 Aug 2016 12:27:40 +0200
sched/x86/64, kgdb: Clear GDB_PS on 64-bit
switch_to() no longer saves EFLAGS, so it's bogus to look for it on the stack. Set it to zero like 32-bit.
--- arch/x86/kernel/kgdb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/kernel/kgdb.c +1 -1
@@ -176,7 +176,7 @@ void sleeping_thread_to_gdb_regs(unsigned long *gdb_regs, struct task_struct *p) gdb_regs[GDB_FS ] =0xFFFF ; gdb_regs[GDB_GS ] =0xFFFF ;#else - gdb_regs32[GDB_PS ] = *(unsigned long *)(p->thread.sp +8 ); + gdb_regs32[GDB_PS ] =0 ; gdb_regs32[GDB_CS ] = __KERNEL_CS; gdb_regs32[GDB_SS ] = __KERNEL_DS; gdb_regs[GDB_R8 ] =0 ;
PATCH v3 3/7 x86: Add struct inactive_task_frame
In reply to Brian GerstAdd struct inactive_task_frame, which defines the layout of the stack for a sleeping process. For now, the only defined field is the BP register (frame pointer).
--- arch/x86/include/asm/stacktrace.h | 4 ++-- arch/x86/include/asm/switch_to.h | 5 +++++ arch/x86/kernel/kgdb.c | 3 ++- arch/x86/kernel/process.c | 3 ++- 4 files changed, 11 insertions(+), 4 deletions(-)
arch/x86/include/asm/stacktrace.h +2 -2
@@ -8,6 +8,7 @@#include <linux/uaccess.h> #include <linux/ptrace.h> +#include <asm/switch_to.h> extern int kstack_depth_to_print;
@@ -70,8 +71,7 @@ stack_frame(struct task_struct *task, struct pt_regs *regs) return bp; } -/* bp is the last reg pushed by switch_to */ -return *(unsigned long *)task->thread.sp; +return ((struct inactive_task_frame *)task->thread.sp)->bp; }#else static inline unsigned long
arch/x86/include/asm/switch_to.h +5 -0
@@ -8,6 +8,11 @@ struct tss_struct;void __switch_to_xtra (struct task_struct *prev_p,struct task_struct *next_p,struct tss_struct *tss); +/* data that is pointed to by thread.sp */ +struct inactive_task_frame { +unsigned long bp; +}; +#ifdef CONFIG_X86_32 #ifdef CONFIG_CC_STACKPROTECTOR
arch/x86/kernel/kgdb.c +2 -1
@@ -50,6 +50,7 @@#include <asm/apicdef.h> #include <asm/apic.h> #include <asm/nmi.h> +#include <asm/switch_to.h> struct dbg_reg_def_t dbg_reg_def[DBG_MAX_REG_NUM ] = {
@@ -166,7 +167,7 @@ void sleeping_thread_to_gdb_regs(unsigned long *gdb_regs, struct task_struct *p) gdb_regs[GDB_DX ] =0 ; gdb_regs[GDB_SI ] =0 ; gdb_regs[GDB_DI ] =0 ; - gdb_regs[GDB_BP ] = *(unsigned long *)p->thread.sp; + gdb_regs[GDB_BP ] = ((struct inactive_task_frame *)p->thread.sp)->bp;#ifdef CONFIG_X86_32 gdb_regs[GDB_DS ] = __KERNEL_DS; gdb_regs[GDB_ES ] = __KERNEL_DS;
arch/x86/kernel/process.c +2 -1
@@ -32,6 +32,7 @@#include <asm/tlbflush.h> #include <asm/mce.h> #include <asm/vm86.h> +#include <asm/switch_to.h> /* * per-CPU TSS segments. Threads are completely 'soft' on Linux,
@@ -556,7 +557,7 @@ unsigned long get_wchan(struct task_struct *p)if (sp < bottom || sp > top)return 0 ; - fp =READ_ONCE_NOCHECK (*(unsigned long *)sp); + fp =READ_ONCE_NOCHECK (((struct inactive_task_frame *)sp)->bp); do { if (fp < bottom || fp > top) return 0;
Signature · 1 line
2.5.5
tip-bot for Brian Gerst
merge bot[tip:x86/asm] sched/x86: Add 'struct inactive_task_frame' to better document the sleeping task stack frame
sched/x86: Add 'struct inactive_task_frame' to better document the sleeping task stack frame
Commit-ID: 7b32aeadbc95d4a41402c1c0da6aa3ab51af4c10 Gitweb: http://git.kernel.org/tip/7b32aeadbc95d4a41402c1c0da6aa3ab51af4c10 Author: Brian Gerst <[email protected]> AuthorDate: Sat, 13 Aug 2016 12:38:18 -0400 Committer: Ingo Molnar <[email protected]> CommitDate: Wed, 24 Aug 2016 12:27:41 +0200
sched/x86: Add 'struct inactive_task_frame' to better document the sleeping task stack frame
Add 'struct inactive_task_frame', which defines the layout of the stack for a sleeping process. For now, the only defined field is the BP register (frame pointer).
--- arch/x86/include/asm/stacktrace.h | 4 ++-- arch/x86/include/asm/switch_to.h | 5 +++++ arch/x86/kernel/kgdb.c | 3 ++- arch/x86/kernel/process.c | 3 ++- 4 files changed, 11 insertions(+), 4 deletions(-)
arch/x86/include/asm/stacktrace.h +2 -2
@@ -8,6 +8,7 @@#include <linux/uaccess.h> #include <linux/ptrace.h> +#include <asm/switch_to.h> extern int kstack_depth_to_print;
@@ -70,8 +71,7 @@ stack_frame(struct task_struct *task, struct pt_regs *regs) return bp; } -/* bp is the last reg pushed by switch_to */ -return *(unsigned long *)task->thread.sp; +return ((struct inactive_task_frame *)task->thread.sp)->bp; }#else static inline unsigned long
arch/x86/include/asm/switch_to.h +5 -0
@@ -30,6 +30,11 @@ static inline void prepare_switch_to(struct task_struct *prev,#endif } +/* data that is pointed to by thread.sp */ +struct inactive_task_frame { +unsigned long bp; +}; +#ifdef CONFIG_X86_32 #ifdef CONFIG_CC_STACKPROTECTOR
arch/x86/kernel/kgdb.c +2 -1
@@ -50,6 +50,7 @@#include <asm/apicdef.h> #include <asm/apic.h> #include <asm/nmi.h> +#include <asm/switch_to.h> struct dbg_reg_def_t dbg_reg_def[DBG_MAX_REG_NUM ] = {
@@ -166,7 +167,7 @@ void sleeping_thread_to_gdb_regs(unsigned long *gdb_regs, struct task_struct *p) gdb_regs[GDB_DX ] =0 ; gdb_regs[GDB_SI ] =0 ; gdb_regs[GDB_DI ] =0 ; - gdb_regs[GDB_BP ] = *(unsigned long *)p->thread.sp; + gdb_regs[GDB_BP ] = ((struct inactive_task_frame *)p->thread.sp)->bp;#ifdef CONFIG_X86_32 gdb_regs[GDB_DS ] = __KERNEL_DS; gdb_regs[GDB_ES ] = __KERNEL_DS;
arch/x86/kernel/process.c +2 -1
@@ -32,6 +32,7 @@#include <asm/tlbflush.h> #include <asm/mce.h> #include <asm/vm86.h> +#include <asm/switch_to.h> /* * per-CPU TSS segments. Threads are completely 'soft' on Linux,
@@ -556,7 +557,7 @@ unsigned long get_wchan(struct task_struct *p)if (sp < bottom || sp > top)return 0 ; - fp =READ_ONCE_NOCHECK (*(unsigned long *)sp); + fp =READ_ONCE_NOCHECK (((struct inactive_task_frame *)sp)->bp); do { if (fp < bottom || fp > top) return 0;
PATCH v3 4/7 x86: Rewrite switch_to() code
In reply to Brian GerstMove 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.
--- arch/x86/entry/entry_32.S | 37 ++++++++++ arch/x86/entry/entry_64.S | 41 ++++++++++- 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 | 9 ++- arch/x86/kernel/process_64.c | 9 ++- arch/x86/kernel/smpboot.c | 1 - 11 files changed, 125 insertions(+), 130 deletions(-)
arch/x86/entry/entry_32.S +37 -0
@@ -204,6 +204,43 @@ 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 inactive_task_frame + */ + pushl %ebp + pushl %ebx + pushl %edi + pushl %esi + + /* 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 %esi + popl %edi + popl %ebx + popl %ebp + + jmp __switch_to +END(__switch_to_asm) + +/* + * A newly forked process directly context switches into this address. + * + * eax: prev task we switched from + */ ENTRY(ret_from_fork) pushl %eax call schedule_tail
arch/x86/entry/entry_64.S +38 -3
51 lines not on this page · raw message
arch/x86/include/asm/processor.h +0 -3
@@ -389,9 +389,6 @@ struct thread_struct {unsigned short fsindex;unsigned short gsindex;#endif -#ifdef CONFIG_X86_32 -unsigned long ip; -#endif #ifdef CONFIG_X86_64 unsigned long fsbase;unsigned long gsbase;
arch/x86/include/asm/switch_to.h +21 -116
156 lines not on this page · raw message
arch/x86/include/asm/thread_info.h +0 -2
@@ -95,7 +95,6 @@ struct thread_info {#define TIF_UPROBE 12/* breakpointed or singlestepping */ #define TIF_NOTSC 16/* TSC is not accessible in userland */ #define TIF_IA32 17/* IA32 compatibility process */ -#define TIF_FORK 18/* ret_from_fork */ #define TIF_NOHZ 19/* in adaptive nohz mode */ #define TIF_MEMDIE 20/* is terminating due to OOM killer */ #define TIF_POLLING_NRFLAG 21/* idle is polling for TIF_NEED_RESCHED */
@@ -119,7 +118,6 @@ struct thread_info {#define _TIF_UPROBE (1 << TIF_UPROBE)#define _TIF_NOTSC (1 << TIF_NOTSC)#define _TIF_IA32 (1 << TIF_IA32) -#define _TIF_FORK (1 << TIF_FORK)#define _TIF_NOHZ (1 << TIF_NOHZ)#define _TIF_POLLING_NRFLAG (1 << TIF_POLLING_NRFLAG)#define _TIF_IO_BITMAP (1 << TIF_IO_BITMAP)
arch/x86/kernel/asm-offsets.c +6 -0
@@ -29,6 +29,12 @@void common (void ) {BLANK (); +OFFSET (TASK_threadsp, task_struct, thread.sp); +#ifdef CONFIG_CC_STACKPROTECTOR +OFFSET (TASK_stack_canary, task_struct, stack_canary); +#endif + +BLANK ();OFFSET (TI_flags, thread_info, flags);OFFSET (TI_status, thread_info, status);
arch/x86/kernel/asm-offsets_32.c +5 -0
@@ -57,6 +57,11 @@ void foo(void)/* Size of SYSENTER_stack */ DEFINE (SIZEOF_SYSENTER_stack,sizeof (((struct tss_struct *)0 )->SYSENTER_stack)); +#ifdef CONFIG_CC_STACKPROTECTOR +BLANK (); +OFFSET (stack_canary_offset, stack_canary, canary); +#endif +#if defined(CONFIG_LGUEST ) || defined(CONFIG_LGUEST_GUEST ) || defined(CONFIG_LGUEST_MODULE )BLANK ();OFFSET (LGUEST_DATA_irq_enabled, lguest_data, irq_enabled);
arch/x86/kernel/asm-offsets_64.c +5 -0
@@ -56,6 +56,11 @@ int main(void)OFFSET (TSS_sp0, tss_struct, x86_tss.sp0);BLANK (); +#ifdef CONFIG_CC_STACKPROTECTOR +DEFINE (stack_canary_offset, offsetof(union irq_stack_union , stack_canary)); +BLANK (); +#endif +DEFINE (__NR_syscall_max,sizeof (syscalls_64) -1 );DEFINE (NR_syscalls,sizeof (syscalls_64));
arch/x86/kernel/process_32.c +6 -3
@@ -133,17 +133,20 @@ int copy_thread_tls(unsigned long clone_flags, unsigned long sp, unsigned long arg, struct task_struct *p, unsigned long tls) { struct pt_regs *childregs = task_pt_regs(p); + struct fork_frame *fork_frame = container_of(childregs, struct fork_frame, regs); +struct inactive_task_frame *frame = &fork_frame->frame;struct task_struct *tsk;int err; - p->thread.sp = (unsigned long ) childregs; + frame->bp =0 ; + p->thread.sp = (unsigned long ) fork_frame; p->thread.sp0 = (unsigned long ) (childregs+1 );memset (p->thread.ptrace_bps,0 ,sizeof (p->thread.ptrace_bps));if (unlikely (p->flags &PF_KTHREAD )) {/* kernel thread */ memset (childregs,0 ,sizeof (struct pt_regs )); - p->thread.ip = (unsigned long ) ret_from_kernel_thread; + frame->ret_addr = (unsigned long ) ret_from_kernel_thread;task_user_gs (p) = __KERNEL_STACK_CANARY; childregs->ds = __USER_DS; childregs->es = __USER_DS;
@@ -161,7 +164,7 @@ int copy_thread_tls(unsigned long clone_flags, unsigned long sp,if (sp) childregs->sp = sp; - p->thread.ip = (unsigned long ) ret_from_fork; + frame->ret_addr = (unsigned long ) ret_from_fork;task_user_gs (p) =get_user_gs (current_pt_regs ()); p->thread.io_bitmap_ptr =NULL ;
arch/x86/kernel/process_64.c +7 -2
@@ -141,12 +141,17 @@ int copy_thread_tls(unsigned long clone_flags, unsigned long sp, {int err;struct pt_regs *childregs; +struct fork_frame *fork_frame; +struct inactive_task_frame *frame;struct task_struct *me = current; p->thread.sp0 = (unsigned long )task_stack_page (p) +THREAD_SIZE ; childregs =task_pt_regs (p); - p->thread.sp = (unsigned long ) childregs; -set_tsk_thread_flag (p,TIF_FORK ); + fork_frame = container_of(childregs, struct fork_frame, regs); + frame = &fork_frame->frame; + frame->bp =0 ; + frame->ret_addr = (unsigned long ) ret_from_fork; + p->thread.sp = (unsigned long ) fork_frame; p->thread.io_bitmap_ptr =NULL ;savesegment (gs, p->thread.gsindex);
arch/x86/kernel/smpboot.c +0 -1
@@ -935,7 +935,6 @@ void common_cpu_up(unsigned int cpu, struct task_struct *idle)per_cpu (cpu_current_top_of_stack, cpu) = (unsigned long )task_stack_page (idle) +THREAD_SIZE ;#else -clear_tsk_thread_flag (idle,TIF_FORK ); initial_gs =per_cpu_offset (cpu);#endif }
Signature · 1 line
2.5.5
tip-bot for Brian Gerst
merge bot[tip:x86/asm] sched/x86: Rewrite the switch_to() code
sched/x86: Rewrite the switch_to() code
Commit-ID: 0100301bfdf56a2a370c7157b5ab0fbf9313e1cd Gitweb: http://git.kernel.org/tip/0100301bfdf56a2a370c7157b5ab0fbf9313e1cd Author: Brian Gerst <[email protected]> AuthorDate: Sat, 13 Aug 2016 12:38:19 -0400 Committer: Ingo Molnar <[email protected]> CommitDate: Wed, 24 Aug 2016 12:31:41 +0200
sched/x86: Rewrite the switch_to() code
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.
--- arch/x86/entry/entry_32.S | 37 ++++++++++ arch/x86/entry/entry_64.S | 41 ++++++++++- arch/x86/include/asm/processor.h | 3 - arch/x86/include/asm/switch_to.h | 139 ++++++------------------------------- 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 | 9 ++- arch/x86/kernel/process_64.c | 9 ++- arch/x86/kernel/smpboot.c | 1 - 11 files changed, 125 insertions(+), 132 deletions(-)
arch/x86/entry/entry_32.S +37 -0
@@ -204,6 +204,43 @@ 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 inactive_task_frame + */ + pushl %ebp + pushl %ebx + pushl %edi + pushl %esi + + /* 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 %esi + popl %edi + popl %ebx + popl %ebp + + jmp __switch_to +END(__switch_to_asm) + +/* + * A newly forked process directly context switches into this address. + * + * eax: prev task we switched from + */ ENTRY(ret_from_fork) pushl %eax call schedule_tail
arch/x86/entry/entry_64.S +38 -3
51 lines not on this page · raw message
arch/x86/include/asm/processor.h +0 -3
@@ -389,9 +389,6 @@ struct thread_struct {unsigned short fsindex;unsigned short gsindex;#endif -#ifdef CONFIG_X86_32 -unsigned long ip; -#endif #ifdef CONFIG_X86_64 unsigned long fsbase;unsigned long gsbase;
arch/x86/include/asm/switch_to.h +21 -118
160 lines not on this page · raw message
arch/x86/include/asm/thread_info.h +0 -2
@@ -95,7 +95,6 @@ struct thread_info {#define TIF_UPROBE 12/* breakpointed or singlestepping */ #define TIF_NOTSC 16/* TSC is not accessible in userland */ #define TIF_IA32 17/* IA32 compatibility process */ -#define TIF_FORK 18/* ret_from_fork */ #define TIF_NOHZ 19/* in adaptive nohz mode */ #define TIF_MEMDIE 20/* is terminating due to OOM killer */ #define TIF_POLLING_NRFLAG 21/* idle is polling for TIF_NEED_RESCHED */
@@ -119,7 +118,6 @@ struct thread_info {#define _TIF_UPROBE (1 << TIF_UPROBE)#define _TIF_NOTSC (1 << TIF_NOTSC)#define _TIF_IA32 (1 << TIF_IA32) -#define _TIF_FORK (1 << TIF_FORK)#define _TIF_NOHZ (1 << TIF_NOHZ)#define _TIF_POLLING_NRFLAG (1 << TIF_POLLING_NRFLAG)#define _TIF_IO_BITMAP (1 << TIF_IO_BITMAP)
arch/x86/kernel/asm-offsets.c +6 -0
@@ -29,6 +29,12 @@void common (void ) {BLANK (); +OFFSET (TASK_threadsp, task_struct, thread.sp); +#ifdef CONFIG_CC_STACKPROTECTOR +OFFSET (TASK_stack_canary, task_struct, stack_canary); +#endif + +BLANK ();OFFSET (TI_flags, thread_info, flags);OFFSET (TI_status, thread_info, status);
arch/x86/kernel/asm-offsets_32.c +5 -0
@@ -57,6 +57,11 @@ void foo(void)/* Size of SYSENTER_stack */ DEFINE (SIZEOF_SYSENTER_stack,sizeof (((struct tss_struct *)0 )->SYSENTER_stack)); +#ifdef CONFIG_CC_STACKPROTECTOR +BLANK (); +OFFSET (stack_canary_offset, stack_canary, canary); +#endif +#if defined(CONFIG_LGUEST ) || defined(CONFIG_LGUEST_GUEST ) || defined(CONFIG_LGUEST_MODULE )BLANK ();OFFSET (LGUEST_DATA_irq_enabled, lguest_data, irq_enabled);
arch/x86/kernel/asm-offsets_64.c +5 -0
@@ -56,6 +56,11 @@ int main(void)OFFSET (TSS_sp0, tss_struct, x86_tss.sp0);BLANK (); +#ifdef CONFIG_CC_STACKPROTECTOR +DEFINE (stack_canary_offset, offsetof(union irq_stack_union , stack_canary)); +BLANK (); +#endif +DEFINE (__NR_syscall_max,sizeof (syscalls_64) -1 );DEFINE (NR_syscalls,sizeof (syscalls_64));
arch/x86/kernel/process_32.c +6 -3
@@ -133,17 +133,20 @@ int copy_thread_tls(unsigned long clone_flags, unsigned long sp, unsigned long arg, struct task_struct *p, unsigned long tls) { struct pt_regs *childregs = task_pt_regs(p); + struct fork_frame *fork_frame = container_of(childregs, struct fork_frame, regs); +struct inactive_task_frame *frame = &fork_frame->frame;struct task_struct *tsk;int err; - p->thread.sp = (unsigned long ) childregs; + frame->bp =0 ; + p->thread.sp = (unsigned long ) fork_frame; p->thread.sp0 = (unsigned long ) (childregs+1 );memset (p->thread.ptrace_bps,0 ,sizeof (p->thread.ptrace_bps));if (unlikely (p->flags &PF_KTHREAD )) {/* kernel thread */ memset (childregs,0 ,sizeof (struct pt_regs )); - p->thread.ip = (unsigned long ) ret_from_kernel_thread; + frame->ret_addr = (unsigned long ) ret_from_kernel_thread;task_user_gs (p) = __KERNEL_STACK_CANARY; childregs->ds = __USER_DS; childregs->es = __USER_DS;
@@ -161,7 +164,7 @@ int copy_thread_tls(unsigned long clone_flags, unsigned long sp,if (sp) childregs->sp = sp; - p->thread.ip = (unsigned long ) ret_from_fork; + frame->ret_addr = (unsigned long ) ret_from_fork;task_user_gs (p) =get_user_gs (current_pt_regs ()); p->thread.io_bitmap_ptr =NULL ;
arch/x86/kernel/process_64.c +7 -2
@@ -141,12 +141,17 @@ int copy_thread_tls(unsigned long clone_flags, unsigned long sp, {int err;struct pt_regs *childregs; +struct fork_frame *fork_frame; +struct inactive_task_frame *frame;struct task_struct *me = current; p->thread.sp0 = (unsigned long )task_stack_page (p) +THREAD_SIZE ; childregs =task_pt_regs (p); - p->thread.sp = (unsigned long ) childregs; -set_tsk_thread_flag (p,TIF_FORK ); + fork_frame = container_of(childregs, struct fork_frame, regs); + frame = &fork_frame->frame; + frame->bp =0 ; + frame->ret_addr = (unsigned long ) ret_from_fork; + p->thread.sp = (unsigned long ) fork_frame; p->thread.io_bitmap_ptr =NULL ;savesegment (gs, p->thread.gsindex);
arch/x86/kernel/smpboot.c +0 -1
@@ -942,7 +942,6 @@ void common_cpu_up(unsigned int cpu, struct task_struct *idle)per_cpu (cpu_current_top_of_stack, cpu) = (unsigned long )task_stack_page (idle) +THREAD_SIZE ;#else -clear_tsk_thread_flag (idle,TIF_FORK ); initial_gs =per_cpu_offset (cpu);#endif }
PATCH v3 5/7 x86: Pass kernel thread parameters in fork_frame
In reply to Brian GerstInstead of setting up a fake pt_regs context, put the kernel thread function pointer and arg into the unused callee-restored registers of struct fork_frame.
--- arch/x86/entry/entry_32.S | 31 +++++++++++++++---------------- arch/x86/entry/entry_64.S | 37 +++++++++++++++++-------------------- arch/x86/include/asm/switch_to.h | 2 ++ arch/x86/kernel/process_32.c | 18 ++++-------------- arch/x86/kernel/process_64.c | 12 +++--------- 5 files changed, 41 insertions(+), 59 deletions(-)
arch/x86/entry/entry_32.S +15 -16
@@ -240,35 +240,34 @@ END(__switch_to_asm) * A newly forked process directly context switches into this address. * * eax: prev task we switched from + * ebx: kernel thread func (NULL for user thread) + * edi: kernel thread arg */ ENTRY(ret_from_fork) pushl %eax call schedule_tail popl %eax + testl %ebx, %ebx + jnz 1f /* kernel threads are uncommon */ + +2: /* When we fork, we trace the syscall return in the child, too. */ movl %esp, %eax call syscall_return_slowpath jmp restore_all -END(ret_from_fork) - -ENTRY(ret_from_kernel_thread) - pushl %eax - call schedule_tail - popl %eax - movl PT_EBP(%esp), %eax - call *PT_EBX(%esp) - movl $0, PT_EAX(%esp) + /* kernel thread */ +1: movl %edi, %eax + call *%ebx /* - * Kernel threads return to userspace as if returning from a syscall. - * We should check whether anything actually uses this path and, if so, - * consider switching it over to ret_from_fork. + * A kernel thread is allowed to return here after successfully + * calling do_execve(). Exit to userspace to complete the execve() + * syscall. */ - movl %esp, %eax - call syscall_return_slowpath - jmp restore_all -ENDPROC(ret_from_kernel_thread) + movl $0, PT_EAX(%esp) + jmp 2b +END(ret_from_fork) /* * Return to user mode is not as complex as all this looks,
arch/x86/entry/entry_64.S +17 -20
@@ -407,37 +407,34 @@ END(__switch_to_asm) * A newly forked process directly context switches into this address. * * rax: prev task we switched from + * rbx: kernel thread func (NULL for user thread) + * r12: kernel thread arg */ ENTRY(ret_from_fork) movq %rax, %rdi call schedule_tail /* rdi: 'prev' task parameter */ - testb $3, CS(%rsp) /* from kernel_thread? */ - jnz 1f - - /* - * We came from kernel_thread. This code path is quite twisted, and - * someone should clean it up. - * - * copy_thread_tls stashes the function pointer in RBX and the - * parameter to be passed in RBP. The called function is permitted - * to call do_execve and thereby jump to user mode. - */ - movq RBP(%rsp), %rdi - call *RBX(%rsp) - movl $0, RAX(%rsp) - - /* - * Fall through as though we're exiting a syscall. This makes a - * twisted sort of sense if we just called do_execve. - */ + testq %rbx, %rbx /* from kernel_thread? */ + jnz 1f /* kernel threads are uncommon */ -1: +2: movq %rsp, %rdi call syscall_return_slowpath /* returns with IRQs disabled */ TRACE_IRQS_ON /* user mode is traced as IRQS on */ SWAPGS jmp restore_regs_and_iret + +1: + /* kernel thread */ + movq %r12, %rdi + call *%rbx + /* + * A kernel thread is allowed to return here after successfully + * calling do_execve(). Exit to userspace to complete the execve() + * syscall. + */ + movq $0, RAX(%rsp) + jmp 2b END(ret_from_fork) /*
arch/x86/include/asm/switch_to.h +2 -0
@@ -12,6 +12,8 @@ struct tss_struct;void __switch_to_xtra (struct task_struct *prev_p,struct task_struct *next_p,struct tss_struct *tss); +asmlinkage void ret_from_fork(void); +/* data that is pointed to by thread.sp */ struct inactive_task_frame {#ifdef CONFIG_X86_64
arch/x86/kernel/process_32.c +4 -14
@@ -55,9 +55,6 @@#include <asm/switch_to.h> #include <asm/vm86.h> -asmlinkage void ret_from_fork(void) __asm__("ret_from_fork"); -asmlinkage void ret_from_kernel_thread(void) __asm__("ret_from_kernel_thread"); -/* * Return saved PC of a blocked thread. */
@@ -139,6 +136,7 @@ int copy_thread_tls(unsigned long clone_flags, unsigned long sp,int err; frame->bp =0 ; + frame->ret_addr = (unsigned long ) ret_from_fork; p->thread.sp = (unsigned long ) fork_frame; p->thread.sp0 = (unsigned long ) (childregs+1 );memset (p->thread.ptrace_bps,0 ,sizeof (p->thread.ptrace_bps));
@@ -146,25 +144,17 @@ int copy_thread_tls(unsigned long clone_flags, unsigned long sp,if (unlikely (p->flags &PF_KTHREAD )) {/* kernel thread */ memset (childregs,0 ,sizeof (struct pt_regs )); - frame->ret_addr = (unsigned long ) ret_from_kernel_thread; -task_user_gs (p) = __KERNEL_STACK_CANARY; - childregs->ds = __USER_DS; - childregs->es = __USER_DS; - childregs->fs = __KERNEL_PERCPU; - childregs->bx = sp;/* function */ - childregs->bp = arg; - childregs->orig_ax =-1 ; - childregs->cs = __KERNEL_CS |get_kernel_rpl (); - childregs->flags =X86_EFLAGS_IF |X86_EFLAGS_FIXED ; + frame->bx = sp;/* function */ + frame->di = arg; p->thread.io_bitmap_ptr =NULL ;return 0 ; } + frame->bx =0 ; *childregs = *current_pt_regs (); childregs->ax =0 ;if (sp) childregs->sp = sp; - frame->ret_addr = (unsigned long ) ret_from_fork;task_user_gs (p) =get_user_gs (current_pt_regs ()); p->thread.io_bitmap_ptr =NULL ;
arch/x86/kernel/process_64.c +3 -9
@@ -50,8 +50,6 @@#include <asm/switch_to.h> #include <asm/xen/hypervisor.h> -asmlinkage extern void ret_from_fork(void); -__visible DEFINE_PER_CPU (unsigned long ,rsp_scratch );/* Prints also some state that isn't saved in the pt_regs */
@@ -165,15 +163,11 @@ int copy_thread_tls(unsigned long clone_flags, unsigned long sp,if (unlikely (p->flags &PF_KTHREAD )) {/* kernel thread */ memset (childregs,0 ,sizeof (struct pt_regs )); - childregs->sp = (unsigned long )childregs; - childregs->ss = __KERNEL_DS; - childregs->bx = sp;/* function */ - childregs->bp = arg; - childregs->orig_ax =-1 ; - childregs->cs = __KERNEL_CS |get_kernel_rpl (); - childregs->flags =X86_EFLAGS_IF |X86_EFLAGS_FIXED ; + frame->bx = sp;/* function */ + frame->r12 = arg;return 0 ; } + frame->bx =0 ; *childregs = *current_pt_regs (); childregs->ax =0 ;
Signature · 1 line
2.5.5
tip-bot for Brian Gerst
merge bot[tip:x86/asm] sched/x86: Pass kernel thread parameters in 'struct fork_frame'
sched/x86: Pass kernel thread parameters in 'struct fork_frame'
Commit-ID: 616d24835eeafa8ef3466479db028abfdfc77531 Gitweb: http://git.kernel.org/tip/616d24835eeafa8ef3466479db028abfdfc77531 Author: Brian Gerst <[email protected]> AuthorDate: Sat, 13 Aug 2016 12:38:20 -0400 Committer: Ingo Molnar <[email protected]> CommitDate: Wed, 24 Aug 2016 12:31:50 +0200
sched/x86: Pass kernel thread parameters in 'struct fork_frame'
Instead of setting up a fake pt_regs context, put the kernel thread function pointer and arg into the unused callee-restored registers of 'struct fork_frame'.
--- arch/x86/entry/entry_32.S | 31 +++++++++++++++---------------- arch/x86/entry/entry_64.S | 37 +++++++++++++++++-------------------- arch/x86/include/asm/switch_to.h | 2 ++ arch/x86/kernel/process_32.c | 18 ++++-------------- arch/x86/kernel/process_64.c | 12 +++--------- 5 files changed, 41 insertions(+), 59 deletions(-)
arch/x86/entry/entry_32.S +15 -16
@@ -240,35 +240,34 @@ END(__switch_to_asm) * A newly forked process directly context switches into this address. * * eax: prev task we switched from + * ebx: kernel thread func (NULL for user thread) + * edi: kernel thread arg */ ENTRY(ret_from_fork) pushl %eax call schedule_tail popl %eax + testl %ebx, %ebx + jnz 1f /* kernel threads are uncommon */ + +2: /* When we fork, we trace the syscall return in the child, too. */ movl %esp, %eax call syscall_return_slowpath jmp restore_all -END(ret_from_fork) - -ENTRY(ret_from_kernel_thread) - pushl %eax - call schedule_tail - popl %eax - movl PT_EBP(%esp), %eax - call *PT_EBX(%esp) - movl $0, PT_EAX(%esp) + /* kernel thread */ +1: movl %edi, %eax + call *%ebx /* - * Kernel threads return to userspace as if returning from a syscall. - * We should check whether anything actually uses this path and, if so, - * consider switching it over to ret_from_fork. + * A kernel thread is allowed to return here after successfully + * calling do_execve(). Exit to userspace to complete the execve() + * syscall. */ - movl %esp, %eax - call syscall_return_slowpath - jmp restore_all -ENDPROC(ret_from_kernel_thread) + movl $0, PT_EAX(%esp) + jmp 2b +END(ret_from_fork) /* * Return to user mode is not as complex as all this looks,
arch/x86/entry/entry_64.S +17 -20
@@ -407,37 +407,34 @@ END(__switch_to_asm) * A newly forked process directly context switches into this address. * * rax: prev task we switched from + * rbx: kernel thread func (NULL for user thread) + * r12: kernel thread arg */ ENTRY(ret_from_fork) movq %rax, %rdi call schedule_tail /* rdi: 'prev' task parameter */ - testb $3, CS(%rsp) /* from kernel_thread? */ - jnz 1f - - /* - * We came from kernel_thread. This code path is quite twisted, and - * someone should clean it up. - * - * copy_thread_tls stashes the function pointer in RBX and the - * parameter to be passed in RBP. The called function is permitted - * to call do_execve and thereby jump to user mode. - */ - movq RBP(%rsp), %rdi - call *RBX(%rsp) - movl $0, RAX(%rsp) - - /* - * Fall through as though we're exiting a syscall. This makes a - * twisted sort of sense if we just called do_execve. - */ + testq %rbx, %rbx /* from kernel_thread? */ + jnz 1f /* kernel threads are uncommon */ -1: +2: movq %rsp, %rdi call syscall_return_slowpath /* returns with IRQs disabled */ TRACE_IRQS_ON /* user mode is traced as IRQS on */ SWAPGS jmp restore_regs_and_iret + +1: + /* kernel thread */ + movq %r12, %rdi + call *%rbx + /* + * A kernel thread is allowed to return here after successfully + * calling do_execve(). Exit to userspace to complete the execve() + * syscall. + */ + movq $0, RAX(%rsp) + jmp 2b END(ret_from_fork) /*
arch/x86/include/asm/switch_to.h +2 -0
@@ -34,6 +34,8 @@ static inline void prepare_switch_to(struct task_struct *prev,#endif } +asmlinkage void ret_from_fork(void); +/* data that is pointed to by thread.sp */ struct inactive_task_frame {#ifdef CONFIG_X86_64
arch/x86/kernel/process_32.c +4 -14
@@ -55,9 +55,6 @@#include <asm/switch_to.h> #include <asm/vm86.h> -asmlinkage void ret_from_fork(void) __asm__("ret_from_fork"); -asmlinkage void ret_from_kernel_thread(void) __asm__("ret_from_kernel_thread"); -/* * Return saved PC of a blocked thread. */
@@ -139,6 +136,7 @@ int copy_thread_tls(unsigned long clone_flags, unsigned long sp,int err; frame->bp =0 ; + frame->ret_addr = (unsigned long ) ret_from_fork; p->thread.sp = (unsigned long ) fork_frame; p->thread.sp0 = (unsigned long ) (childregs+1 );memset (p->thread.ptrace_bps,0 ,sizeof (p->thread.ptrace_bps));
@@ -146,25 +144,17 @@ int copy_thread_tls(unsigned long clone_flags, unsigned long sp,if (unlikely (p->flags &PF_KTHREAD )) {/* kernel thread */ memset (childregs,0 ,sizeof (struct pt_regs )); - frame->ret_addr = (unsigned long ) ret_from_kernel_thread; -task_user_gs (p) = __KERNEL_STACK_CANARY; - childregs->ds = __USER_DS; - childregs->es = __USER_DS; - childregs->fs = __KERNEL_PERCPU; - childregs->bx = sp;/* function */ - childregs->bp = arg; - childregs->orig_ax =-1 ; - childregs->cs = __KERNEL_CS |get_kernel_rpl (); - childregs->flags =X86_EFLAGS_IF |X86_EFLAGS_FIXED ; + frame->bx = sp;/* function */ + frame->di = arg; p->thread.io_bitmap_ptr =NULL ;return 0 ; } + frame->bx =0 ; *childregs = *current_pt_regs (); childregs->ax =0 ;if (sp) childregs->sp = sp; - frame->ret_addr = (unsigned long ) ret_from_fork;task_user_gs (p) =get_user_gs (current_pt_regs ()); p->thread.io_bitmap_ptr =NULL ;
arch/x86/kernel/process_64.c +3 -9
@@ -50,8 +50,6 @@#include <asm/switch_to.h> #include <asm/xen/hypervisor.h> -asmlinkage extern void ret_from_fork(void); -__visible DEFINE_PER_CPU (unsigned long ,rsp_scratch );/* Prints also some state that isn't saved in the pt_regs */
@@ -165,15 +163,11 @@ int copy_thread_tls(unsigned long clone_flags, unsigned long sp,if (unlikely (p->flags &PF_KTHREAD )) {/* kernel thread */ memset (childregs,0 ,sizeof (struct pt_regs )); - childregs->sp = (unsigned long )childregs; - childregs->ss = __KERNEL_DS; - childregs->bx = sp;/* function */ - childregs->bp = arg; - childregs->orig_ax =-1 ; - childregs->cs = __KERNEL_CS |get_kernel_rpl (); - childregs->flags =X86_EFLAGS_IF |X86_EFLAGS_FIXED ; + frame->bx = sp;/* function */ + frame->r12 = arg;return 0 ; } + frame->bx =0 ; *childregs = *current_pt_regs (); childregs->ax =0 ;
PATCH v3 6/7 x86: Fix thread_saved_pc()
In reply to Brian Gerstthread_saved_pc() was using a completely bogus method to get the return address. Since switch_to() was previously inlined, there was no sane way to know where on the stack the return address was stored. Now with the frame of a sleeping thread well defined, this can be implemented correctly.
--- arch/x86/include/asm/processor.h | 10 ++-------- arch/x86/kernel/process.c | 11 +++++++++++ arch/x86/kernel/process_32.c | 8 -------- 3 files changed, 13 insertions(+), 16 deletions(-)
arch/x86/include/asm/processor.h +2 -8
@@ -721,8 +721,6 @@ static inline void spin_lock_prefetch(const void *x) .addr_limit = KERNEL_DS, \ } -extern unsigned long thread_saved_pc (struct task_struct *tsk); -/* * TOP_OF_KERNEL_STACK_PADDING reserves 8 bytes on top of the ring0 stack. * This is necessary to guarantee that the entire "struct pt_regs"
@@ -773,17 +771,13 @@ extern unsigned long thread_saved_pc(struct task_struct *tsk); .addr_limit = KERNEL_DS, \ } -/* -* Return saved PC of a blocked thread. -* What is this good for? it will be always the scheduler or ret_from_fork. -*/ -#define thread_saved_pc (t) READ_ONCE_NOCHECK(*(unsigned long *)((t)->thread.sp - 8)) -#define task_pt_regs (tsk) ((struct pt_regs *)(tsk)->thread.sp0 - 1)extern unsigned long KSTK_ESP (struct task_struct *task);#endif /* CONFIG_X86_64 */ +extern unsigned long thread_saved_pc (struct task_struct *tsk); +extern void start_thread (struct pt_regs *regs,unsigned long new_ip,unsigned long new_sp);
arch/x86/kernel/process.c +11 -0
@@ -514,6 +514,17 @@ unsigned long arch_randomize_brk(struct mm_struct *mm) }/* +* Return saved PC of a blocked thread. +* What is this good for? it will be always the scheduler or ret_from_fork. +*/ +unsigned long thread_saved_pc (struct task_struct *tsk) +{ +struct inactive_task_frame *frame = + (struct inactive_task_frame *)READ_ONCE (tsk->thread.sp); +return READ_ONCE_NOCHECK (frame->ret_addr); +} + +/* * Called from fs/proc with a reference on @p to find the function * which called into schedule(). This needs to be done carefully * because the task might wake up and we might look at a stack
arch/x86/kernel/process_32.c +0 -8
@@ -55,14 +55,6 @@#include <asm/switch_to.h> #include <asm/vm86.h> -/* -* Return saved PC of a blocked thread. -*/ -unsigned long thread_saved_pc (struct task_struct *tsk) -{ -return ((unsigned long *)tsk->thread.sp)[3 ]; -} -void __show_regs (struct pt_regs *regs,int all) {unsigned long cr0 =0L , cr2 =0L , cr3 =0L , cr4 =0L ;
Signature · 1 line
2.5.5
tip-bot for Brian Gerst
merge bot[tip:x86/asm] sched/x86: Fix thread_saved_pc()
sched/x86: Fix thread_saved_pc()
Commit-ID: ffcb043ba524d3fbd979a9dac2c9ce8ad352000d Gitweb: http://git.kernel.org/tip/ffcb043ba524d3fbd979a9dac2c9ce8ad352000d Author: Brian Gerst <[email protected]> AuthorDate: Sat, 13 Aug 2016 12:38:21 -0400 Committer: Ingo Molnar <[email protected]> CommitDate: Wed, 24 Aug 2016 12:31:51 +0200
sched/x86: Fix thread_saved_pc()
thread_saved_pc() was using a completely bogus method to get the return address. Since switch_to() was previously inlined, there was no sane way to know where on the stack the return address was stored. Now with the frame of a sleeping thread well defined, this can be implemented correctly.
--- arch/x86/include/asm/processor.h | 10 ++-------- arch/x86/kernel/process.c | 11 +++++++++++ arch/x86/kernel/process_32.c | 8 -------- 3 files changed, 13 insertions(+), 16 deletions(-)
arch/x86/include/asm/processor.h +2 -8
@@ -721,8 +721,6 @@ static inline void spin_lock_prefetch(const void *x) .addr_limit = KERNEL_DS, \ } -extern unsigned long thread_saved_pc (struct task_struct *tsk); -/* * TOP_OF_KERNEL_STACK_PADDING reserves 8 bytes on top of the ring0 stack. * This is necessary to guarantee that the entire "struct pt_regs"
@@ -773,17 +771,13 @@ extern unsigned long thread_saved_pc(struct task_struct *tsk); .addr_limit = KERNEL_DS, \ } -/* -* Return saved PC of a blocked thread. -* What is this good for? it will be always the scheduler or ret_from_fork. -*/ -#define thread_saved_pc (t) READ_ONCE_NOCHECK(*(unsigned long *)((t)->thread.sp - 8)) -#define task_pt_regs (tsk) ((struct pt_regs *)(tsk)->thread.sp0 - 1)extern unsigned long KSTK_ESP (struct task_struct *task);#endif /* CONFIG_X86_64 */ +extern unsigned long thread_saved_pc (struct task_struct *tsk); +extern void start_thread (struct pt_regs *regs,unsigned long new_ip,unsigned long new_sp);
arch/x86/kernel/process.c +11 -0
@@ -514,6 +514,17 @@ unsigned long arch_randomize_brk(struct mm_struct *mm) }/* +* Return saved PC of a blocked thread. +* What is this good for? it will be always the scheduler or ret_from_fork. +*/ +unsigned long thread_saved_pc (struct task_struct *tsk) +{ +struct inactive_task_frame *frame = + (struct inactive_task_frame *)READ_ONCE (tsk->thread.sp); +return READ_ONCE_NOCHECK (frame->ret_addr); +} + +/* * Called from fs/proc with a reference on @p to find the function * which called into schedule(). This needs to be done carefully * because the task might wake up and we might look at a stack
arch/x86/kernel/process_32.c +0 -8
@@ -55,14 +55,6 @@#include <asm/switch_to.h> #include <asm/vm86.h> -/* -* Return saved PC of a blocked thread. -*/ -unsigned long thread_saved_pc (struct task_struct *tsk) -{ -return ((unsigned long *)tsk->thread.sp)[3 ]; -} -void __show_regs (struct pt_regs *regs,int all) {unsigned long cr0 =0L , cr2 =0L , cr3 =0L , cr4 =0L ;
PATCH v3 7/7 Revert "sched: Mark __schedule() stack frame as non-standard"
In reply to Brian GerstNow that the x86 switch_to() uses the standard C calling convention, STACK_FRAME_NON_STANDARD is no longer needed.
--- kernel/sched/core.c | 1 - 1 file changed, 1 deletion(-)
kernel/sched/core.c +0 -1
@@ -3384,7 +3384,6 @@ static void __sched notrace __schedule(bool preempt)balance_callback (rq); } -STACK_FRAME_NON_STANDARD (__schedule);/* switch_to() */ static inline void sched_submit_work (struct task_struct *tsk) {
Signature · 1 line
2.5.5
tip-bot for Brian Gerst
merge bot[tip:x86/asm] sched: Remove __schedule() non-standard frame annotation
sched: Remove __schedule() non-standard frame annotation
Commit-ID: 01175255fd8e3e993353a779f819ec8c0c59137e Gitweb: http://git.kernel.org/tip/01175255fd8e3e993353a779f819ec8c0c59137e Author: Brian Gerst <[email protected]> AuthorDate: Sat, 13 Aug 2016 12:38:22 -0400 Committer: Ingo Molnar <[email protected]> CommitDate: Wed, 24 Aug 2016 12:31:51 +0200
sched: Remove __schedule() non-standard frame annotation
Now that the x86 switch_to() uses the standard C calling convention, the STACK_FRAME_NON_STANDARD() annotation is no longer needed.
--- kernel/sched/core.c | 1 - 1 file changed, 1 deletion(-)
kernel/sched/core.c +0 -1
@@ -3381,7 +3381,6 @@ static void __sched notrace __schedule(bool preempt)balance_callback (rq); } -STACK_FRAME_NON_STANDARD (__schedule);/* switch_to() */ static inline void sched_submit_work (struct task_struct *tsk) {
Quoted 7 lines · Brian Gerst
This patch set simplifies the
switch_to()code, by moving the stack switch code out of line into an asm stub before calling__switch_to(). This ends up being more readable, and using the C calling convention instead of clobbering all registers improves code generation. It also allows newly forked processes to construct a special stack frame to seamlessly flow toret_from_fork, instead of using a test and branch, or an unbalanced call/ret.
Do you have performance numbers? Is it noticeable/measurable?
Linus
Quoted 10 lines · Linus Torvalds
Quoted 7 lines · Brian Gerst
This patch set simplifies the
switch_to()code, by moving the stack switch code out of line into an asm stub before calling__switch_to(). This ends up being more readable, and using the C calling convention instead of clobbering all registers improves code generation. It also allows newly forked processes to construct a special stack frame to seamlessly flow toret_from_fork, instead of using a test and branch, or an unbalanced call/ret.Do you have performance numbers? Is it noticeable/measurable?
How do I measure it? The perf documentation isn't easy to understand.
It shouldn't be a significant change. On a 64-bit defconfig build, __schedule() shrinks by 103 bytes. It's hard to analyse what exactly changes, but it's likely that GCC can allocate registers better without all the clobbers of the old inline asm version interfering. The new stub adds just 39 bytes.
--
Brian Gerst
Quoted 11 lines · Brian Gerst
Quoted 9 lines · Linus Torvalds
Quoted 7 lines · Brian Gerst
This patch set simplifies the
switch_to()code, by moving the stack switch code out of line into an asm stub before calling__switch_to(). This ends up being more readable, and using the C calling convention instead of clobbering all registers improves code generation. It also allows newly forked processes to construct a special stack frame to seamlessly flow toret_from_fork, instead of using a test and branch, or an unbalanced call/ret.
Quoted 3 lines
Quoted 1 line
Do you have performance numbers? Is it noticeable/measurable?
How do I measure it? The perf documentation isn't easy to understand.
Something like this:
taskset 1 perf stat -a -e '{instructions,cycles}' --repeat 10 perf bench sched pipe... will give a very good idea about the general impact of these changes on context switch overhead.
Thanks,
Ingo
Quoted 16 lines · Ingo Molnar
Quoted 13 lines · Brian Gerst
Quoted 10 lines · Linus Torvalds
Quoted 7 lines · Brian Gerst
This patch set simplifies the
switch_to()code, by moving the stack switch code out of line into an asm stub before calling__switch_to(). This ends up being more readable, and using the C calling convention instead of clobbering all registers improves code generation. It also allows newly forked processes to construct a special stack frame to seamlessly flow toret_from_fork, instead of using a test and branch, or an unbalanced call/ret.Do you have performance numbers? Is it noticeable/measurable?
Quoted 9 lines
Quoted 1 line
How do I measure it? The perf documentation isn't easy to understand.
Something like this:
taskset 1 perf stat -a -e '{instructions,cycles}' --repeat 10 perf bench sched pipe... will give a very good idea about the general impact of these changes on context switch overhead.
I will be quite surprised if you can measure any effect at all. I've never seen context switches take fewer than ~2k cycles, and on my laptop, they take 8k-9k cycles. The scheduler is really, really slow.
(Why doesn't that perf command show cycles per context switch?)
--Andy
Quoted 4 lines · Andy Lutomirski
I will be quite surprised if you can measure any effect at all. I've never seen context switches take fewer than ~2k cycles, and on my laptop, they take 8k-9k cycles. The scheduler is really, really slow.
Indeed, I think I still have a bugzilla entry somewhere regarding the huge performance regression when we first switched over to the current scheduler.
Over the years people have been trying to hack around it, e.g., network driver polling, low latency socket, anything to avoid going into the scheduler. We really should fix it properly some day.
Cheers,
Signature · 3 lines
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Quoted 16 lines · Ingo Molnar
Quoted 13 lines · Brian Gerst
Quoted 10 lines · Linus Torvalds
Quoted 7 lines · Brian Gerst
This patch set simplifies the
switch_to()code, by moving the stack switch code out of line into an asm stub before calling__switch_to(). This ends up being more readable, and using the C calling convention instead of clobbering all registers improves code generation. It also allows newly forked processes to construct a special stack frame to seamlessly flow toret_from_fork, instead of using a test and branch, or an unbalanced call/ret.Do you have performance numbers? Is it noticeable/measurable?
Quoted 8 lines
Quoted 1 line
How do I measure it? The perf documentation isn't easy to understand.
Something like this:
taskset 1 perf stat -a -e '{instructions,cycles}' --repeat 10 perf bench sched pipe... will give a very good idea about the general impact of these changes on context switch overhead.
Before:
Performance counter stats for 'system wide' (10 runs):
12,010,932,128 instructions # 1.03 insn per
cycle ( +- 0.31% )
11,691,797,513 cycles
( +- 0.76% )
3.487329979 seconds time elapsed
( +- 0.78% )After:
Performance counter stats for 'system wide' (10 runs):
12,097,706,506 instructions # 1.04 insn per
cycle ( +- 0.14% )
11,612,167,742 cycles
( +- 0.81% )
3.451278789 seconds time elapsed
( +- 0.82% )The numbers with or without this patch series are roughly the same. There is noticeable variation in the numbers each time I run it, so I'm not sure how good of a benchmark this is.
--
Brian Gerst
Quoted 29 lines · Brian Gerst
Quoted 6 lines
Something like this:
taskset 1 perf stat -a -e '{instructions,cycles}' --repeat 10 perf bench sched pipe... will give a very good idea about the general impact of these changes on context switch overhead.
Before:
Performance counter stats for 'system wide' (10 runs):12,010,932,128 instructions # 1.03 insn per cycle ( +- 0.31% ) 11,691,797,513 cycles ( +- 0.76% ) 3.487329979 seconds time elapsed ( +- 0.78% )After:
Performance counter stats for 'system wide' (10 runs):12,097,706,506 instructions # 1.04 insn per cycle ( +- 0.14% ) 11,612,167,742 cycles ( +- 0.81% ) 3.451278789 seconds time elapsed ( +- 0.82% )
Quoted 3 lines
The numbers with or without this patch series are roughly the same. There is noticeable variation in the numbers each time I run it, so I'm not sure how good of a benchmark this is.
Weird, I get an order of magnitude lower noise:
triton:~/tip> taskset 1 perf stat -a -e '{instructions,cycles}' --repeat 10 perf bench sched pipe >/dev/null
Performance counter stats for 'system wide' (10 runs):
11,503,026,062 instructions # 1.23 insn per cycle ( +- 2.64% )
9,377,410,613 cycles ( +- 2.05% )
1.669425407 seconds time elapsed ( +- 0.12% )
But note that I also had '--sync' for perf stat and did a >/dev/null at the end to make sure no terminal output and subsequent Xorg activities interfere. Also, full screen terminal.
Maybe try 'taskset 4' as well to put the workload on another CPU, if the first CPU is busier than the others?
(Any Hyperthreading on your test system?)
Thanks,
Ingo
Quoted 47 lines · Ingo Molnar
Quoted 29 lines · Brian Gerst
Quoted 6 lines
Something like this:
taskset 1 perf stat -a -e '{instructions,cycles}' --repeat 10 perf bench sched pipe... will give a very good idea about the general impact of these changes on context switch overhead.
Before:
Performance counter stats for 'system wide' (10 runs):12,010,932,128 instructions # 1.03 insn per cycle ( +- 0.31% ) 11,691,797,513 cycles ( +- 0.76% ) 3.487329979 seconds time elapsed ( +- 0.78% )After:
Performance counter stats for 'system wide' (10 runs):12,097,706,506 instructions # 1.04 insn per cycle ( +- 0.14% ) 11,612,167,742 cycles ( +- 0.81% ) 3.451278789 seconds time elapsed ( +- 0.82% )Quoted 3 lines
The numbers with or without this patch series are roughly the same. There is noticeable variation in the numbers each time I run it, so I'm not sure how good of a benchmark this is.
Weird, I get an order of magnitude lower noise:
triton:~/tip> taskset 1 perf stat -a -e '{instructions,cycles}' --repeat 10 perf bench sched pipe >/dev/null
Performance counter stats for 'system wide' (10 runs):
11,503,026,062 instructions # 1.23 insn per cycle ( +- 2.64% )
9,377,410,613 cycles ( +- 2.05% )1.669425407 seconds time elapsed ( +- 0.12% )
Quoted 8 lines
But note that I also had '--sync' for perf stat and did a >/dev/null at the end to make sure no terminal output and subsequent Xorg activities interfere. Also, full screen terminal.
Maybe try 'taskset 4' as well to put the workload on another CPU, if the first CPU is busier than the others?
(Any Hyperthreading on your test system?)
It is an AMD Phenom(tm) II X6 1055T, no hyperthreading.
--
Brian Gerst
Quoted 48 lines · Ingo Molnar
Quoted 29 lines · Brian Gerst
Quoted 6 lines
Something like this:
taskset 1 perf stat -a -e '{instructions,cycles}' --repeat 10 perf bench sched pipe... will give a very good idea about the general impact of these changes on context switch overhead.
Before:
Performance counter stats for 'system wide' (10 runs):12,010,932,128 instructions # 1.03 insn per cycle ( +- 0.31% ) 11,691,797,513 cycles ( +- 0.76% ) 3.487329979 seconds time elapsed ( +- 0.78% )After:
Performance counter stats for 'system wide' (10 runs):12,097,706,506 instructions # 1.04 insn per cycle ( +- 0.14% ) 11,612,167,742 cycles ( +- 0.81% ) 3.451278789 seconds time elapsed ( +- 0.82% )Quoted 3 lines
The numbers with or without this patch series are roughly the same. There is noticeable variation in the numbers each time I run it, so I'm not sure how good of a benchmark this is.
Weird, I get an order of magnitude lower noise:
triton:~/tip> taskset 1 perf stat -a -e '{instructions,cycles}' --repeat 10 perf bench sched pipe >/dev/null
Performance counter stats for 'system wide' (10 runs):
11,503,026,062 instructions # 1.23 insn per cycle ( +- 2.64% )
9,377,410,613 cycles ( +- 2.05% )1.669425407 seconds time elapsed ( +- 0.12% )
Quoted 9 lines
But note that I also had '--sync' for perf stat and did a >/dev/null at the end to make sure no terminal output and subsequent Xorg activities interfere. Also, full screen terminal.
Maybe try 'taskset 4' as well to put the workload on another CPU, if the first CPU is busier than the others?
(Any Hyperthreading on your test system?)
I've never investigated for real, but I suspect that cgroups are a big part of it. If you do a regular perf recording, I think you'll find that nearly all of the time is in the scheduler.
Quoted 8 lines · Brian Gerst
This patch set simplifies the
switch_to()code, by moving the stack switch code out of line into an asm stub before calling__switch_to(). This ends up being more readable, and using the C calling convention instead of clobbering all registers improves code generation. It also allows newly forked processes to construct a special stack frame to seamlessly flow toret_from_fork, instead of using a test and branch, or an unbalanced call/ret.
Quoted 4 lines
Changes from v2:
- Updated comments around kernel threads being uncommon for fork, etc.
- RemovedSTACK_FRAME_NON_STANDARDannotation from__schedule()per Josh Poimboeuf
- A few minor cleanups added
There are a few minor conflicts with my x86 stack dump patch set, but for the most part they should be orthogonal.
For the series:
Signature · 1 line
Josh