mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Don Zickus <dzickus@redhat.com>
To: <x86@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
	jwjstone@fastmail.fm, LKML <linux-kernel@vger.kernel.org>,
	Don Zickus <dzickus@redhat.com>
Subject: [PATCH 2/2 v2] watchdog:  Always return NOTIFY_OK during cpu up/down events
Date: Mon,  7 Mar 2011 16:37:40 -0500	[thread overview]
Message-ID: <1299533860-1642-2-git-send-email-dzickus@redhat.com> (raw)
In-Reply-To: <1299533860-1642-1-git-send-email-dzickus@redhat.com>

This patch addresses a couple of problems.  One was the case when the
hardlockup failed to start, it also failed to start the softlockup.
There were valid cases when the hardlockup shouldn't start and that
shouldn't block the softlockup (no lapic, bios controls perf counters).

The second problem was when the hardlockup failed to start on boxes
(from a no lapic or bios controlled perf counter case), it reported
failure to the cpu notifier chain.  This blocked the notifier from
continuing to start other more critical pieces of cpu bring-up (in
our case based on a 2.6.32 fork, it was the mce).  As a result,
during soft cpu online/offline testing, the system would panic
when a cpu was offlined because the cpu notifier would succeed in
processing a watchdog disable cpu event and would panic in the mce
case as a result of un-initialized variables from a never executed
cpu up event.

I realized the hardlockup/softlockup cases are really just debugging
aids and should never impede the progress of a cpu up/down event.
Therefore I modified the code to always return NOTIFY_OK and instead
rely on printks to inform the user of problems.

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

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index f7c0272..c52645b 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -418,19 +418,22 @@ static int watchdog_prepare_cpu(int cpu)
 static int watchdog_enable(int cpu)
 {
 	struct task_struct *p = per_cpu(softlockup_watchdog, cpu);
-	int err;
+	int err = 0;
 
 	/* enable the perf event */
 	err = watchdog_nmi_enable(cpu);
-	if (err)
-		return err;
+
+	/* Regardless of err above, fall through and start softlockup */
 
 	/* create the watchdog thread */
 	if (!p) {
 		p = kthread_create(watchdog, (void *)(unsigned long)cpu, "watchdog/%d", cpu);
 		if (IS_ERR(p)) {
 			printk(KERN_ERR "softlockup watchdog for %i failed\n", cpu);
-			return PTR_ERR(p);
+			if (!err)
+				/* if hardlockup hasn't already set this */
+				err = PTR_ERR(p);
+			goto out;
 		}
 		kthread_bind(p, cpu);
 		per_cpu(watchdog_touch_ts, cpu) = 0;
@@ -438,7 +441,8 @@ static int watchdog_enable(int cpu)
 		wake_up_process(p);
 	}
 
-	return 0;
+out:
+	return err;
 }
 
 static void watchdog_disable(int cpu)
@@ -550,7 +554,13 @@ cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu)
 		break;
 #endif /* CONFIG_HOTPLUG_CPU */
 	}
-	return notifier_from_errno(err);
+
+	/*
+	 * hardlockup and softlockup are not important enough
+	 * to block cpu bring up.  Just always succeed and
+	 * rely on printk output to flag problems.
+	 */
+	return NOTIFY_OK;
 }
 
 static struct notifier_block __cpuinitdata cpu_nfb = {
-- 
1.7.3.5


  reply	other threads:[~2011-03-07 21:38 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-07 21:37 [PATCH 1/2 v2] watchdog, nmi: Allow hardlockup to panic by default Don Zickus
2011-03-07 21:37 ` Don Zickus [this message]
2011-03-17  9:12   ` [PATCH 2/2 v2] watchdog: Always return NOTIFY_OK during cpu up/down events Peter Zijlstra
2011-03-17 12:16   ` WANG Cong
2011-03-17  9:12 ` [PATCH 1/2 v2] watchdog, nmi: Allow hardlockup to panic by default Peter Zijlstra
2011-03-17 12:05 ` WANG Cong
2011-03-18  1:50 ` Andrew Morton
2011-03-18 17:19   ` Don Zickus
2011-03-18 18:23     ` Andrew Morton
2011-03-18 18:58       ` Don Zickus

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=1299533860-1642-2-git-send-email-dzickus@redhat.com \
    --to=dzickus@redhat.com \
    --cc=jwjstone@fastmail.fm \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=x86@kernel.org \
    /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

Powered by JetHome