virtualization.lists.linux-foundation.org archive mirror
 help / color / mirror / Atom feed
From: Jeremy Fitzhardinge <jeremy@goop.org>
To: Ingo Molnar <mingo@elte.hu>
Cc: Prarit Bhargava <prarit@redhat.com>,
	Rick Lindsley <ricklind@us.ibm.com>,
	john stultz <johnstul@us.ibm.com>,
	Eric Dumazet <dada1@cosmosbay.com>,
	virtualization@lists.osdl.org, Paul Mackerras <paulus@samba.org>,
	Martin Schwidefsky <schwidefsky@de.ibm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Linux Kernel <linux-kernel@vger.kernel.org>
Subject: [patch 1/4] Ignore stolen time in the softlockup watchdog
Date: Tue, 27 Mar 2007 14:49:20 -0700	[thread overview]
Message-ID: <20070327215827.871954359@goop.org> (raw)
In-Reply-To: 20070327214919.800272641@goop.org

[-- Attachment #1: softlockup-ignore-stolen-time.patch --]
[-- Type: text/plain, Size: 4125 bytes --]

The softlockup watchdog is currently a nuisance in a virtual machine,
since the whole system could have the CPU stolen from it for a long
period of time.  While it would be unlikely for a guest domain to be
denied timer interrupts for over 10s, it could happen and any softlockup
message would be completely spurious.

Earlier I proposed that sched_clock() return time in unstolen
nanoseconds, which is how Xen and VMI currently implement it.  If the
softlockup watchdog uses sched_clock() to measure time, it would
automatically ignore stolen time, and therefore only report when the
guest itself locked up.  When running native, sched_clock() returns
real-time nanoseconds, so the behaviour would be unchanged.

Note that sched_clock() used this way is inherently per-cpu, so this
patch makes sure that the per-processor watchdog thread initialized
its own timestamp.

Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: john stultz <johnstul@us.ibm.com>
Cc: Zachary Amsden <zach@vmware.com>
Cc: James Morris <jmorris@namei.org>
Cc: Dan Hecht <dhecht@vmware.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Chris Lalancette <clalance@redhat.com>
Cc: Rick Lindsley <ricklind@us.ibm.com>
Cc: Eric Dumazet <dada1@cosmosbay.com>
---
 kernel/softlockup.c |   37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)

===================================================================
--- a/kernel/softlockup.c
+++ b/kernel/softlockup.c
@@ -35,9 +35,19 @@ static struct notifier_block panic_block
 	.notifier_call = softlock_panic,
 };
 
+/*
+ * Returns seconds, approximately.  We don't need nanosecond
+ * resolution, and we don't need to waste time with a big divide when
+ * 2^30ns == 1.074s.
+ */
+static unsigned long get_timestamp(void)
+{
+	return sched_clock() >> 30;  /* 2^30 ~= 10^9 */
+}
+
 void touch_softlockup_watchdog(void)
 {
-	__raw_get_cpu_var(touch_timestamp) = jiffies;
+	__raw_get_cpu_var(touch_timestamp) = get_timestamp();
 }
 EXPORT_SYMBOL(touch_softlockup_watchdog);
 
