Re: i386 doublefault handler is broken with CONFIG_DEBUG_SPINLOCK

From: Chuck Ebbert
Date: Fri Aug 10 2007 - 15:11:44 EST


On 08/10/2007 04:23 AM, Andi Kleen wrote:
>
> This is the patch i came up with in the end. Passes testing.
> I also fixed some more minor things.

Looks good to me. Missed adding KERN_EMERG for the printk()
of the TSS address, though.

>
> Fix double fault handler
>
> From: Chuck Ebbert <cebbert@xxxxxxxxxx>
>
> The new percpu code has apparently broken the doublefault handler
> when CONFIG_DEBUG_SPINLOCK is set. Doublefault is handled by
> a hardware task, making the check
>
> SPIN_BUG_ON(lock->owner == current, lock, "recursion");
>
> fault because it uses the FS register to access the percpu data
> for current, and that register is zero in the new TSS. (The trace
> I saw was on 2.6.20 where it was GS, but it looks like this will
> still happen with FS on 2.6.22.)
>
> Initializing FS in the doublefault_tss should fix it.
>
> AK: Also fix broken ptr_ok() and turn printks into KERN_EMERG
> AK: And add a PANIC prefix to make clear the system will hang
> AK: (e.g. x86-64 will recover)
>
> Signed-off-by: Chuck Ebbert <cebbert@xxxxxxxxxx>
> Signed-off-by: Andi Kleen <ak@xxxxxxx>
>
> arch/i386/kernel/doublefault.c | 1 +
> 1 file changed, 1 insertion(+)
>
> Index: linux/arch/i386/kernel/doublefault.c
> ===================================================================
> --- linux.orig/arch/i386/kernel/doublefault.c
> +++ linux/arch/i386/kernel/doublefault.c
> @@ -13,7 +13,7 @@
> static unsigned long doublefault_stack[DOUBLEFAULT_STACKSIZE];
> #define STACK_START (unsigned long)(doublefault_stack+DOUBLEFAULT_STACKSIZE)
>
> -#define ptr_ok(x) ((x) > PAGE_OFFSET && (x) < PAGE_OFFSET + 0x1000000)
> +#define ptr_ok(x) ((x) > PAGE_OFFSET && (x) < PAGE_OFFSET + MAXMEM)
>
> static void doublefault_fn(void)
> {
> @@ -23,7 +23,7 @@ static void doublefault_fn(void)
> store_gdt(&gdt_desc);
> gdt = gdt_desc.address;
>
> - printk("double fault, gdt at %08lx [%d bytes]\n", gdt, gdt_desc.size);
> + printk(KERN_EMERG "PANIC: double fault, gdt at %08lx [%d bytes]\n", gdt, gdt_desc.size);
>
> if (ptr_ok(gdt)) {
> gdt += GDT_ENTRY_TSS << 3;
> @@ -35,11 +35,11 @@ static void doublefault_fn(void)
> if (ptr_ok(tss)) {
> struct i386_hw_tss *t = (struct i386_hw_tss *)tss;
>
> - printk("eip = %08lx, esp = %08lx\n", t->eip, t->esp);
> + printk(KERN_EMERG "eip = %08lx, esp = %08lx\n", t->eip, t->esp);
>
> - printk("eax = %08lx, ebx = %08lx, ecx = %08lx, edx = %08lx\n",
> + printk(KERN_EMERG "eax = %08lx, ebx = %08lx, ecx = %08lx, edx = %08lx\n",
> t->eax, t->ebx, t->ecx, t->edx);
> - printk("esi = %08lx, edi = %08lx\n",
> + printk(KERN_EMERG "esi = %08lx, edi = %08lx\n",
> t->esi, t->edi);
> }
> }
> @@ -63,6 +63,7 @@ struct tss_struct doublefault_tss __cach
> .cs = __KERNEL_CS,
> .ss = __KERNEL_DS,
> .ds = __USER_DS,
> + .fs = __KERNEL_PERCPU,
>
> .__cr3 = __pa(swapper_pg_dir)
> }
>
>
>
>

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/