Messages in this thread | | | Date | Mon, 23 Feb 2015 13:17:34 -0800 | From | Andrew Morton <> | Subject | Re: [PATCH 6/9] watchdog: implement error handling for failure to set up hardware perf events |
| |
On Thu, 5 Feb 2015 15:40:22 -0500 Don Zickus <dzickus@redhat.com> wrote:
> From: Ulrich Obergfell <uobergfe@redhat.com> > > If watchdog_nmi_enable() fails to set up the hardware perf event > of one CPU, the entire hard lockup detector is deemed unreliable. > Hence, disable the hard lockup detector and shut down the hardware > perf events on all CPUs. > > Signed-off-by: Ulrich Obergfell <uobergfe@redhat.com> > Signed-off-by: Don Zickus <dzickus@redhat.com> > --- > kernel/watchdog.c | 18 ++++++++++++++++++ > 1 files changed, 18 insertions(+), 0 deletions(-) > > diff --git a/kernel/watchdog.c b/kernel/watchdog.c > index 26002ed..7ad8949 100644 > --- a/kernel/watchdog.c > +++ b/kernel/watchdog.c > @@ -502,6 +502,15 @@ static void watchdog(unsigned int cpu) > __this_cpu_write(soft_lockup_hrtimer_cnt, > __this_cpu_read(hrtimer_interrupts)); > __touch_watchdog(); > + > + /* > + * watchdog_nmi_enable() clears the NMI_WATCHDOG_ENABLED bit in the > + * failure path. Check for failures that can occur asynchronously - > + * for example, when CPUs are on-lined - and shut down the hardware > + * perf event on each CPU accordingly. > + */ > + if (!(watchdog_enabled & NMI_WATCHDOG_ENABLED)) > + watchdog_nmi_disable(cpu);
Silently disabling the hardware watchdog. Wouldn't it be better to emit a printk to alert the operator about this event?
> > #ifdef CONFIG_HARDLOCKUP_DETECTOR > @@ -552,6 +561,15 @@ handle_err: > goto out_save; > } > > + /* > + * Disable the hard lockup detector if _any_ CPU fails to set up > + * set up the hardware perf event. The watchdog() function checks > + * the NMI_WATCHDOG_ENABLED bit periodically. > + */ > + smp_mb__before_atomic(); > + clear_bit(NMI_WATCHDOG_ENABLED_BIT, &watchdog_enabled); > + smp_mb__after_atomic();
Please send along a patch which adds comments explaining what these barriers are for.
What are these barriers for? ;)
> /* skip displaying the same error again */ > if (cpu > 0 && (PTR_ERR(event) == cpu0_err)) > return PTR_ERR(event);
| |