@@ -48,10 +58,18 @@ void softlockup_tick(void)
 void softlockup_tick(void)
 {
 	int this_cpu = smp_processor_id();
-	unsigned long touch_timestamp = per_cpu(touch_timestamp, this_cpu);
+	unsigned long touch_timestamp = __get_cpu_var(touch_timestamp);
+	unsigned long print_timestamp;
+	unsigned long now;
 
-	/* prevent double reports: */
-	if (per_cpu(print_timestamp, this_cpu) == touch_timestamp ||
+	/* watchdog task hasn't updated timestamp yet */
+	if (touch_timestamp == 0)
+		return;
+
+	print_timestamp = __get_cpu_var(print_timestamp);
+
+	/* report at most once a second */
+	if (print_timestamp < (touch_timestamp + 1) ||
 		did_panic ||
 			!per_cpu(watchdog_task, this_cpu))
 		return;
@@ -62,12 +80,14 @@ void softlockup_tick(void)
 		return;
 	}
 
+	now = get_timestamp();
+
 	/* Wake up the high-prio watchdog task every second: */
-	if (time_after(jiffies, touch_timestamp + HZ))
+	if (now > (touch_timestamp + 1))
 		wake_up_process(per_cpu(watchdog_task, this_cpu));
 
 	/* Warn about unreasonable 10+ seconds delays: */
-	if (time_after(jiffies, touch_timestamp + 10*HZ)) {
+	if (now > (touch_timestamp + 10)) {
 		per_cpu(print_timestamp, this_cpu) = touch_timestamp;
 
 		spin_lock(&print_lock);
@@ -87,6 +107,9 @@ static int watchdog(void * __bind_cpu)
 
 	sched_setscheduler(current, SCHED_FIFO, &param);
 	current->flags |= PF_NOFREEZE;
+
+	/* initialize timestamp */
+	touch_softlockup_watchdog();
 
 	/*
 	 * Run briefly once per second to reset the softlockup timestamp.
@@ -120,7 +143,7 @@ cpu_callback(struct notifier_block *nfb,
 			printk("watchdog for %i failed\n", hotcpu);
 			return NOTIFY_BAD;
 		}
-  		per_cpu(touch_timestamp, hotcpu) = jiffies;
+  		per_cpu(touch_timestamp, hotcpu) = 0;
   		per_cpu(watchdog_task, hotcpu) = p;
 		kthread_bind(p, hotcpu);
  		break;

-- 

  reply	other threads:[~2007-03-27 21:49 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-03-27 21:49 [patch 0/4] Revised softlockup watchdog improvement patches Jeremy Fitzhardinge
2007-03-27 21:49 ` Jeremy Fitzhardinge [this message]
2007-04-24  6:49   ` [patch 1/4] Ignore stolen time in the softlockup watchdog Andrew Morton
2007-04-24  6:58     ` Jeremy Fitzhardinge
2007-04-24  7:09       ` Andrew Morton
2007-04-24 17:51     ` Jeremy Fitzhardinge
2007-04-24 17:57       ` Andrew Morton
2007-04-24 18:16         ` Jeremy Fitzhardinge
2007-04-24 18:32           ` Andrew Morton
2007-04-24 20:00             ` Jeremy Fitzhardinge
2007-04-24 20:14               ` Andrew Morton
2007-04-24 20:46                 ` Jeremy Fitzhardinge
2007-04-24 20:24               ` Jeremy Fitzhardinge
2007-04-24 20:33                 ` Andrew Morton
2007-04-24 20:48                   ` Jeremy Fitzhardinge
2007-04-24 20:52                 ` Daniel Walker
2007-04-24 20:59                   ` Ingo Molnar
2007-04-24 21:01                     ` Daniel Walker
2007-04-24 21:14                     ` Andrew Morton
2007-04-24 21:20                   ` Andi Kleen
2007-04-24 21:33                     ` Daniel Walker
2007-03-27 21:49 ` [patch 2/4] percpu enable flag for " Jeremy Fitzhardinge
2007-03-27 21:49 ` [patch 3/4] Locally disable the softlockup watchdog rather than touching it Jeremy Fitzhardinge
2007-03-28 13:33   ` Prarit Bhargava
2007-03-28 13:50     ` Andi Kleen
2007-03-28 14:00       ` Prarit Bhargava
2007-03-28 14:09         ` Andi Kleen
2007-03-28 14:13           ` Prarit Bhargava
2007-03-28 14:44     ` Jeremy Fitzhardinge
2007-03-28 14:51       ` Prarit Bhargava
2007-03-28 15:22         ` Jeremy Fitzhardinge
2007-03-28 15:27           ` Prarit Bhargava
2007-03-27 21:49 ` [patch 4/4] Add global disable/enable for softlockup watchdog Jeremy Fitzhardinge

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=20070327215827.871954359@goop.org \
    --to=jeremy@goop.org \
    --cc=akpm@linux-foundation.org \
    --cc=dada1@cosmosbay.com \
    --cc=johnstul@us.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=paulus@samba.org \
    --cc=prarit@redhat.com \
    --cc=ricklind@us.ibm.com \
    --cc=schwidefsky@de.ibm.com \
    --cc=tglx@linutronix.de \
    --cc=virtualization@lists.osdl.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;
as well as URLs for NNTP newsgroup(s).