linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: LKML <linux-kernel@vger.kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Borislav Petkov <bp@alien8.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Sebastian Siewior <bigeasy@linutronix.de>,
	Nicholas Piggin <npiggin@gmail.com>,
	Don Zickus <dzickus@redhat.com>,
	Chris Metcalf <cmetcalf@mellanox.com>,
	Ulrich Obergfell <uobergfe@redhat.com>
Subject: [patch V2 09/29] lockup_detector/perf: Remove broken self disable on failure
Date: Tue, 12 Sep 2017 21:37:03 +0200	[thread overview]
Message-ID: <20170912194146.806708429@linutronix.de> (raw)
In-Reply-To: 20170912193654.321505854@linutronix.de

[-- Attachment #1: lockup_detectorperf_Remove_broken_self_disable_on_failure.patch --]
[-- Type: text/plain, Size: 5151 bytes --]

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>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Chris Metcalf <cmetcalf@mellanox.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Sebastian Siewior <bigeasy@linutronix.de>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Ulrich Obergfell <uobergfe@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Link: http://lkml.kernel.org/r/20170831073053.680086886@linutronix.de

---
 kernel/watchdog.c     |   15 ---------------
 kernel/watchdog_hld.c |   20 +++++++-------------
 2 files changed, 7 insertions(+), 28 deletions(-)

--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -485,21 +485,6 @@ 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.
-	 *
-	 * The only non-obvious place this bit can be cleared is through
-	 * watchdog_nmi_enable(), so a pr_info() is placed there.  Placing a
-	 * pr_info here would be too noisy as it would result in a message
-	 * every few seconds if the hardlockup was disabled but the softlockup
-	 * enabled.
-	 */
-	if (!(watchdog_enabled & NMI_WATCHDOG_ENABLED))
-		watchdog_nmi_disable(cpu);
 }
 
 static struct smp_hotplug_thread watchdog_threads = {
--- a/kernel/watchdog_hld.c
+++ b/kernel/watchdog_hld.c
@@ -23,6 +23,7 @@ static DEFINE_PER_CPU(bool, watchdog_nmi
 static DEFINE_PER_CPU(struct perf_event *, watchdog_ev);
 
 static unsigned long hardlockup_allcpu_dumped;
+static bool hardlockup_detector_disabled;
 
 void arch_touch_nmi_watchdog(void)
 {
@@ -178,6 +179,10 @@ int watchdog_nmi_enable(unsigned int cpu
 	if (!(watchdog_enabled & NMI_WATCHDOG_ENABLED))
 		goto out;
 
+	/* A failure disabled the hardlockup detector permanently */
+	if (hardlockup_detector_disabled)
+		return -ENODEV;
+
 	/* is it already setup and enabled? */
 	if (event && event->state > PERF_EVENT_STATE_OFF)
 		goto out;
@@ -206,18 +211,6 @@ int watchdog_nmi_enable(unsigned int cpu
 		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.
-	 *
-	 * The barriers are for syncing up watchdog_enabled across all the
-	 * cpus, as clear_bit() does not use barriers.
-	 */
-	smp_mb__before_atomic();
-	clear_bit(NMI_WATCHDOG_ENABLED_BIT, &watchdog_enabled);
-	smp_mb__after_atomic();
-
 	/* skip displaying the same error again */
 	if (!firstcpu && (PTR_ERR(event) == firstcpu_err))
 		return PTR_ERR(event);
@@ -232,7 +225,8 @@ int watchdog_nmi_enable(unsigned int cpu
 		pr_err("disabled (cpu%i): unable to create perf event: %ld\n",
 			cpu, PTR_ERR(event));
 
-	pr_info("Shutting down hard lockup detector on all cpus\n");
+	pr_info("Disabling hard lockup detector permanently\n");
+	hardlockup_detector_disabled = true;
 
 	return PTR_ERR(event);
 

  parent reply	other threads:[~2017-09-12 19:56 UTC|newest]

Thread overview: 77+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-12 19:36 [patch V2 00/29] lockup_detector: Cure hotplug deadlocks and replace duct tape Thomas Gleixner
2017-09-12 19:36 ` [patch V2 01/29] hardlockup_detector: Provide interface to stop/restart perf events Thomas Gleixner
2017-09-14 10:40   ` [tip:core/urgent] watchdog/hardlockup: " tip-bot for Peter Zijlstra
2017-09-12 19:36 ` [patch V2 02/29] perf/x86/intel: Sanitize PMU HT bug workaround Thomas Gleixner
2017-09-14 10:40   ` [tip:core/urgent] perf/x86/intel, watchdog/core: " tip-bot for Peter Zijlstra
2017-09-12 19:36 ` [patch V2 03/29] lockup_detector: Provide interface to stop from poweroff() Thomas Gleixner
2017-09-14 10:40   ` [tip:core/urgent] watchdog/core: " tip-bot for Thomas Gleixner
2017-09-12 19:36 ` [patch V2 04/29] parisc: Use lockup_detector_stop() Thomas Gleixner
2017-09-14  8:59   ` Helge Deller
2017-09-14 13:46     ` Don Zickus
2017-09-14 10:41   ` [tip:core/urgent] parisc, watchdog/core: " tip-bot for Thomas Gleixner
2017-09-12 19:36 ` [patch V2 05/29] lockup_detector: Remove broken suspend/resume interfaces Thomas Gleixner
2017-09-14 10:41   ` [tip:core/urgent] watchdog/core: " tip-bot for Thomas Gleixner
2017-09-12 19:37 ` [patch V2 06/29] lockup_detector: Rework cpu hotplug locking Thomas Gleixner
2017-09-14 10:41   ` [tip:core/urgent] watchdog/core: Rework CPU " tip-bot for Thomas Gleixner
2017-09-12 19:37 ` [patch V2 07/29] lockup_detector: Rename watchdog_proc_mutex Thomas Gleixner
2017-09-14 10:42   ` [tip:core/urgent] watchdog/core: " tip-bot for Thomas Gleixner
2017-09-12 19:37 ` [patch V2 08/29] lockup_detector: Mark hardlockup_detector_disable() __init Thomas Gleixner
2017-09-14 10:42   ` [tip:core/urgent] watchdog/core: " tip-bot for Thomas Gleixner
2017-09-12 19:37 ` Thomas Gleixner [this message]
2017-09-14 10:43   ` [tip:core/urgent] watchdog/hardlockup/perf: Remove broken self disable on failure tip-bot for Thomas Gleixner
2017-09-12 19:37 ` [patch V2 10/29] lockup_detector/perf: Prevent cpu hotplug deadlock Thomas Gleixner
2017-09-14 10:43   ` [tip:core/urgent] watchdog/hardlockup/perf: Prevent CPU " tip-bot for Thomas Gleixner
2017-09-12 19:37 ` [patch V2 11/29] lockup_detector: Remove park_in_progress obfuscation Thomas Gleixner
2017-09-12 19:37 ` [patch V2 12/29] lockup_detector: Cleanup stub functions Thomas Gleixner
2017-09-14 10:44   ` [tip:core/urgent] watchdog/core: Clean up " tip-bot for Thomas Gleixner
2017-09-12 19:37 ` [patch V2 13/29] lockup_detector: Cleanup the ifdef maze Thomas Gleixner
2017-09-14 10:44   ` [tip:core/urgent] watchdog/core: Clean up the #ifdef maze tip-bot for Thomas Gleixner
2017-09-12 19:37 ` [patch V2 14/29] lockup_detector: Split out cpumask write function Thomas Gleixner
2017-09-14 10:45   ` [tip:core/urgent] watchdog/core: " tip-bot for Thomas Gleixner
2017-09-12 19:37 ` [patch V2 15/29] smpboot/threads: Avoid runtime allocation Thomas Gleixner
2017-09-14 10:45   ` [tip:core/urgent] smpboot/threads, watchdog/core: " tip-bot for Thomas Gleixner
2017-09-12 19:37 ` [patch V2 16/29] lockup_detector: Create new thread handling infrastructure Thomas Gleixner
2017-09-14 10:45   ` [tip:core/urgent] watchdog/core: " tip-bot for Thomas Gleixner
2017-09-12 19:37 ` [patch V2 17/29] lockup_detector: Get rid of the thread teardown/setup dance Thomas Gleixner
2017-09-14 10:46   ` [tip:core/urgent] watchdog/core: " tip-bot for Thomas Gleixner
2017-09-12 19:37 ` [patch V2 18/29] lockup_detector: Further simplify sysctl handling Thomas Gleixner
2017-09-14 10:46   ` [tip:core/urgent] watchdog/core: " tip-bot for Thomas Gleixner
2017-09-12 19:37 ` [patch V2 19/29] lockup_detector: Cleanup header mess Thomas Gleixner
2017-09-14 10:47   ` [tip:core/urgent] watchdog/core: Clean up " tip-bot for Thomas Gleixner
2017-09-12 19:37 ` [patch V2 20/29] lockup_detector/sysctl: Get rid of the ifdeffery Thomas Gleixner
2017-09-14 10:47   ` [tip:core/urgent] watchdog/sysctl: Get rid of the #ifdeffery tip-bot for Thomas Gleixner
2017-09-12 19:37 ` [patch V2 21/29] lockup_detector: Cleanup sysctl variable name space Thomas Gleixner
2017-09-14 10:47   ` [tip:core/urgent] watchdog/sysctl: Clean up " tip-bot for Thomas Gleixner
2017-09-12 19:37 ` [patch V2 22/29] lockup_detector: Make watchdog_nmi_reconfigure() two stage Thomas Gleixner
2017-09-14 10:48   ` [tip:core/urgent] watchdog/core, powerpc: " tip-bot for Thomas Gleixner
2017-10-03  0:29   ` [patch V2 22/29] lockup_detector: " Michael Ellerman
2017-10-03  6:50     ` Thomas Gleixner
2017-10-03  7:04       ` Thomas Gleixner
2017-10-03 10:01         ` Nicholas Piggin
2017-10-03 10:56           ` Thomas Gleixner
2017-10-03 11:36       ` Michael Ellerman
2017-10-03 12:13         ` Thomas Gleixner
2017-10-03 13:20           ` Thomas Gleixner
2017-10-03 19:27             ` Thomas Gleixner
2017-10-04  5:53               ` Michael Ellerman
2017-10-05 16:17               ` Don Zickus
2017-09-12 19:37 ` [patch V2 23/29] lockup_detector: Get rid of the racy update loop Thomas Gleixner
2017-09-14 10:48   ` [tip:core/urgent] watchdog/core: " tip-bot for Thomas Gleixner
2017-09-12 19:37 ` [patch V2 24/29] lockup_detector/perf: Implement init time perf validation Thomas Gleixner
2017-09-14 10:48   ` [tip:core/urgent] watchdog/hardlockup/perf: " tip-bot for Thomas Gleixner
2017-09-12 19:37 ` [patch V2 25/29] lockup_detector: Implement init time detection of perf Thomas Gleixner
2017-09-13 18:02   ` Don Zickus
2017-09-13 18:05     ` Thomas Gleixner
2017-09-14  5:27       ` Ingo Molnar
2017-09-14 10:49   ` [tip:core/urgent] watchdog/hardlockup/perf: " tip-bot for Thomas Gleixner
2017-09-12 19:37 ` [patch V2 26/29] lockup_detector/perf: Implement CPU enable replacement Thomas Gleixner
2017-09-14 10:49   ` [tip:core/urgent] watchdog/hardlockup/perf: " tip-bot for Thomas Gleixner
2017-09-12 19:37 ` [patch V2 27/29] lockup_detector: Use new perf CPU enable mechanism Thomas Gleixner
2017-09-14 10:50   ` [tip:core/urgent] watchdog/hardlockup/perf: " tip-bot for Thomas Gleixner
2017-09-12 19:37 ` [patch V2 28/29] lockup_detector/perf: Simplify deferred event destroy Thomas Gleixner
2017-09-14 10:50   ` [tip:core/urgent] watchdog/hardlockup/perf: " tip-bot for Thomas Gleixner
2017-09-12 19:37 ` [patch V2 29/29] lockup_detector: Cleanup hotplug locking mess Thomas Gleixner
2017-09-14 10:50   ` [tip:core/urgent] watchdog/hardlockup: Clean up " tip-bot for Thomas Gleixner
2017-09-13 18:06 ` [patch V2 00/29] lockup_detector: Cure hotplug deadlocks and replace duct tape Don Zickus
2017-09-14  5:27   ` Ingo Molnar
2017-09-14  8:11   ` Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170912194146.806708429@linutronix.de \
    --to=tglx@linutronix.de \
    --cc=akpm@linux-foundation.org \
    --cc=bigeasy@linutronix.de \
    --cc=bp@alien8.de \
    --cc=cmetcalf@mellanox.com \
    --cc=dzickus@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=npiggin@gmail.com \
    --cc=peterz@infradead.org \
    --cc=uobergfe@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).