watchdog/hardlockup/perf: Remove broken self disable on failure
authorThomas Gleixner <tglx@linutronix.de>
Tue, 12 Sep 2017 19:37:03 +0000 (21:37 +0200)
committerIngo Molnar <mingo@kernel.org>
Thu, 14 Sep 2017 09:41:04 +0000 (11:41 +0200)
commit20d853fd0703b1d73c35a22024c0d4fcbcc57c8c
treea837e131080d503c230d0bd147f9e3b2696379dd
parent7a3558200739e1378800a7a6d7f63c031115f7a4
watchdog/hardlockup/perf: Remove broken self disable on failure

The self disabling feature is broken vs. CPU hotplug locking:

CPU 0    CPU 1
cpus_write_lock();
 cpu_up(1)
   wait_for_completion()
   ....
   unpark_watchdog()
   ->unpark()
     perf_event_create() <- fails
       watchdog_enable &= ~NMI_WATCHDOG;
   ....
cpus_write_unlock();
   CPU 2
cpus_write_lock()
 cpu_down(2)
   wait_for_completion()
   wakeup(watchdog);
     watchdog()
     if (!(watchdog_enable & NMI_WATCHDOG))
watchdog_nmi_disable()
  perf_event_disable()
  ....
  cpus_read_lock();

   stop_smpboot_threads()
     park_watchdog();
       wait_for_completion(watchdog->parked);

Result: End of hotplug and instantaneous full lockup of the machine.

There is a similar problem with disabling the watchdog via the user space
interface as the sysctl function fiddles with watchdog_enable directly.

It's very debatable whether this is required at all. If the watchdog works
nicely on N CPUs and it fails to enable on the N + 1 CPU either during
hotplug or because the user space interface disabled it via sysctl cpumask
and then some perf user grabbed the counter which is then unavailable for
the watchdog when the sysctl cpumask gets changed back.

There is no real justification for this.

One of the reasons WHY this is done is the utter stupidity of the init code
of the perf NMI watchdog. Instead of checking upfront at boot whether PERF
is available and functional at all, it just does this check at run time
over and over when user space fiddles with the sysctl. That's broken beyond
repair along with the idiotic error code dependent warn level printks and
the even more silly printk rate limiting.

If the init code checks whether perf works at boot time, then this mess can
be more or less avoided completely. Perf does not come magically into life
at runtime. Brain usage while coding is overrated.

Remove the cruft and add a temporary safe guard which gets removed later.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Don Zickus <dzickus@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Chris Metcalf <cmetcalf@mellanox.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Sebastian Siewior <bigeasy@linutronix.de>
Cc: Ulrich Obergfell <uobergfe@redhat.com>
Link: http://lkml.kernel.org/r/20170912194146.806708429@linutronix.de
Signed-off-by: Ingo Molnar <mingo@kernel.org>
kernel/watchdog.c
kernel/watchdog_hld.c