lkml.org 
[lkml]   [2012]   [Dec]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH v2] watchdog: Fix disable/enable regression
    Date
    commit 8d451690 ("watchdog: Fix CPU hotplug regression") cause
    an oops or hard lockup when doing

    echo 0 > /proc/sys/kernel/nmi_watchdog
    echo 1 > /proc/sys/kernel/nmi_watchdog

    and the kernel is booted with nmi_watchdog=1 (default)

    Running laptop-mode-tools and disconnecting/connecting AC power
    will cause this to trigger, making it a common failure scenario
    on laptops.

    Instead of bailing out of watchdog_disable() when !watchdog_enabled
    we can initialize the hrtimer regardless of watchdog_enabled status.
    This makes it safe to call watchdog_disable() in the nmi_watchdog=0
    case, without the negative effect on the enabled => disabled =>
    enabled case.

    All these tests pass with this patch:
    - nmi_watchdog=1
    echo 0 > /proc/sys/kernel/nmi_watchdog
    echo 1 > /proc/sys/kernel/nmi_watchdog

    - nmi_watchdog=0
    echo 0 > /sys/devices/system/cpu/cpu1/online

    - nmi_watchdog=0
    echo mem > /sys/power/state

    Cc: <stable@vger.kernel.org> # v3.7
    Cc: Norbert Warmuth <nwarmuth@t-online.de>
    Cc: Joseph Salisbury <joseph.salisbury@canonical.com>
    Cc: Thomas Gleixner <tglx@linutronix.de>
    Signed-off-by: Bjørn Mork <bjorn@mork.no>
    ---
    v2:
    implemented an alternate workaround for the original problem.

    Hello Thomas,

    I will not claim to understand this code, but it seemed to me like the
    original problem was caused by the missing initialization of the hrtimer
    in the disabled case. Calling hrtimer_cancel() on an initialized timer
    not running should be perfectly OK. And watchdog_nmi_disable() will
    not do anything unless the event is initialized. So this patch looks
    like a fix.


    Bjørn

    kernel/watchdog.c | 11 ++++-------
    1 files changed, 4 insertions(+), 7 deletions(-)

    diff --git a/kernel/watchdog.c b/kernel/watchdog.c
    index c8c21be..762081c 100644
    --- a/kernel/watchdog.c
    +++ b/kernel/watchdog.c
    @@ -343,6 +343,10 @@ static void watchdog_enable(unsigned int cpu)
    {
    struct hrtimer *hrtimer = &__raw_get_cpu_var(watchdog_hrtimer);

    + /* kick off the timer for the hardlockup detector */
    + hrtimer_init(hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
    + hrtimer->function = watchdog_timer_fn;
    +
    if (!watchdog_enabled) {
    kthread_park(current);
    return;
    @@ -351,10 +355,6 @@ static void watchdog_enable(unsigned int cpu)
    /* Enable the perf event */
    watchdog_nmi_enable(cpu);

    - /* kick off the timer for the hardlockup detector */
    - hrtimer_init(hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
    - hrtimer->function = watchdog_timer_fn;
    -
    /* done here because hrtimer_start can only pin to smp_processor_id() */
    hrtimer_start(hrtimer, ns_to_ktime(get_sample_period()),
    HRTIMER_MODE_REL_PINNED);
    @@ -368,9 +368,6 @@ static void watchdog_disable(unsigned int cpu)
    {
    struct hrtimer *hrtimer = &__raw_get_cpu_var(watchdog_hrtimer);

    - if (!watchdog_enabled)
    - return;
    -
    watchdog_set_prio(SCHED_NORMAL, 0);
    hrtimer_cancel(hrtimer);
    /* disable the perf event */
    --
    1.7.2.5
    --
    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: 2012-12-14 15:01    [W:0.027 / U:93.176 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site