mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Don Zickus <dzickus@redhat.com>
To: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Robert Richter <robert.richter@amd.com>,
	ying.huang@intel.com, Andi Kleen <andi@firstfloor.org>,
	gorcunov@gmail.com, LKML <linux-kernel@vger.kernel.org>,
	Don Zickus <dzickus@redhat.com>
Subject: [PATCH 9/9] watchdog:  touch_nmi_watchdog should only touch local cpu not every one
Date: Tue, 30 Nov 2010 17:27:30 -0500	[thread overview]
Message-ID: <1291156050-4482-10-git-send-email-dzickus@redhat.com> (raw)
In-Reply-To: <1291156050-4482-1-git-send-email-dzickus@redhat.com>

I ran into a scenario where while one cpu was stuck and should have panic'd
because of the NMI watchdog, it didn't.  The reason was another cpu was spewing
stack dumps on to the console.  Upon investigation, I noticed that when writing
to the console and also when dumping the stack, the watchdog is touched.

This causes all the cpus to reset their NMI watchdog flags and the 'stuck' cpu
just spins forever.

This change causes the semantics of touch_nmi_watchdog to be changed slightly.
Previously, I accidentally changed the semantics and we noticed there was a
codepath in which touch_nmi_watchdog could be touched from a preemtible area.
That caused a BUG() to happen when CONFIG_DEBUG_PREEMPT was enabled.  I believe
it was the acpi code.

My attempt here re-introduces the change to have the touch_nmi_watchdog() code
only touch the local cpu instead of all of the cpus.  But instead of using
__get_cpu_var(), I use the __raw_get_cpu_var() version.

This avoids the preemption problem.  However my reasoning wasn't because I was
trying to be lazy.  Instead I rationalized it as, well if preemption is enabled
then interrupts should be enabled to and the NMI watchdog will have no reason
to trigger.  So it won't matter if the wrong cpu is touched because the percpu
interrupt counters the NMI watchdog uses should still be incrementing.

Signed-off-by: Don Zickus <dzickus@redhat.com>
---
 kernel/watchdog.c |   17 +++++++++--------
 1 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index e0f44dc..792a4ed 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -143,14 +143,15 @@ void touch_all_softlockup_watchdogs(void)
 #ifdef CONFIG_HARDLOCKUP_DETECTOR
 void touch_nmi_watchdog(void)
 {
-	if (watchdog_enabled) {
-		unsigned cpu;
+	/*
+	 * Using __raw here because some code paths have
+	 * preemption enabled.  If preemption is enabled
+	 * then interrupts should be enabled too, in which
+	 * case we shouldn't have to worry about the watchdog
+	 * going off.
+	 */
+	__raw_get_cpu_var(watchdog_nmi_touch) = true;
 
-		for_each_present_cpu(cpu) {
-			if (per_cpu(watchdog_nmi_touch, cpu) != true)
-				per_cpu(watchdog_nmi_touch, cpu) = true;
-		}
-	}
 	touch_softlockup_watchdog();
 }
 EXPORT_SYMBOL(touch_nmi_watchdog);
-- 
1.7.3.2


  parent reply	other threads:[~2010-11-30 22:29 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-30 22:27 [V3 PATCH 0/9] x86, NMI: give NMI handler a face-lift Don Zickus
2010-11-30 22:27 ` [PATCH 1/9] x86, NMI: Add NMI symbol constants and rename memory parity to PCI SERR Don Zickus
2010-11-30 22:27 ` [PATCH 2/9] x86, NMI: Add touch_nmi_watchdog to io_check_error delay Don Zickus
2010-11-30 22:27 ` [PATCH 3/9] x86, NMI: Rewrite NMI handler Don Zickus
2010-11-30 22:27 ` [PATCH 4/9] x86, NMI: Remove DIE_NMI_IPI and add priorties to handlers Don Zickus
2010-12-01 18:41   ` Cyrill Gorcunov
2010-12-01 18:53     ` Peter Zijlstra
2010-12-01 19:01       ` Cyrill Gorcunov
2010-12-01 21:30     ` Andi Kleen
2010-12-01 21:35       ` Cyrill Gorcunov
2010-11-30 22:27 ` [PATCH 5/9] x86, NMI: Allow NMI reason io port (0x61) to be processed on any CPU Don Zickus
2010-11-30 22:27 ` [PATCH 6/9] x86: only call smp_processor_id in non-preempt cases Don Zickus
2010-12-01 18:07   ` Cyrill Gorcunov
2010-11-30 22:27 ` [PATCH 7/9] x86: Avoid calling arch_trigger_all_cpu_backtrace() at the same time Don Zickus
2010-11-30 22:27 ` [PATCH 8/9] panic: ratelimit panic messages Don Zickus
2010-11-30 22:27 ` Don Zickus [this message]
2010-12-22  3:16 ` [V3 PATCH 0/9] x86, NMI: give NMI handler a face-lift Huang Ying

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=1291156050-4482-10-git-send-email-dzickus@redhat.com \
    --to=dzickus@redhat.com \
    --cc=andi@firstfloor.org \
    --cc=gorcunov@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=robert.richter@amd.com \
    --cc=ying.huang@intel.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

all inboxes | Powered by JetHome®