x86: Consolidate 32 and 64 bit thread_info
Subsystems (2)
Thomas, Ingo, and Peter(s),
On IRC, Peter Zijlstra mentioned to me the difference between 32bit and 64bit x86 on how thread_info is handled with interrupts. I mentioned that they should be more the same, and Peter agreed.
I decided to do so, and ended up with the following patches.
I marked this for v3.2 as it should be tested a bit before going into mainline. I've ran several configs on it, but more testing should be done.
Some of the patches are clean ups, but the main ones are patch 3 and 4 that do the work to convert 32bit of thread_info to be more like the 64bit.
-- Steve
Please pull the latest tip/x86/thread_info tree, which can be found at:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
tip/x86/thread_info
Head SHA1: a9db5af270460d73139d83e39d1dabc9c475a821
Steven Rostedt (5):
x86: Do not differentiate between x86_64 and i386 with init_per_cpu
x86: Nuke the supervisor_stack field in i386 thread_info
x86: Remove previous_esp from i386 thread_info structure
x86: Keep thread_info on thread stack in x86_32
x86: Clean up dumpstack_64.c code
----
arch/x86/include/asm/percpu.h | 4 +- arch/x86/include/asm/processor.h | 9 +++ arch/x86/include/asm/thread_info.h | 56 +++-------------- arch/x86/kernel/cpu/common.c | 8 +- arch/x86/kernel/dumpstack_32.c | 44 ++++++++++++- arch/x86/kernel/dumpstack_64.c | 121 +++++++++++++++++++++++++----------- arch/x86/kernel/irq_32.c | 86 +++++++++++--------------- arch/x86/kernel/process_32.c | 4 + arch/x86/kernel/smpboot.c | 2 +- 9 files changed, 191 insertions(+), 143 deletions(-)
PATCH 1/5 x86: Do not differentiate between x86_64 and i386 with init_per_cpu
From: Steven Rostedt <[email protected]>
There's nothing different between the way x86_64 and x86_32 treate init_per_cpu.
Although only x86_64 uses INIT_PER_CPU_VAR() in the head.S file, there is still no reason to keep the macro x86_64 only.
--- arch/x86/include/asm/percpu.h | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
arch/x86/include/asm/percpu.h +2 -2
@@ -33,7 +33,7 @@#define PER_CPU_VAR (var) var#endif /* SMP */ -#ifdef CONFIG_X86_64_SMP +#ifdef CONFIG_SMP #define INIT_PER_CPU_VAR (var) init_per_cpu__##var#else #define INIT_PER_CPU_VAR (var) var
@@ -77,7 +77,7 @@#define DECLARE_INIT_PER_CPU (var) \ extern typeof(var) init_per_cpu_var(var) -#ifdef CONFIG_X86_64_SMP +#ifdef CONFIG_SMP #define init_per_cpu_var (var) init_per_cpu__##var#else #define init_per_cpu_var (var) var
Signature · 1 line
1.7.5.4
Re: [PATCH 1/5] x86: Do not differentiate between x86_64 and i386 with init_per_cpu
Even though 32-bit doesn't currently use these macros, please keep the current define, since only 64-bit SMP is special in this regard. 64-bit SMP uses a zero-based percpu segment, which requires the linker fixup for statically initialized percpu pointers. This fixup isn't needed on 32-bit SMP (or UP).
--
Brian Gerst
Quoted 49 lines · Steven Rostedt
From: Steven Rostedt <[email protected]>
There's nothing different between the way
x86_64andx86_32treateinit_per_cpu.Although only
x86_64usesINIT_PER_CPU_VAR()in thehead.Sfile, there is still no reason to keep the macrox86_64only.Cc Thomas Gleixner <[email protected]>Cc Peter Zijlstra <[email protected]>Cc H. Peter Anvin <[email protected]>Cc Ingo Molnar <[email protected]>Signed-off-by Steven Rostedt <[email protected]>---
arch/x86/include/asm/percpu.h | 4 ++--1 files changed, 2 insertions(+), 2 deletions(-)arch/x86/include/asm/percpu.h +2
-3@@ -33,7 +33,7 @@ #define PER_CPU_VAR(var) var #endif /* SMP */ -#ifdef CONFIG_X86_64_SMP +#ifdef CONFIG_SMP #define INIT_PER_CPU_VAR(var) init_per_cpu__##var #else #define INIT_PER_CPU_VAR(var) var@@ -77,7 +77,7 @@ #define DECLARE_INIT_PER_CPU(var) \ extern typeof(var) init_per_cpu_var(var) -#ifdef CONFIG_X86_64_SMP +#ifdef CONFIG_SMP #define init_per_cpu_var(var) init_per_cpu__##var #else #define init_per_cpu_var(var) var --1.7.5.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/5] x86: Do not differentiate between x86_64 and i386 with init_per_cpu
Quoted 5 lines · Brian Gerst
Even though 32-bit doesn't currently use these macros, please keep the current define, since only 64-bit SMP is special in this regard. 64-bit SMP uses a zero-based percpu segment, which requires the linker fixup for statically initialized percpu pointers. This fixup isn't needed on 32-bit SMP (or UP).
OK, fine, I'll remove this patch from the set and rebase.
Thanks,
-- Steve
PATCH 2/5 x86: Nuke the supervisor_stack field in i386 thread_info
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
Nothing references the supervisor_stack in the thread_info field, and it does not exist in x86_64. To make the two more the same, it is being removed.
--- arch/x86/include/asm/thread_info.h | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
arch/x86/include/asm/thread_info.h +0 -1
@@ -38,7 +38,6 @@ struct thread_info {unsigned long previous_esp;/* ESP of the previous stack in case of nested (IRQ) stacks */ -__u8 supervisor_stack[0 ];#endif int uaccess_err; };
Signature · 1 line
1.7.5.4
PATCH 3/5 x86: Remove previous_esp from i386 thread_info structure
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
The i386 thread_info contains a previous_esp field that is used to daisy chain the different stacks for dump_stack() (ie. irq, softirq, thread stacks).
The goal is to eventual make i386 handling of thread_info the same as x86_64, which means that the thread_info will not be in the stack but as a per_cpu variable. We will no longer depend on thread_info being able to daisy chain different stacks as it will only exist in one location (the thread stack).
By moving previous_esp out of thread_info, and just placing it above the thread_info of the current stack, we can still use the stacks to daisy chain which stack was in use when preempted by an interrupt.
--- arch/x86/include/asm/thread_info.h | 5 ----- arch/x86/kernel/dumpstack_32.c | 10 +++++++++- arch/x86/kernel/irq_32.c | 13 +++++++++---- 3 files changed, 18 insertions(+), 10 deletions(-)
arch/x86/include/asm/thread_info.h +0 -5
@@ -34,11 +34,6 @@ struct thread_info {mm_segment_t addr_limit;struct restart_block restart_block;void __user *sysenter_return; -#ifdef CONFIG_X86_32 -unsigned long previous_esp;/* ESP of the previous stack in -case of nested (IRQ) stacks -*/ -#endif int uaccess_err; };
arch/x86/kernel/dumpstack_32.c +9 -1
@@ -22,6 +22,7 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs, const struct stacktrace_ops *ops, void *data) { int graph = 0; +u32 *prev_esp;if (!task) task = current;
@@ -44,9 +45,16 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs, ((unsigned long )stack & (~(THREAD_SIZE -1 ))); bp = ops->walk_stack (context, stack, bp, ops, data,NULL , &graph); - stack = (unsigned long *)context->previous_esp; +/* Stop if not on irq stack */ +if (task_stack_page (task) == context) +break ; + +/* The previous esp is just above the context */ + prev_esp = (u32 *) ((char *)context +sizeof (struct thread_info )); + stack = (unsigned long *)*prev_esp;if (!stack)break ; +if (ops->stack (data,"IRQ" ) <0 )break ;touch_nmi_watchdog ();
arch/x86/kernel/irq_32.c +9 -4
@@ -76,7 +76,7 @@ static inline int execute_on_irq_stack(int overflow, struct irq_desc *desc, int irq) { union irq_ctx *curctx, *irqctx; -u32 *isp, arg1, arg2; +u32 *isp, *prev_esp, arg1, arg2; curctx = (union irq_ctx *)current_thread_info (); irqctx =__this_cpu_read (hardirq_ctx);
@@ -93,7 +93,9 @@ execute_on_irq_stack(int overflow, struct irq_desc *desc, int irq)/* build the stack frame on the IRQ stack */ isp = (u32 *) ((char *)irqctx +sizeof (*irqctx)); irqctx->tinfo.task = curctx->tinfo.task; - irqctx->tinfo.previous_esp = current_stack_pointer; +/* Save the next esp after thread_info */ + prev_esp = (u32 *) ((char *)irqctx +sizeof (struct thread_info )); + *prev_esp = current_stack_pointer;/* * Copy the softirq bits in preempt_count so that the
@@ -154,7 +156,7 @@ asmlinkage void do_softirq(void)unsigned long flags;struct thread_info *curctx;union irq_ctx *irqctx; -u32 *isp; +u32 *isp, *prev_esp;if (in_interrupt ())return ;
@@ -165,11 +167,14 @@ asmlinkage void do_softirq(void) curctx =current_thread_info (); irqctx =__this_cpu_read (softirq_ctx); irqctx->tinfo.task = curctx->task; - irqctx->tinfo.previous_esp = current_stack_pointer;/* build the stack frame on the softirq stack */ isp = (u32 *) ((char *)irqctx +sizeof (*irqctx)); +/* Push the previous esp onto the stack */ + prev_esp = (u32 *) ((char *)irqctx +sizeof (struct thread_info )); + *prev_esp = current_stack_pointer; +call_on_stack (__do_softirq, isp);/* * Shouldn't happen, we returned above if in_interrupt():
Signature · 1 line
1.7.5.4
PATCH 4/5 x86: Keep thread_info on thread stack in x86_32
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
x86_64 uses a per_cpu variable kernel_stack to always point to the thread stack of current. This is where the thread_info is stored and is accessed from this location even when the irq or exception stack is in use. This removes the complexity of having to maintain the thread info on the stack when interrupts are running and having to copy the preempt_count and other fields to the interrupt stack.
x86_32 uses the old method of copying the thread_info from the thread stack to the exception stack just before executing the exception.
Having the two different requires #ifdefs and also the x86_32 way is a bit of a pain to maintain. By converting x86_32 to the same method of x86_64, we can remove #ifdefs, clean up the x86_32 code a little, and remove the overhead of the copy.
--- arch/x86/include/asm/processor.h | 9 ++++ arch/x86/include/asm/thread_info.h | 50 ++++------------------ arch/x86/kernel/cpu/common.c | 8 ++-- arch/x86/kernel/dumpstack_32.c | 40 +++++++++++++++--- arch/x86/kernel/irq_32.c | 81 ++++++++++++++---------------------- arch/x86/kernel/process_32.c | 4 ++ arch/x86/kernel/smpboot.c | 2 +- 7 files changed, 93 insertions(+), 101 deletions(-)
arch/x86/include/asm/processor.h +9 -0
@@ -414,6 +414,15 @@ struct stack_canary { }; DECLARE_PER_CPU_ALIGNED(struct stack_canary, stack_canary);#endif +/* +* per-CPU IRQ handling stacks +*/ +struct irq_stack { +u32 stack[THREAD_SIZE /sizeof (u32)]; +} __attribute__((aligned (THREAD_SIZE ))); + +DECLARE_PER_CPU(struct irq_stack *, hardirq_stack); +DECLARE_PER_CPU(struct irq_stack *, softirq_stack);#endif /* X86_64 */ extern unsigned int xstate_size;
arch/x86/include/asm/thread_info.h +10 -40
71 lines not on this page · raw message
arch/x86/kernel/cpu/common.c +4 -4
@@ -1019,6 +1019,10 @@ static __init int setup_disablecpuid(char *arg) }__setup ("clearcpuid=" , setup_disablecpuid); +DEFINE_PER_CPU(unsigned long, kernel_stack) = + (unsigned long )&init_thread_union -KERNEL_STACK_OFFSET +THREAD_SIZE ; +EXPORT_PER_CPU_SYMBOL (kernel_stack); +#ifdef CONFIG_X86_64 struct desc_ptr idt_descr = {NR_VECTORS *16 -1 , (unsigned long ) idt_table };
@@ -1033,10 +1037,6 @@ DEFINE_PER_CPU(struct task_struct *, current_task) ____cacheline_aligned = &init_task;EXPORT_PER_CPU_SYMBOL (current_task); -DEFINE_PER_CPU(unsigned long, kernel_stack) = - (unsigned long )&init_thread_union -KERNEL_STACK_OFFSET +THREAD_SIZE ; -EXPORT_PER_CPU_SYMBOL (kernel_stack); - DEFINE_PER_CPU(char *, irq_stack_ptr) =init_per_cpu_var (irq_stack_union.irq_stack) +IRQ_STACK_SIZE -64 ;
arch/x86/kernel/dumpstack_32.c +34 -6
@@ -16,11 +16,33 @@#include <asm/stacktrace.h> +static void *is_irq_stack (void *p,void *irq) +{ +if (p < irq || p >= (irq +THREAD_SIZE )) +return NULL ; +return irq +THREAD_SIZE ; +} + + +static void *is_hardirq_stack (unsigned long *stack,int cpu) +{ +void *irq =per_cpu (hardirq_stack, cpu); + +return is_irq_stack (stack, irq); +} + +static void *is_softirq_stack (unsigned long *stack,int cpu) +{ +void *irq =per_cpu (softirq_stack, cpu); + +return is_irq_stack (stack, irq); +}void dump_trace (struct task_struct *task,struct pt_regs *regs,unsigned long *stack,unsigned long bp,const struct stacktrace_ops *ops,void *data) { +const unsigned cpu =get_cpu ();int graph =0 ;u32 *prev_esp;
@@ -40,17 +62,22 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs,for (;;) {struct thread_info *context; +void *end_stack =NULL ; + + end_stack =is_hardirq_stack (stack, cpu); +if (!end_stack) + end_stack =is_softirq_stack (stack, cpu); - context = (struct thread_info *) - ((unsigned long )stack & (~(THREAD_SIZE -1 ))); - bp = ops->walk_stack (context, stack, bp, ops, data,NULL , &graph); + context =task_thread_info (task); + bp = ops->walk_stack (context, stack, bp, ops, data, + end_stack, &graph);/* Stop if not on irq stack */ -if (task_stack_page (task) == context) +if (!end_stack)break ; -/* The previous esp is just above the context */ - prev_esp = (u32 *) ((char *)context +sizeof (struct thread_info )); +/* The previous esp is saved on the bottom of the stack */ + prev_esp = (u32 *)(end_stack -THREAD_SIZE ); stack = (unsigned long *)*prev_esp;if (!stack)break ;
@@ -59,6 +86,7 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs,break ;touch_nmi_watchdog (); } +put_cpu (); }EXPORT_SYMBOL (dump_trace);
arch/x86/kernel/irq_32.c +31 -50
138 lines not on this page · raw message
arch/x86/kernel/process_32.c +4 -0
@@ -341,6 +341,10 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)if (get_kernel_rpl () &&unlikely (prev->iopl != next->iopl))set_iopl_mask (next->iopl); +percpu_write (kernel_stack, + (unsigned long )task_stack_page (next_p) + +THREAD_SIZE -KERNEL_STACK_OFFSET ); +/* * Now maybe handle debug registers and/or IO bitmaps */
arch/x86/kernel/smpboot.c +1 -1
@@ -715,10 +715,10 @@ do_rest:#else clear_tsk_thread_flag (c_idle.idle,TIF_FORK ); initial_gs =per_cpu_offset (cpu); +#endif per_cpu (kernel_stack, cpu) = (unsigned long )task_stack_page (c_idle.idle) -KERNEL_STACK_OFFSET +THREAD_SIZE ; -#endif early_gdt_descr.address = (unsigned long )get_cpu_gdt_table (cpu); initial_code = (unsigned long )start_secondary; stack_start = c_idle.idle->thread.sp;
Signature · 1 line
1.7.5.4
PATCH 5/5 x86: Clean up dumpstack_64.c code
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
The dump_trace() function in dumpstack_64.c is hard to follow. The test for exception stack is processed differently than the test for irq stack, and the normal stack is outside completely.
By restructuring this code to have all the stacks determined by a single function that returns an enum of the following:
STACK_IS_NORMAL
STACK_IS_EXCEPTION
STACK_IS_IRQ
STACK_IS_UNKNOWN
and has the logic of each within a switch statement. This should make the code much easier to read and understand.
--- arch/x86/kernel/dumpstack_64.c | 121 ++++++++++++++++++++++++++++------------ 1 files changed, 85 insertions(+), 36 deletions(-)
arch/x86/kernel/dumpstack_64.c +85 -36
@@ -132,6 +132,45 @@ fixup_bp_irq_link(unsigned long bp, unsigned long *stack,return bp; } +static const unsigned long irq_stack_size = + (IRQ_STACK_SIZE -64 ) /sizeof (unsigned long ); + +enum stack_type { +STACK_IS_UNKNOWN , +STACK_IS_NORMAL , +STACK_IS_EXCEPTION , +STACK_IS_IRQ , +}; + +static enum stack_type +analyze_stack (int cpu,struct task_struct *task, +unsigned long *stack,unsigned long **stack_end,char **id) +{ +unsigned long *irq_stack; +unsigned long addr; +unsigned used =0 ; + + addr = ((unsigned long )stack & (~(THREAD_SIZE -1 ))); +if ((unsigned long )task_stack_page (task) == addr) +return STACK_IS_NORMAL ; + + *stack_end =in_exception_stack (cpu, (unsigned long )stack, + &used, id); +if (*stack_end) +return STACK_IS_EXCEPTION ; + + *stack_end = (unsigned long *)per_cpu (irq_stack_ptr, cpu); +if (!*stack_end) +return STACK_IS_UNKNOWN ; + + irq_stack = *stack_end - irq_stack_size; + +if (in_irq_stack (stack, irq_stack, *stack_end)) +return STACK_IS_IRQ ; + +return STACK_IS_UNKNOWN ; +} +/* * x86-64 can have up to three kernel stacks: * process stack
@@ -144,12 +183,11 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs, const struct stacktrace_ops *ops, void *data) { const unsigned cpu = get_cpu(); -unsigned long *irq_stack_end = - (unsigned long *)per_cpu (irq_stack_ptr, cpu); -unsigned used =0 ;struct thread_info *tinfo; -int graph =0 ; +unsigned long *irq_stack;unsigned long dummy; +int graph =0 ; +int done =0 ;if (!task) task = current;
@@ -168,51 +206,62 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs,* exceptions */ tinfo =task_thread_info (task); -for (;;) { +while (!done) { +unsigned long *stack_end; +enum stack_type stype;char *id; -unsigned long *estack_end; - estack_end =in_exception_stack (cpu, (unsigned long )stack, - &used, &id); -if (estack_end) { + stype =analyze_stack (cpu, task, stack, &stack_end, &id); + +/* Default finish unless specified to continue */ + done =1 ; + +switch (stype) { + +/* Break out early if we are on the thread stack */ +case STACK_IS_NORMAL : +break ; + +case STACK_IS_EXCEPTION : +if (ops->stack (data, id) <0 )break ; bp = ops->walk_stack (tinfo, stack, bp, ops, - data, estack_end, &graph); + data, stack_end, &graph); ops->stack (data,"<EOE>" );/* * We link to the next stack via the * second-to-last pointer (index -2 to end) in the * exception stack: */ - stack = (unsigned long *) estack_end[-2 ]; -continue ; - } -if (irq_stack_end) { -unsigned long *irq_stack; - irq_stack = irq_stack_end - - (IRQ_STACK_SIZE -64 ) /sizeof (*irq_stack); - -if (in_irq_stack (stack, irq_stack, irq_stack_end)) { -if (ops->stack (data,"IRQ" ) <0 ) -break ; - bp = ops->walk_stack (tinfo, stack, bp, - ops, data, irq_stack_end, &graph); -/* -* We link to the next stack (which would be -* the process stack normally) the last -* pointer (index -1 to end) in the IRQ stack: -*/ - stack = (unsigned long *) (irq_stack_end[-1 ]); - bp =fixup_bp_irq_link (bp, stack, irq_stack, - irq_stack_end); - irq_stack_end =NULL ; - ops->stack (data,"EOI" ); -continue ; - } + stack = (unsigned long *) stack_end[-2 ]; + done =0 ; +break ; + +case STACK_IS_IRQ : + +if (ops->stack (data,"IRQ" ) <0 ) +break ; + bp = ops->walk_stack (tinfo, stack, bp, + ops, data, stack_end, &graph); +/* +* We link to the next stack (which would be +* the process stack normally) the last +* pointer (index -1 to end) in the IRQ stack: +*/ + stack = (unsigned long *) (stack_end[-1 ]); + irq_stack = stack_end - irq_stack_size; + bp =fixup_bp_irq_link (bp, stack, irq_stack, + stack_end); + ops->stack (data,"EOI" ); + done =0 ; +break ; + +case STACK_IS_UNKNOWN : + ops->stack (data,"UNK" ); +break ; } -break ; }/*
Signature · 1 line
1.7.5.4