public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jeremy Fitzhardinge <jeremy@goop.org>
To: Prarit Bhargava <prarit@redhat.com>
Cc: linux-kernel@vger.kernel.org, akpm@linux-foundation.org,
	clalance@redhat.com, mingo@redhat.com, davej@redhat.com,
	Thilo.Cestonaro.external@fujitsu-siemens.com
Subject: Re: [PATCH]: Fix bogus softlockup warning with sysrq-t
Date: Tue, 27 Mar 2007 09:46:40 -0700	[thread overview]
Message-ID: <46094A70.9080901@goop.org> (raw)
In-Reply-To: <460902CF.1030202@redhat.com>

Prarit Bhargava wrote:
> I think that's a good idea -- I'll propose an add on patch to fix the
> sysrq-t case ...

I'm working on this patch at the moment.  I'm just wondering what
happens if you do a global re-enable while a CPU is locally disabled.  I
think it won't matter; it will end up in the "enabled but need to update
timestamp" state, and the next time it gets a timer tick, it will simply
update the timestamp and carry on.

(This is relative to the other two softlockup patches, but modified
since I posted them.)

    J

diff -r 4c81d8cafb67 drivers/char/sysrq.c
--- a/drivers/char/sysrq.c	Tue Mar 27 01:16:07 2007 -0700
+++ b/drivers/char/sysrq.c	Tue Mar 27 01:18:05 2007 -0700
@@ -408,6 +408,8 @@ void __handle_sysrq(int key, struct tty_
 	int i;
 	unsigned long flags;
 
+	softlockup_global_disable();
+
 	spin_lock_irqsave(&sysrq_key_table_lock, flags);
 	orig_log_level = console_loglevel;
 	console_loglevel = 7;
@@ -445,6 +447,8 @@ void __handle_sysrq(int key, struct tty_
 		console_loglevel = orig_log_level;
 	}
 	spin_unlock_irqrestore(&sysrq_key_table_lock, flags);
+
+	softlockup_global_enable();
 }
 
 /*
diff -r 4c81d8cafb67 include/linux/sched.h
--- a/include/linux/sched.h	Tue Mar 27 01:16:07 2007 -0700
+++ b/include/linux/sched.h	Tue Mar 27 01:18:05 2007 -0700
@@ -235,6 +235,8 @@ extern void softlockup_tick(void);
 extern void softlockup_tick(void);
 extern void softlockup_enable(void);
 extern void softlockup_disable(void);
+extern void softlockup_global_enable(void);
+extern void softlockup_global_disable(void);
 extern void spawn_softlockup_task(void);
 extern void touch_softlockup_watchdog(void);
 #else
@@ -245,6 +247,12 @@ static inline void softlockup_enable(voi
 {
 }
 static inline void softlockup_disable(void)
+{
+}
+static inline void softlockup_global_enable(void)
+{
+}
+static inline void softlockup_global_disable(void)
 {
 }
 static inline void spawn_softlockup_task(void)
diff -r 4c81d8cafb67 kernel/softlockup.c
--- a/kernel/softlockup.c	Tue Mar 27 01:16:07 2007 -0700
+++ b/kernel/softlockup.c	Tue Mar 27 01:18:05 2007 -0700
@@ -17,10 +17,16 @@
 
 static DEFINE_SPINLOCK(print_lock);
 
+enum enable {
+	SL_OFF = 0,		/* disabled */
+	SL_UPDATE,		/* enabled, but timestamp old */
+	SL_ON,			/* enabled */
+};
+
 static DEFINE_PER_CPU(unsigned long long, touch_timestamp);
 static DEFINE_PER_CPU(unsigned long long, print_timestamp);
 static DEFINE_PER_CPU(struct task_struct *, watchdog_task);
-static DEFINE_PER_CPU(int, enabled);
+static DEFINE_PER_CPU(enum enable, enabled);
 
 static int did_panic = 0;
 
@@ -39,6 +45,8 @@ void touch_softlockup_watchdog(void)
 void touch_softlockup_watchdog(void)
 {
 	__raw_get_cpu_var(touch_timestamp) = sched_clock();
+	barrier();			/* update timestamp before enable */
+	__raw_get_cpu_var(enabled) = SL_ON;
 }
 EXPORT_SYMBOL(touch_softlockup_watchdog);
 
@@ -57,13 +65,27 @@ void softlockup_enable(void)
 void softlockup_enable(void)
 {
 	touch_softlockup_watchdog();
-	barrier();			/* update timestamp before enable */
-	__get_cpu_var(enabled) = 1;
 }
 
 void softlockup_disable(void)
 {
-	__get_cpu_var(enabled) = 0;
+	__get_cpu_var(enabled) = SL_OFF;
+}
+
+void softlockup_global_enable()
+{
+	unsigned cpu;
+
+	for_each_online_cpu(cpu)
+		per_cpu(enabled, cpu) = SL_UPDATE;
+}
+
+void softlockup_global_disable()
+{
+	unsigned cpu;
+
+	for_each_online_cpu(cpu)
+		per_cpu(enabled, cpu) = SL_OFF;
 }
 
 /*
@@ -79,9 +101,19 @@ void softlockup_tick(void)
 
 	touch_timestamp = get_timestamp(&__get_cpu_var(touch_timestamp));
 
-	/* return if not enabled */
-	if (!__get_cpu_var(enabled))
-		return;
+	switch(__get_cpu_var(enabled)) {
+	case SL_OFF:
+		/* not enabled */
+		return;
+
+	case SL_UPDATE:
+		/* update timestamp */
+		touch_softlockup_watchdog();
+		break;
+
+	case SL_ON:
+		break;
+	}
 
 	print_timestamp = __get_cpu_var(print_timestamp);
 


  reply	other threads:[~2007-03-27 16:46 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-03-15 13:22 [PATCH]: Fix bogus softlockup warning with sysrq-t Prarit Bhargava
2007-03-23 23:46 ` Rick Lindsley
2007-03-27  5:43 ` Jeremy Fitzhardinge
2007-03-27  6:47   ` Cestonaro, Thilo (external)
2007-03-27 11:41   ` Prarit Bhargava
2007-03-27 16:46     ` Jeremy Fitzhardinge [this message]
2007-03-27 16:56       ` Prarit Bhargava
2007-03-27 17:35       ` Prarit Bhargava

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=46094A70.9080901@goop.org \
    --to=jeremy@goop.org \
    --cc=Thilo.Cestonaro.external@fujitsu-siemens.com \
    --cc=akpm@linux-foundation.org \
    --cc=clalance@redhat.com \
    --cc=davej@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=prarit@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