lkml.org 
[lkml]   [2008]   [May]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH] x86: fix app crashes after SMP resume

    * 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.
    >
    > (In passing, fix error code from native_cpu_up: do_boot_cpu returns a
    > variety of diagnostic values, Dprintk what it says but convert to
    > -EIO. And save_pg_dir separately before zap_low_mappings: doesn't
    > matter now, but zapping twice in succession wiped out resume's
    > swsusp_pg_dir.)
    >
    > That worked well on the duo and one quad, but wouldn't boot 3rd or 4th
    > cpu on P4 Xeon, oopsing just after unlock_ipi_call_lock. The TLB
    > flush IPI now being sent reveals a long-standing bug: the booting cpu
    > has its APIC readied in smp_callin at the top of start_secondary, but
    > isn't put into the cpu_online_map until just before that
    > unlock_ipi_call_lock.
    >
    > So native_smp_call_function_mask to online cpus would
    > send_IPI_allbutself, including the cpu just coming up, though it has
    > been excluded from the count to wait for: by the time it handles the
    > IPI, the call data on native_smp_call_function_mask's stack may well
    > have been overwritten.
    >
    > So fall back to send_IPI_mask while cpu_online_map does not match
    > cpu_callout_map: perhaps there's a better APICological fix to be made
    > at the start_secondary end, but I wouldn't know that.

    applied, thanks Hugh! You've once again proven that you are worth your
    weight in gold and more. Patch looks very clean and simplifies the code.

    > I've often wondered: should git reject any commit with "bad" in the
    > id?

    hehe :)

    ob'note: would be nice to have the suspend+resume self-test/debug patch
    below upstream. It programs the RTC to a 5 second sleep and resumes the
    computer, which then self-wakeups afterwards.

    Ingo

    -------------------------------->
    Subject: suspend+resume self-test
    From: David Brownell <david-b@pacbell.net>

    Signed-off-by: Ingo Molnar <mingo@elte.hu>
    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
    ---
    kernel/power/Kconfig | 10 +++
    kernel/power/main.c | 163 +++++++++++++++++++++++++++++++++++++++++++++++++++
    2 files changed, 173 insertions(+)

    Index: linux/kernel/power/Kconfig
    ===================================================================
    --- linux.orig/kernel/power/Kconfig
    +++ linux/kernel/power/Kconfig
    @@ -94,6 +94,16 @@ config SUSPEND
    powered and thus its contents are preserved, such as the
    suspend-to-RAM state (e.g. the ACPI S3 state).

    +config PM_TEST_SUSPEND
    + bool "Test suspend/resume and wakealarm during bootup"
    + depends on SUSPEND && PM_DEBUG && RTC_LIB=y
    + ---help---
    + This option will suspend your machine during bootup, and make
    + it wake up a few seconds later using the RTC's wakeup alarm.
    +
    + You probably want to have your system's RTC driver statically
    + linked, ensuring that it's available when this test runs.
    +
    config SUSPEND_FREEZER
    bool "Enable freezer for suspend to RAM/standby" \
    if ARCH_WANTS_FREEZER_CONTROL || BROKEN
    Index: linux/kernel/power/main.c
    ===================================================================
    --- linux.orig/kernel/power/main.c
    +++ linux/kernel/power/main.c
    @@ -132,6 +132,52 @@ static inline int suspend_test(int level

    #ifdef CONFIG_SUSPEND

    +#ifdef CONFIG_PM_TEST_SUSPEND
    +
    +/*
    + * We test the system suspend code by setting an RTC wakealarm a short
    + * time in the future, then suspending. Suspending the devices won't
    + * normally take long ... some systems only need a few milliseconds.
    + *
    + * The time it takes is system-specific though, so when we test this
    + * during system bootup we allow a LOT of time.
    + */
    +#define TEST_SUSPEND_SECONDS 5
    +
    +static unsigned long suspend_test_start_time;
    +
    +static void suspend_test_start(void)
    +{
    + /* FIXME Use better timebase than "jiffies", ideally a clocksource.
    + * What we want is a hardware counter that will work correctly even
    + * during the irqs-are-off stages of the suspend/resume cycle...
    + */
    + suspend_test_start_time = jiffies;
    +}
    +
    +static void suspend_test_finish(const char *label)
    +{
    + long nj = jiffies - suspend_test_start_time;
    + unsigned msec;
    +
    + msec = jiffies_to_msecs((nj >= 0) ? nj : -nj);
    + pr_info("PM: %s took %d.%03d seconds\n", label,
    + msec / 1000, msec % 1000);
    + WARN_ON_ONCE(msec > ((TEST_SUSPEND_SECONDS+5) * 1000));
    +}
    +
    +#else
    +
    +static void suspend_test_start(void)
    +{
    +}
    +
    +static void suspend_test_finish(const char *label)
    +{
    +}
    +
    +#endif
    +
    /* This is just an arbitrary number */
    #define FREE_PAGE_NUMBER (100)

    @@ -264,11 +310,14 @@ int suspend_devices_and_enter(suspend_st
    goto Close;
    }
    suspend_console();
    +
    + suspend_test_start();
    error = device_suspend(PMSG_SUSPEND);
    if (error) {
    printk(KERN_ERR "PM: Some devices failed to suspend\n");
    goto Resume_console;
    }
    + suspend_test_finish("suspend devices");

    if (suspend_test(TEST_DEVICES))
    goto Resume_devices;
    @@ -291,7 +340,9 @@ int suspend_devices_and_enter(suspend_st
    if (suspend_ops->finish)
    suspend_ops->finish();
    Resume_devices:
    + suspend_test_start();
    device_resume();
    + suspend_test_finish("resume devices");
    Resume_console:
    resume_console();
    Close:
    @@ -515,3 +566,115 @@ static int __init pm_init(void)
    }

    core_initcall(pm_init);
    +
    +
    +#ifdef CONFIG_PM_TEST_SUSPEND
    +
    +#include <linux/rtc.h>
    +
    +/*
    + * To test system suspend, we need a hands-off mechanism to resume the
    + * system. RTCs with wakeup alarms are the the most common mechanism
    + * that's self-contained.
    + */
    +
    +static void __init test_wakealarm(struct rtc_device *rtc, suspend_state_t state)
    +{
    + static char err_readtime [] __initdata =
    + KERN_ERR "PM: can't read %s time, err %d\n";
    + static char err_wakealarm [] __initdata =
    + KERN_ERR "PM: can't set %s wakealarm, err %d\n";
    + static char err_suspend [] __initdata =
    + KERN_ERR "PM: suspend test failed, error %d\n";
    + static char info_test [] __initdata =
    + KERN_INFO "PM: test RTC wakeup from '%s' suspend\n";
    +
    + unsigned long now;
    + struct rtc_wkalrm alm;
    + int status;
    +
    + /* this may fail if the RTC hasn't been initialized */
    + status = rtc_read_time(rtc, &alm.time);
    + if (status < 0) {
    + printk(err_readtime, rtc->dev.bus_id, status);
    + return;
    + }
    + rtc_tm_to_time(&alm.time, &now);
    +
    + memset(&alm, 0, sizeof alm);
    + rtc_time_to_tm(now + TEST_SUSPEND_SECONDS, &alm.time);
    + alm.enabled = true;
    +
    + status = rtc_set_alarm(rtc, &alm);
    + if (status < 0) {
    + printk(err_wakealarm, rtc->dev.bus_id, status);
    + return;
    + }
    +
    + if (state == PM_SUSPEND_MEM) {
    + printk(info_test, pm_states[state]);
    + status = pm_suspend(state);
    + if (status == -ENODEV)
    + state = PM_SUSPEND_STANDBY;
    + }
    + if (state == PM_SUSPEND_STANDBY) {
    + printk(info_test, pm_states[state]);
    + status = pm_suspend(state);
    + }
    + if (status < 0)
    + printk(err_suspend, status);
    +}
    +
    +static int __init has_wakealarm(struct device *dev, void *name_ptr)
    +{
    + struct rtc_device *candidate = to_rtc_device(dev);
    +
    + if (!candidate->ops->set_alarm)
    + return 0;
    + if (!device_may_wakeup(candidate->dev.parent))
    + return 0;
    +
    + *(char **)name_ptr = dev->bus_id;
    + return 1;
    +}
    +
    +/*
    + * We normally test Suspend-to-RAM, with standby as a backup when
    + * the system doesn't support that state. But we also need to be
    + * able to disable the powerup test, and tell it to ignore STR since
    + * the RTC may not work then.
    + */
    +static suspend_state_t test_state __initdata = PM_SUSPEND_MEM;
    +
    +static int __init setup_test_suspend(char *value)
    +{
    + /* FIXME accept "standby", etc */
    + test_state = PM_SUSPEND_ON;
    + return 0;
    +}
    +__setup("test_suspend", setup_test_suspend);
    +
    +static int __init test_suspend(void)
    +{
    + static char warn_no_rtc[] __initdata =
    + KERN_WARNING "PM: no wakealarm-capable RTC driver is ready\n";
    +
    + char *pony = NULL;
    + struct rtc_device *rtc = NULL;
    +
    + class_find_device(rtc_class, &pony, has_wakealarm);
    + if (pony)
    + rtc = rtc_class_open(pony);
    +
    + if (rtc) {
    + if (test_state != PM_SUSPEND_ON)
    + test_wakealarm(rtc, test_state);
    + rtc_class_close(rtc);
    + } else
    + printk(warn_no_rtc);
    +
    + return 0;
    +}
    +late_initcall(test_suspend);
    +
    +#endif /* CONFIG_PM_TEST_SUSPEND */

    \
     
     \ /
      Last update: 2008-05-13 16:07    [W:0.038 / U:0.948 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site