Readd panic blinking in 2.6
diff mbox series

Message ID m3llet4456.fsf@averell.firstfloor.org
State New, archived
Headers show
Series
  • Readd panic blinking in 2.6
Related show

Commit Message

Andi Kleen Sept. 29, 2004, 1:05 p.m. UTC
Later 2.4 had a feature that would make the keyboard LEDs
blink when a panic occurred.  This patch adds it to 2.6 too.

This is useful when your machine is in X and locks up.
With the blinking keyboard lights you at least know
that a panic happened, not that it randomly locked up.

I cleaned it up a bit and ported it to the new keyboard
driver. Unlike 2.4 it also works now with panic=...
and doesn't rely on the timer interrupt ticking anymore. 
It's also clear, now it uses a generic callback, no ifdefs.
Should also work now with modular keyboard driver
and the panic blink frequency can be configured in sysfs
(this is useful for a few KVMs who don't like this). Setting
it to 0 turns it off.

Work left to do: find some way to use HLT in the busy loops.
Currently machines eat a lot of power in panic and sometimes
they hang in this state for days until somebody can reset them.
Unfortunately this will require relying on the timer interrupts
again.

P.S. Before anyone asks: no, i'm not interested in morse code
output.





-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Comments

Vojtech Pavlik Sept. 29, 2004, 1:21 p.m. UTC | #1
On Wed, Sep 29, 2004 at 03:05:09PM +0200, Andi Kleen wrote:

> diff -u linux-2.6.9rc2-bk11/drivers/input/serio/i8042.c-PANIC linux-2.6.9rc2-bk11/drivers/input/serio/i8042.c
> --- linux-2.6.9rc2-bk11/drivers/input/serio/i8042.c-PANIC	2004-09-25 16:03:19.000000000 +0200
> +++ linux-2.6.9rc2-bk11/drivers/input/serio/i8042.c	2004-09-27 01:22:23.000000000 +0200
> @@ -887,6 +887,40 @@
>          0
>  };
>  
> +static int blink_frequency = 500;
> +module_param_named(panicblink, blink_frequency, int, 0600);
> +
> +#define DELAY mdelay(1), delay++
> +
> +/* Tell the user who may be running in X and not see the console that we have 
> +   panic'ed. This is to distingush panics from "real" lockups.  */
> +static long i8042_panic_blink(long count)
> +{ 
> +	long delay = 0;
> +	static long last_blink;
> +	static char led;
> +	/* Roughly 1/2s frequency. KDB uses about 1s. Make sure it is 
> +	   different. */
> +	if (!blink_frequency) 
> +		return 0;
> +	if (count - last_blink < blink_frequency)
> +		return 0;
> +	led ^= 0x01 | 0x04;
> +	while (i8042_read_status() & I8042_STR_IBF)
> +		DELAY;

This here relies on interrupts working, since to clear IBF you need to
read from the input port, which is only done in the interrupt routine.
What you should do here is to disable the interrupts on the i8042 by
setting the CTR to an appropriate value and then flush the input port
after a response from the keyboard has arrived.

> +	i8042_write_data(0xed); /* set leds */
> +	DELAY;
> +	while (i8042_read_status() & I8042_STR_IBF)
> +		DELAY;
> +	DELAY;
> +	i8042_write_data(led);
> +	DELAY;
> +	last_blink = count;
> +	return delay;
> +}  

Also, you shouldn't need the DELAY macro, as i8042.c already has the
primitives to do all the waiting.

Something like

	spin_lock_irqsave(&i8042_lock, flags);
	i8042_flush();
	i8042_ctr &= ~I8042_CTR_KBDINT & ~I8042_CTR_AUXINT;
	i8042_command(&i8042_ctr, I8042_CMD_CTL_WCTR);
	i8042_wait_write();
	i8042_write_data(0xed);
	i8042_wait_read();
	i8042_flush();
	i8042_wait_write();
	i8042_write_data(led);
	i8042_wait_read();
	i8042_flush();
	spin_unlock_irqrestore(&i8042_lock, flags);

