lkml.org 
[lkml]   [2007]   [Sep]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] i386: per-CPU double fault TSS and stack
Date

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/

\
 
 \ /
  Last update: 2007-09-01 12:37    [W:0.088 / U:0.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site