[Devel] [PATCH RHEL7 COMMIT] ms/kernel/watchdog.c: touch_nmi_watchdog should only touch local cpu not every one

Konstantin Khorenko khorenko at virtuozzo.com
Thu Mar 24 07:40:39 PDT 2016


The commit is pushed to "branch-rh7-3.10.0-327.10.1.vz7.12.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-327.10.1.vz7.12.3
------>
commit d1ae98480f8caeb0d337b04a80e9457a9dfe4333
Author: Ben Zhang <benzh at chromium.org>
Date:   Thu Mar 24 18:40:26 2016 +0400

    ms/kernel/watchdog.c: touch_nmi_watchdog should only touch local cpu not every one
    
    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.
    
    Don said:
    
    : I'm ok with this patch, though it does alter the behaviour of how
    : touch_nmi_watchdog works.  For the most part I don't think most callers
    : need to touch all of the watchdogs (on each cpu).  Perhaps a corner case
    : will pop up (the scheduler??  to mimic touch_all_softlockup_watchdogs() ).
    :
    : But this does address an issue where if a system is locked up and one cpu
    : is spewing out useful debug messages (or error messages), the hard lockup
    : will fail to go off.  We have seen this on RHEL also.
    
    Signed-off-by: Don Zickus <dzickus at redhat.com>
    Signed-off-by: Ben Zhang <benzh at chromium.org>
    Signed-off-by: Andrew Morton <akpm at linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
    
    (cherry picked from commit 62572e29bc530b38921ef6059088b4788a9832a5)
    Signed-off-by: Vladimir Davydov <vdavydov at virtuozzo.com>
    Acked-by: Dmitry Monakhov <dmonakhov at virtuozzo.com>
---
 kernel/watchdog.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 8290af4..ba61141 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -201,14 +201,14 @@ void touch_all_softlockup_watchdogs(void)
 #ifdef CONFIG_HARDLOCKUP_DETECTOR
 void touch_nmi_watchdog(void)
 {
-	if (watchdog_user_enabled) {
-		unsigned cpu;
-
-		for_each_present_cpu(cpu) {
-			if (per_cpu(watchdog_nmi_touch, cpu) != true)
-				per_cpu(watchdog_nmi_touch, cpu) = true;
-		}
-	}
+	/*
+	 * 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;
 	touch_softlockup_watchdog();
 }
 EXPORT_SYMBOL(touch_nmi_watchdog);


More information about the Devel mailing list