On Wed, Jun 21, 2017 at 11:53:57PM +0200, Thomas Gleixner wrote:
> On Wed, 21 Jun 2017, kan.li...@intel.com wrote:
> > We now have more and more systems where the Turbo range is wide enough
> > that the NMI watchdog expires faster than the soft watchdog timer that
> > updates the interrupt tick the NMI watchdog relies on.
> >
> > This problem was originally added by commit 58687acba592
> > ("lockup_detector: Combine nmi_watchdog and softlockup detector").
> > Previously the NMI watchdog would always check jiffies, which were
> > ticking fast enough. But now the backing is quite slow so the expire
> > time becomes more sensitive.
>
> And slapping a factor 3 on the NMI period is the wrong answer to the
> problem. The simple solution would be to increase the hrtimer frequency,
> but that's not really desired either.
>
> Find an untested patch below, which should cure the issue.
A simple low pass filter. It compiles. :-) I don't think I have knowledge
to test it. Kan?
Cheers,
Don
>
> Thanks,
>
> tglx
>
> 8<---------------
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -96,6 +96,7 @@ config X86
> select GENERIC_STRNCPY_FROM_USER
> select GENERIC_STRNLEN_USER
> select GENERIC_TIME_VSYSCALL
> + select HARDLOCKUP_CHECK_TIMESTAMP if X86_64
> select HAVE_ACPI_APEI if ACPI
> select HAVE_ACPI_APEI_NMI if ACPI
> select HAVE_ALIGNED_STRUCT_PAGE if SLUB
> --- a/include/linux/nmi.h
> +++ b/include/linux/nmi.h
> @@ -155,6 +155,14 @@ extern int sysctl_hardlockup_all_cpu_bac
> #define sysctl_softlockup_all_cpu_backtrace 0
> #define sysctl_hardlockup_all_cpu_backtrace 0
> #endif
> +
> +#if defined(CONFIG_HARDLOCKUP_CHECK_TIMESTAMP) && \
> + defined(CONFIG_HARDLOCKUP_DETECTOR)
> +void watchdog_update_hrtimer_threshold(u64 period);
> +#else
> +static inline void watchdog_update_hrtimer_threshold(u64 period) { }
> +#endif
> +
> extern bool is_hardlockup(void);
> struct ctl_table;
> extern int proc_watchdog(struct ctl_table *, int ,
> --- a/kernel/watchdog.c
> +++ b/kernel/watchdog.c
> @@ -161,6 +161,7 @@ static void set_sample_period(void)
> * hardlockup detector generates a warning
> */
> sample_period = get_softlockup_thresh() * ((u64)NSEC_PER_SEC / 5);
> + watchdog_update_hrtimer_threshold(sample_period);
> }
>
> /* Commands for resetting the watchdog */
> --- a/kernel/watchdog_hld.c
> +++ b/kernel/watchdog_hld.c
> @@ -70,6 +70,54 @@ void touch_nmi_watchdog(void)
> }
> EXPORT_SYMBOL(touch_nmi_watchdog);
>
> +#ifdef CONFIG_HARDLOCKUP_CHECK_TIMESTAMP
> +static DEFINE_PER_CPU(ktime_t, last_timestamp);
> +static ktime_t watchdog_hrtimer_sample_threshold __read_mostly;
> +
> +void watchdog_update_hrtimer_threshold(u64 period)
> +{
> + /*
> + * The hrtimer runs with a period of (watchdog_threshold * 2) / 5
> + *
> + * So it runs effectively with 2.5 times the rate of the NMI
> + * watchdog. That means the hrtimer should fire 2-3 times before
> + * the NMI watchdog expires. The NMI watchdog on x86 is based on
> + * unhalted CPU cycles, so if Turbo-Mode is enabled the CPU cycles
> + * might run way faster than expected and the NMI fires in a
> + * smaller period than the one deduced from the nominal CPU
> + * frequency. Depending on the Turbo-Mode factor this might be fast
> + * enough to get the NMI period smaller than the hrtimer watchdog
> + * period and trigger false positives.
> + *
> + * The sample threshold is used to check in the NMI handler whether
> + * the minimum time between two NMI samples has elapsed. That
> + * prevents false positives.
> + *
> + * Set this to 4/5 of the actual watchdog threshold period so the
> + * hrtimer is guaranteed to fire at least once within the real
> + * watchdog threshold.
> + */
> + watchdog_hrtimer_sample_threshold = period * 2;
> +}
> +
> +static bool watchdog_check_timestamp(void)
> +{
> + ktime_t delta, now = ktime_get_mono_fast_ns();
> +
> + delta = now - __this_cpu_read(last_timestamp);
> + if (delta < watchdog_hrtimer_sample_threshold)
> + return false;
> + __this_cpu_write(last_timestamp, now);
> + return true;
> +}
> +#else
> +static inline bool watchdog_check_timestamp(void)
> +{
> + return true;
> +}
> +#endif
> +
> +
> static struct perf_event_attr wd_hw_attr = {
> .type = PERF_TYPE_HARDWARE,
> .config = PERF_COUNT_HW_CPU_CYCLES,
> @@ -94,6 +142,9 @@ static void watchdog_overflow_callback(s
> return;
> }
>
> + if (!watchdog_check_timestamp())
> + return;
> +
> /* check for a hardlockup
> * This is done by making sure our timer interrupt
> * is incrementing. The timer interrupt should have
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -806,6 +806,9 @@ config HARDLOCKUP_DETECTOR
> depends on LOCKUP_DETECTOR && !HAVE_NMI_WATCHDOG
> depends on PERF_EVENTS && HAVE_PERF_EVENTS_NMI
>
> +config HARDLOCKUP_CHECK_TIMESTAMP
> + bool
> +
> config BOOTPARAM_HARDLOCKUP_PANIC
> bool "Panic (Reboot) On Hard Lockups"
> depends on HARDLOCKUP_DETECTOR
