2012-05-01 17:23:09

by Sameer Nanda

[permalink] [raw]
Subject: [PATCH v2] watchdog: fix for lockup detector breakage on resume

On the suspend/resume path the boot CPU does not go though an
offline->online transition. This breaks the NMI detector
post-resume since it depends on PMU state that is lost when
the system gets suspended.

Fix this by forcing a CPU offline->online transition for the
lockup detector on the boot CPU during resume.

Signed-off-by: Sameer Nanda <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Srivatsa S. Bhat <[email protected]>
---

To provide more context, we enable NMI watchdog on
Chrome OS. We have seen several reports of systems freezing
up completely which indicated that the NMI watchdog was not
firing for some reason.

Debugging further, we found a simple way of repro'ing system
freezes -- issuing the command 'tasket 1 sh -c "echo nmilockup > /proc/breakme"'
after the system has been suspended/resumed one or more times.

With this patch in place, the system freeze result in panics,
as expected. These panics provide a nice stack trace for us
to debug the actual issue causing the freeze.

include/linux/sched.h | 8 ++++++++
kernel/power/suspend.c | 3 +++
kernel/watchdog.c | 17 +++++++++++++++++
3 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 81a173c..44f6046 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -332,6 +332,14 @@ static inline void lockup_detector_init(void)
}
#endif

+#if defined(CONFIG_LOCKUP_DETECTOR) && defined(CONFIG_SUSPEND)
+void lockup_detector_bootcpu_resume(void);
+#else
+static inline void lockup_detector_bootcpu_resume(void)
+{
+}
+#endif
+
#ifdef CONFIG_DETECT_HUNG_TASK
extern unsigned int sysctl_hung_task_panic;
extern unsigned long sysctl_hung_task_check_count;
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index 396d262..0d262a8 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -177,6 +177,9 @@ static int suspend_enter(suspend_state_t state, bool *wakeup)
arch_suspend_enable_irqs();
BUG_ON(irqs_disabled());

+ /* Kick the lockup detector */
+ lockup_detector_bootcpu_resume();
+
Enable_cpus:
enable_nonboot_cpus();

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index df30ee0..dae2482 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -585,6 +585,23 @@ static struct notifier_block __cpuinitdata cpu_nfb = {
.notifier_call = cpu_callback
};

+#ifdef CONFIG_SUSPEND
+/*
+ * On exit from suspend we force an offline->online transition on the boot CPU
+ * so that the PMU state that was lost while in suspended state gets set up
+ * properly for the boot CPU. This information is required for restarting the
+ * NMI watchdog.
+ */
+void lockup_detector_bootcpu_resume(void)
+{
+ void *cpu = (void *)(long)smp_processor_id();
+
+ cpu_callback(&cpu_nfb, CPU_DEAD_FROZEN, cpu);
+ cpu_callback(&cpu_nfb, CPU_UP_PREPARE_FROZEN, cpu);
+ cpu_callback(&cpu_nfb, CPU_ONLINE_FROZEN, cpu);
+}
+#endif
+
void __init lockup_detector_init(void)
{
void *cpu = (void *)(long)smp_processor_id();
--
1.7.7.3


2012-05-07 03:41:16

by Anshuman Khandual

[permalink] [raw]
Subject: Re: [PATCH v2] watchdog: fix for lockup detector breakage on resume

On Tuesday 01 May 2012 10:52 PM, Sameer Nanda wrote:

> On the suspend/resume path the boot CPU does not go though an
> offline->online transition. This breaks the NMI detector
> post-resume since it depends on PMU state that is lost when
> the system gets suspended.


We should not have allowed the PMU to go with events counting on it across the suspend/resume transition
and find out that the state has been lost. This patch solves the problem of the NMI detector as we restart the
counter again when the boot cpu comes back online during resume. But the original cause (PMU going with
counters into the suspend state) which triggered this problem is still there. May be we should have called
perf_event_exit() on the boot cpu before going into the suspend state.

Regards
Anshuman

2012-06-08 21:44:50

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v2] watchdog: fix for lockup detector breakage on resume

On Mon, 07 May 2012 08:54:57 +0530
Anshuman Khandual <[email protected]> wrote:

> On Tuesday 01 May 2012 10:52 PM, Sameer Nanda wrote:
>
> > On the suspend/resume path the boot CPU does not go though an
> > offline->online transition. This breaks the NMI detector
> > post-resume since it depends on PMU state that is lost when
> > the system gets suspended.
>
>
> We should not have allowed the PMU to go with events counting on it across the suspend/resume transition
> and find out that the state has been lost. This patch solves the problem of the NMI detector as we restart the
> counter again when the boot cpu comes back online during resume. But the original cause (PMU going with
> counters into the suspend state) which triggered this problem is still there. May be we should have called
> perf_event_exit() on the boot cpu before going into the suspend state.
>

That sounds like a nicer solution.

An implementation would be nice ;) I'll keep the original patch on life
support until we get all this nailed down.