would be safer and more correct.
Tonnerre Sept. 29, 2004, 10:05 p.m. UTC | #2
Salut,

On Wed, Sep 29, 2004 at 03:05:09PM +0200, Andi Kleen wrote:
> Later 2.4 had a feature that would make the keyboard LEDs
> blink when a panic occurred.  This patch adds it to 2.6 too.

What happened to the morse code panic patches? ;)

			    Tonnerre
Andi Kleen Sept. 30, 2004, 8:01 a.m. UTC | #3
On Thu, Sep 30, 2004 at 12:05:59AM +0200, Tonnerre wrote:
> Salut,
> 
> On Wed, Sep 29, 2004 at 03:05:09PM +0200, Andi Kleen wrote:
> > Later 2.4 had a feature that would make the keyboard LEDs
> > blink when a panic occurred.  This patch adds it to 2.6 too.
> 
> What happened to the morse code panic patches? ;)

You didn't read my full original mail, did you? 

-Andi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Andi Kleen Sept. 30, 2004, 8:42 a.m. UTC | #4
> 
> Something like
> 
> 	spin_lock_irqsave(&i8042_lock, flags);
> 	i8042_flush();
> 	i8042_ctr &= ~I8042_CTR_KBDINT & ~I8042_CTR_AUXINT;
> 	i8042_command(&i8042_ctr, I8042_CMD_CTL_WCTR);
> 	i8042_wait_write();
> 	i8042_write_data(0xed);
> 	i8042_wait_read();
> 	i8042_flush();
> 	i8042_wait_write();
> 	i8042_write_data(led);
> 	i8042_wait_read();
> 	i8042_flush();
> 	spin_unlock_irqrestore(&i8042_lock, flags);
> 
> would be safer and more correct.

That all takes far too many locks. The risk of deadlocking during
panic is too great. I think the delay is fine (worked great under 2.4),
just need to fix the IBF issue you mentioned.

-Andi

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Vojtech Pavlik Sept. 30, 2004, 9:06 a.m. UTC | #5
On Thu, Sep 30, 2004 at 10:42:24AM +0200, Andi Kleen wrote:
> > 
> > Something like
> > 
> > 	spin_lock_irqsave(&i8042_lock, flags);
> > 	i8042_flush();
> > 	i8042_ctr &= ~I8042_CTR_KBDINT & ~I8042_CTR_AUXINT;
> > 	i8042_command(&i8042_ctr, I8042_CMD_CTL_WCTR);
> > 	i8042_wait_write();
> > 	i8042_write_data(0xed);
> > 	i8042_wait_read();
> > 	i8042_flush();
> > 	i8042_wait_write();
> > 	i8042_write_data(led);
> > 	i8042_wait_read();
> > 	i8042_flush();
> > 	spin_unlock_irqrestore(&i8042_lock, flags);
> > 
> > would be safer and more correct.
> 
> That all takes far too many locks.

Ouch, yes. Just ignore the locks then. As it is, it would deadlock
immediately.

> The risk of deadlocking during
> panic is too great. I think the delay is fine (worked great under 2.4),
> just need to fix the IBF issue you mentioned.

Patch
diff mbox series

diff -u linux-2.6.9rc2-bk11/drivers/input/serio/i8042.c-PANIC linux-2.6.9rc2-bk11/drivers/input/serio/i8042.c
--- linux-2.6.9rc2-bk11/drivers/input/serio/i8042.c-PANIC	2004-09-25 16:03:19.000000000 +0200
+++ linux-2.6.9rc2-bk11/drivers/input/serio/i8042.c	2004-09-27 01:22:23.000000000 +0200
@@ -887,6 +887,40 @@ 
         0
 };
 
