Messages in this thread | | | From | Andi Kleen <> | Subject | Re: [PATCH] i386: per-CPU double fault TSS and stack | Date | Sat, 1 Sep 2007 12:33:12 +0200 |
| |
Can you cc the next version to Linus please? He's probably best qualified to review the i386 double fault handler because he wrote it originally. I must admit the code always scared me a bit.
> +#ifdef CONFIG_HOTPLUG_CPU > +static void *noinline __init_refok > +#else > +static inline void *__init > +#endif
I really wonder if there isn't a cleaner way to do that :-( These init reference checks are starting to become a major annoyance.
> +do_alloc_bootmem(unsigned long size, unsigned long align, unsigned long goal) > +{ > + return __alloc_bootmem(size, align, goal); > +} > + > /* > * cpu_init() initializes state that is per-CPU. Some data is already > * initialized (naturally) in the bootstrap process, such as the GDT > @@ -659,6 +669,9 @@ void switch_to_new_gdt(void) > void __cpuinit cpu_init(void) > { > int cpu = smp_processor_id(); > +#if N_EXCEPTION_TSS > + unsigned i; > +#endif
Would it be that bad to have the TSS even around without CONFIG_DOUBLEFAULT?
In fact I would prefer to just eliminate CONFIG_DOUBLEFAULT (imho it always a bad idea because the amount of code it saves is miniscule) instead of adding such a ifdef maze.
> -#ifdef CONFIG_DOUBLEFAULT > - /* Set up doublefault TSS pointer in the GDT */ > - __set_tss_desc(cpu, GDT_ENTRY_DOUBLEFAULT_TSS, &doublefault_tss); > +#if N_EXCEPTION_TSS > +#if EXCEPTION_STACK_ORDER > THREAD_ORDER > +#error Assertion failed: EXCEPTION_STACK_ORDER <= THREAD_ORDER > +#endif
BUILD_BUG_ON would look nicer
> + > + /* Set up exception handling stacks */ > +#ifdef CONFIG_SMP > + if (cpu) {
If you move the code after the gs pda setup you could use smp_processor_id() and avoid the ifdefs (on UP it expands to 0 so the optimizer would do it cleanly)
> + BUG_ON(page_count(page)); > + init_page_count(page); > + free_pages(stack, j); > + stack += (PAGE_SIZE << j);
In 2.4-aa I added a alloc_pages_exact() for this. I don't think such games should be played outside page_alloc.c. I would recommend to readd alloc_pages_exact() and then use it.
> -#define DOUBLEFAULT_STACKSIZE (1024) > -static unsigned long doublefault_stack[DOUBLEFAULT_STACKSIZE]; > -#define STACK_START (unsigned long)(doublefault_stack+DOUBLEFAULT_STACKSIZE) > +extern unsigned long max_low_pfn;
No externs in .c
> +#define ptr_ok(x, l) ((x) >= PAGE_OFFSET \ > + && (x) + (l) <= PAGE_OFFSET + max_low_pfn * PAGE_SIZE - 1) > > -#define ptr_ok(x) ((x) > PAGE_OFFSET && (x) < PAGE_OFFSET + MAXMEM) > +#define THREAD_INFO_FROM(x) ((struct thread_info *)((x) & ~(THREAD_SIZE - 1))) > > -static void doublefault_fn(void) > +register const struct i386_hw_tss *self __asm__("ebx");
Can't you just move that to a proper argument register in assembler code?
-Andi - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |