Messages in this thread | | | Date | Tue, 13 May 2008 11:03:21 -0300 | From | "Glauber Costa" <> | Subject | Re: [PATCH] x86: fix app crashes after SMP resume |
| |
On Tue, May 13, 2008 at 10:26 AM, Hugh Dickins <hugh@veritas.com> wrote: > After resume on a 2cpu laptop, kernel builds collapse with a sed hang, > sh or make segfault (often on 20295564), real-time signal to cc1 etc. > > Several hurdles to jump, but a manually-assisted bisect led to -rc1's > d2bcbad5f3ad38a1c09861bca7e252dde7bb8259 x86: do not zap_low_mappings > in __smp_prepare_cpus. Though the low mappings were removed at bootup, > they were left behind (with Global flags helping to keep them in TLB) > after resume or cpu online, causing the crashes seen. > > Reinstate zap_low_mappings (with local __flush_tlb_all) for each cpu_up > on x86_32. This used to be serialized by smp_commenced_mask: that's now > gone, but a low_mappings flag will do. No need for native_smp_cpus_done > to repeat the zap: let mem_init zap BSP's low mappings just like on UP.
Great!
Thanks for finding and fixing this, hugh.
> --- 2.6.26-rc2/arch/x86/kernel/smpboot.c 2008-05-12 02:01:05.000000000 +0100 > +++ linux/arch/x86/kernel/smpboot.c 2008-05-12 14:07:06.000000000 +0100 > @@ -86,6 +86,7 @@ void *x86_bios_cpu_apicid_early_ptr; > > #ifdef CONFIG_X86_32 > u8 apicid_2_node[MAX_APICID]; > +static int low_mappings; > #endif > > /* State of each CPU */ > @@ -326,6 +327,12 @@ static void __cpuinit start_secondary(vo > enable_8259A_irq(0); > } > > +#ifdef CONFIG_X86_32 > + while (low_mappings) > + cpu_relax(); > + __flush_tlb_all(); > +#endif > +
If possible, we don't want to introduce any more ifdefs. Should be better to do it openly, define low_mappings as always 0 for x86_64. We can use an unlikely() test to make it faster too.
> /* This must be done before setting cpu_online_map */ > set_cpu_sibling_map(raw_smp_processor_id()); > wmb(); > @@ -1040,14 +1047,20 @@ int __cpuinit native_cpu_up(unsigned int > #ifdef CONFIG_X86_32 > /* init low mem mapping */ > clone_pgd_range(swapper_pg_dir, swapper_pg_dir + KERNEL_PGD_BOUNDARY, > - min_t(unsigned long, KERNEL_PGD_PTRS, KERNEL_PGD_BOUNDARY)); > + min_t(unsigned long, KERNEL_PGD_PTRS, KERNEL_PGD_BOUNDARY)); > flush_tlb_all(); > -#endif > + low_mappings = 1; > > err = do_boot_cpu(apicid, cpu); > - if (err < 0) { > + > + zap_low_mappings(); > + low_mappings = 0; > +#else > + err = do_boot_cpu(apicid, cpu); > +#endif > + if (err) { > Dprintk("do_boot_cpu failed %d\n", err); > - return err; > + return -EIO; > } > > /* > @@ -1259,9 +1272,6 @@ void __init native_smp_cpus_done(unsigne > setup_ioapic_dest(); > #endif > check_nmi_watchdog(); > -#ifdef CONFIG_X86_32 > - zap_low_mappings(); > -#endif > } > > #ifdef CONFIG_HOTPLUG_CPU > --- 2.6.26-rc2/arch/x86/mm/init_32.c 2008-05-03 21:54:41.000000000 +0100 > +++ linux/arch/x86/mm/init_32.c 2008-05-12 14:07:06.000000000 +0100 > @@ -438,8 +438,6 @@ void zap_low_mappings(void) > { > int i; > > - save_pg_dir(); > - > /* > * Zap initial low-memory mappings. > * > @@ -663,16 +661,8 @@ void __init mem_init(void) > test_wp_bit(); > > cpa_init(); > - > - /* > - * Subtle. SMP is doing it's boot stuff late (because it has to > - * fork idle threads) - but it also needs low mappings for the > - * protected-mode entry to work. We zap these entries only after > - * the WP-bit has been tested. > - */ > -#ifndef CONFIG_SMP > + save_pg_dir(); > zap_low_mappings(); > -#endif > } > > #ifdef CONFIG_MEMORY_HOTPLUG >
-- Glauber Costa. "Free as in Freedom" http://glommer.net
"The less confident you are, the more serious you have to act."
| |