+static int blink_frequency = 500;
+module_param_named(panicblink, blink_frequency, int, 0600);
+
+#define DELAY mdelay(1), delay++
+
+/* Tell the user who may be running in X and not see the console that we have 
+   panic'ed. This is to distingush panics from "real" lockups.  */
+static long i8042_panic_blink(long count)
+{ 
+	long delay = 0;
+	static long last_blink;
+	static char led;
+	/* Roughly 1/2s frequency. KDB uses about 1s. Make sure it is 
+	   different. */
+	if (!blink_frequency) 
+		return 0;
+	if (count - last_blink < blink_frequency)
+		return 0;
+	led ^= 0x01 | 0x04;
+	while (i8042_read_status() & I8042_STR_IBF)
+		DELAY;
+	i8042_write_data(0xed); /* set leds */
+	DELAY;
+	while (i8042_read_status() & I8042_STR_IBF)
+		DELAY;
+	DELAY;
+	i8042_write_data(led);
+	DELAY;
+	last_blink = count;
+	return delay;
+}  
+
+#undef DELAY
+
 /*
  * Suspend/resume handlers for the new PM scheme (driver model)
  */
@@ -1047,6 +1081,8 @@ 
 
 	register_reboot_notifier(&i8042_notifier);
 
+	panic_blink = i8042_panic_blink;
+
 	return 0;
 }
 
@@ -1077,6 +1113,8 @@ 
 	driver_unregister(&i8042_driver);
 
 	i8042_platform_exit();
+
+	panic_blink = NULL;
 }
 
 module_init(i8042_init);
diff -u linux-2.6.9rc2-bk11/include/linux/kernel.h-PANIC linux-2.6.9rc2-bk11/include/linux/kernel.h
--- linux-2.6.9rc2-bk11/include/linux/kernel.h-PANIC	2004-09-25 16:03:29.000000000 +0200
+++ linux-2.6.9rc2-bk11/include/linux/kernel.h	2004-09-27 00:54:20.000000000 +0200
@@ -66,6 +66,7 @@ 
 	})
 
 extern struct notifier_block *panic_notifier_list;
+extern long (*panic_blink)(long time);
 NORET_TYPE void panic(const char * fmt, ...)
 	__attribute__ ((NORET_AND format (printf, 1, 2)));
 asmlinkage NORET_TYPE void do_exit(long error_code)
diff -u linux-2.6.9rc2-bk11/kernel/panic.c-PANIC linux-2.6.9rc2-bk11/kernel/panic.c
--- linux-2.6.9rc2-bk11/kernel/panic.c-PANIC	2004-09-19 12:21:52.000000000 +0200
+++ linux-2.6.9rc2-bk11/kernel/panic.c	2004-09-27 01:18:15.000000000 +0200
@@ -37,6 +37,15 @@ 
 }
 __setup("panic=", panic_setup);
 
+static long no_blink(long time)
+{
+	return 0;
+}
+
+/* Returns how long it waited in ms */
+long (*panic_blink)(long time) = no_blink;
+EXPORT_SYMBOL(panic_blink);
+
 /**
  *	panic - halt the system
  *	@fmt: The text string to print
@@ -49,6 +58,7 @@ 
  
 NORET_TYPE void panic(const char * fmt, ...)
 {
+	long i;
 	static char buf[1024];
 	va_list args;
 #if defined(CONFIG_ARCH_S390)
@@ -70,15 +80,16 @@ 
 
 	if (panic_timeout > 0)
 	{
-		int i;
 		/*
 	 	 * Delay timeout seconds before rebooting the machine. 
 		 * We can't use the "normal" timers since we just panicked..
 	 	 */
 		printk(KERN_EMERG "Rebooting in %d seconds..",panic_timeout);
-		for (i = 0; i < panic_timeout; i++) {
+		for (i = 0; i < panic_timeout*1000; ) {
 			touch_nmi_watchdog();
-			mdelay(1000);
+			i += panic_blink(i);
+			mdelay(1);
+			i++;
 		}
 		/*
 		 *	Should we run the reboot notifier. For the moment Im
@@ -99,8 +110,11 @@ 
         disabled_wait(caller);
 #endif
 	local_irq_enable();
-	for (;;)
-		;
+	for (i = 0;;) { 
+		i += panic_blink(i);
+		mdelay(1); 
+		i++; 
+	}
 }
 
 EXPORT_SYMBOL(panic);