Messages in this thread | | | Date | Tue, 26 Aug 2008 16:48:14 -0400 | From | Mathieu Desnoyers <> | Subject | Re: 2.6.{26.2,27-rc} oops on virtualbox |
| |
* Gerhard Brauer (gerhard.brauer@web.de) wrote: > On Tue, Aug 26, 2008 at 02:15:58PM -0400, Mathieu Desnoyers wrote: > > > > Ok, it might still be caused by paravirt and alternatives instruction > > patching. What if you also do : > > > > alternative_instructions() > > > > + unsigned long flags; > > /* The patching is not fully atomic, so try to avoid local interruptions > > that might execute the to be patched code. > > Other CPUs are not running. */ > > stop_nmi(); > > #ifdef CONFIG_X86_MCE > > stop_mce(); > > #endif > > + local_irq_save(flags); > > > > > > ... > > + local_irq_restore(flags); > > restart_nmi(); > > #ifdef CONFIG_X86_MCE > > restart_mce(); > > #endif > > > > ? > > Hej! This last changes (in addition to the others you mentioned) seems > to be a good shot. I could reboot 8 times the guest, compile several > packages (something which always leeds to the oops) and currently i > build two big packages simultan. So this is heavy IO. > > I will try tomorrow more heavy build tests (to gain the good feeling to > the vbox+guest kernel again like it was with 2.6.25), but i think your > changes goes in the right direction. >
OK, so we have a problem with interrupts coming while we are doing the alternatives patching.
First thing, I wonder if Virtualbox expects the OS to patch all its paravirt instructions in one go ?
Also, could you then try to : - to revert all those changes - Do this to text_poke_early and text_poke :
- put the sync_core() within the irq off critical section (test) - add a wbinvd(); just after the sync_core() in both functions (test).
Thanks,
Mathieu
> Here is the diff what i've changed on your hints: > > ,----[ arch/x86/kernel/alternative.c ] > | --- alternative.c.org 2008-07-13 23:51:29.000000000 +0200 > | +++ alternative.c 2008-08-26 21:35:20.000000000 +0200 > | @@ -343,6 +343,7 @@ > | void alternatives_smp_switch(int smp) > | { > | struct smp_alt_module *mod; > | + unsigned long flags; > | > | #ifdef CONFIG_LOCKDEP > | /* > | @@ -359,7 +360,7 @@ > | return; > | BUG_ON(!smp && (num_online_cpus() > 1)); > | > | - spin_lock(&smp_alt); > | + spin_lock_irqsave(&smp_alt, flags); > | > | /* > | * Avoid unnecessary switches because it forces JIT based VMs to > | @@ -383,7 +384,7 @@ > | mod->text, mod->text_end); > | } > | smp_mode = smp; > | - spin_unlock(&smp_alt); > | + spin_unlock_irqrestore(&smp_alt, flags); > | } > | > | #endif > | @@ -420,6 +421,7 @@ > | > | void __init alternative_instructions(void) > | { > | + unsigned long flags; > | /* The patching is not fully atomic, so try to avoid local interruptions > | that might execute the to be patched code. > | Other CPUs are not running. */ > | @@ -427,6 +429,7 @@ > | #ifdef CONFIG_X86_MCE > | stop_mce(); > | #endif > | + local_irq_save(flags); > | > | apply_alternatives(__alt_instructions, __alt_instructions_end); > | > | @@ -465,6 +468,7 @@ > | (unsigned long)__smp_locks, > | (unsigned long)__smp_locks_end); > | > | + local_irq_restore(flags); > | restart_nmi(); > | #ifdef CONFIG_X86_MCE > | restart_mce(); > | @@ -508,33 +512,5 @@ > | */ > | void *__kprobes text_poke(void *addr, const void *opcode, size_t len) > | { > | - unsigned long flags; > | - char *vaddr; > | - int nr_pages = 2; > | - struct page *pages[2]; > | - int i; > | - > | - if (!core_kernel_text((unsigned long)addr)) { > | - pages[0] = vmalloc_to_page(addr); > | - pages[1] = vmalloc_to_page(addr + PAGE_SIZE); > | - } else { > | - pages[0] = virt_to_page(addr); > | - WARN_ON(!PageReserved(pages[0])); > | - pages[1] = virt_to_page(addr + PAGE_SIZE); > | - } > | - BUG_ON(!pages[0]); > | - if (!pages[1]) > | - nr_pages = 1; > | - vaddr = vmap(pages, nr_pages, VM_MAP, PAGE_KERNEL); > | - BUG_ON(!vaddr); > | - local_irq_save(flags); > | - memcpy(&vaddr[(unsigned long)addr & ~PAGE_MASK], opcode, len); > | - local_irq_restore(flags); > | - vunmap(vaddr); > | - sync_core(); > | - /* Could also do a CLFLUSH here to speed up CPU recovery; but > | - that causes hangs on some VIA CPUs. */ > | - for (i = 0; i < len; i++) > | - BUG_ON(((char *)addr)[i] != ((char *)opcode)[i]); > | - return addr; > | + return text_poke_early(addr, opcode, len); > | } > `---- > > So if Luiz and others could also try all 3 mentioned changes, maybe we > have a solution. I also will build tomorrow a new LiveCD/Install ISO > with these patches to see if the error there is also gone. > > > Thanks, > > > > Mathieu > > Gerhard > > > -- > Was wir wissen, ist ein Tropfen. > Was wir nicht wissen, ein Ozean (Newton)
-- Mathieu Desnoyers OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
| |