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 [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
