Re: [PATCH] watchdog: perform all-CPU backtrace in case of hard lockup

From: Aaron Tomlin
Date: Thu Oct 01 2015 - 03:37:14 EST


On Fri 2015-09-25 13:15 +0200, Jiri Kosina wrote:
> From: Jiri Kosina <jkosina@xxxxxxx>
>
> In many cases of hardlockup reports, it's actually not possible to know
> why it triggered, because the CPU that got stuck is usually waiting on a
> resource (with IRQs disabled) in posession of some other CPU is holding.
>
> IOW, we are often looking at the stacktrace of the victim and not the
> actual offender.
>
> Introduce sysctl / cmdline parameter that makes it possible to have
> hardlockup detector perform all-CPU backtrace.
>
> Signed-off-by: Jiri Kosina <jkosina@xxxxxxx>
> ---
> Documentation/kernel-parameters.txt | 5 +++++
> Documentation/sysctl/kernel.txt | 12 ++++++++++++
> include/linux/nmi.h | 1 +
> kernel/sysctl.c | 9 +++++++++
> kernel/watchdog.c | 33 ++++++++++++++++++++++++++++-----
> 5 files changed, 55 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 22a4b68..b4af96e 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -1246,6 +1246,11 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
> Format: <unsigned int> such that (rxsize & ~0x1fffc0) == 0.
> Default: 1024
>
> + hardlockup_all_cpu_backtrace=
> + [KNL] Should the hard-lockup detector generate
> + backtraces on all cpus.
> + Format: <integer>
> +
> hashdist= [KNL,NUMA] Large hashes allocated during boot
> are distributed across NUMA nodes. Defaults on
> for 64-bit NUMA, off otherwise.
> diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
> index 6fccb69..af70d15 100644
> --- a/Documentation/sysctl/kernel.txt
> +++ b/Documentation/sysctl/kernel.txt
> @@ -33,6 +33,7 @@ show up in /proc/sys/kernel:
> - domainname
> - hostname
> - hotplug
> +- hardlockup_all_cpu_backtrace
> - hung_task_panic
> - hung_task_check_count
> - hung_task_timeout_secs
> @@ -293,6 +294,17 @@ domain names are in general different. For a detailed discussion
> see the hostname(1) man page.
>
> ==============================================================
> +hardlockup_all_cpu_backtrace:
> +
> +This value controls the hard lockup detector behavior when a hard
> +lockup condition is detected as to whether or not to gather further
> +debug information. If enabled, arch-specific all-CPU stack dumping
> +will be initiated.
> +
> +0: do nothing. This is the default behavior.
> +
> +1: on detection capture more debug information.
> +==============================================================
>
> hotplug:
>
> diff --git a/include/linux/nmi.h b/include/linux/nmi.h
> index 78488e0..7ec5b86 100644
> --- a/include/linux/nmi.h
> +++ b/include/linux/nmi.h
> @@ -73,6 +73,7 @@ extern int watchdog_user_enabled;
> extern int watchdog_thresh;
> extern unsigned long *watchdog_cpumask_bits;
> extern int sysctl_softlockup_all_cpu_backtrace;
> +extern int sysctl_hardlockup_all_cpu_backtrace;
> struct ctl_table;
> extern int proc_watchdog(struct ctl_table *, int ,
> void __user *, size_t *, loff_t *);
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index e69201d..efb0370 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -897,6 +897,15 @@ static struct ctl_table kern_table[] = {
> .extra1 = &zero,
> .extra2 = &one,
> },
> + {
> + .procname = "hardlockup_all_cpu_backtrace",
> + .data = &sysctl_hardlockup_all_cpu_backtrace,
> + .maxlen = sizeof(int),
> + .mode = 0644,
> + .proc_handler = proc_dointvec_minmax,
> + .extra1 = &zero,
> + .extra2 = &one,
> + },
> #endif /* CONFIG_SMP */
> #endif
> #if defined(CONFIG_X86_LOCAL_APIC) && defined(CONFIG_X86)
> diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> index 64ed1c3..202999c 100644
> --- a/kernel/watchdog.c
> +++ b/kernel/watchdog.c
> @@ -57,8 +57,10 @@ int __read_mostly watchdog_thresh = 10;
>
> #ifdef CONFIG_SMP
> int __read_mostly sysctl_softlockup_all_cpu_backtrace;
> +int __read_mostly sysctl_hardlockup_all_cpu_backtrace;
> #else
> #define sysctl_softlockup_all_cpu_backtrace 0
> +#define sysctl_hardlockup_all_cpu_backtrace 0
> #endif
> static struct cpumask watchdog_cpumask __read_mostly;
> unsigned long *watchdog_cpumask_bits = cpumask_bits(&watchdog_cpumask);
> @@ -112,6 +114,7 @@ static unsigned long soft_lockup_nmi_warn;
> #ifdef CONFIG_HARDLOCKUP_DETECTOR
> static int hardlockup_panic =
> CONFIG_BOOTPARAM_HARDLOCKUP_PANIC_VALUE;
> +static unsigned long hardlockup_allcpu_dumped;
> /*
> * We may not want to enable hard lockup detection by default in all cases,
> * for example when running the kernel as a guest on a hypervisor. In these
> @@ -173,6 +176,13 @@ static int __init softlockup_all_cpu_backtrace_setup(char *str)
> return 1;
> }
> __setup("softlockup_all_cpu_backtrace=", softlockup_all_cpu_backtrace_setup);
> +static int __init hardlockup_all_cpu_backtrace_setup(char *str)
> +{
> + sysctl_hardlockup_all_cpu_backtrace =
> + !!simple_strtol(str, NULL, 0);
> + return 1;
> +}
> +__setup("hardlockup_all_cpu_backtrace=", hardlockup_all_cpu_backtrace_setup);
> #endif
>
> /*
> @@ -318,17 +328,30 @@ static void watchdog_overflow_callback(struct perf_event *event,
> */
> if (is_hardlockup()) {
> int this_cpu = smp_processor_id();
> + struct pt_regs *regs = get_irq_regs();
>
> /* only print hardlockups once */
> if (__this_cpu_read(hard_watchdog_warn) == true)
> return;
>
> - if (hardlockup_panic)
> - panic("Watchdog detected hard LOCKUP on cpu %d",
> - this_cpu);
> + pr_emerg("Watchdog detected hard LOCKUP on cpu %d", this_cpu);
> + print_modules();
> + print_irqtrace_events(current);
> + if (regs)
> + show_regs(regs);
> else
> - WARN(1, "Watchdog detected hard LOCKUP on cpu %d",
> - this_cpu);
> + dump_stack();
> +
> + /*
> + * Perform all-CPU dump only once to avoid multiple hardlockups
> + * generating interleaving traces
> + */
> + if (sysctl_hardlockup_all_cpu_backtrace &&
> + !test_and_set_bit(0, &hardlockup_allcpu_dumped))
> + trigger_allbutself_cpu_backtrace();

How does this play when 'softlockup_all_cpu_backtrace' is enabled too?

> +
> + if (hardlockup_panic)
> + panic("Hard LOCKUP");
>
> __this_cpu_write(hard_watchdog_warn, true);
> return;

This does indeed appear similar to Linus commit ed235875
("kernel/watchdog.c: print traces for all cpus on lockup detection");
albeit for the hardlockup detector.

Looks fine to me. Thanks!

Reviewed-by: Aaron Tomlin <atomlin@xxxxxxxxxx>

Attachment: signature.asc
Description: PGP signature