2004-09-29 13:07:05

by Andi Kleen

[permalink] [raw]
Subject: [PATCH] Readd panic blinking in 2.6


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.

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);





2004-09-29 13:23:17

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: [PATCH] Readd panic blinking in 2.6

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.

--
Vojtech Pavlik
SuSE Labs, SuSE CR

2004-09-29 22:32:31

by Tonnerre

[permalink] [raw]
Subject: Re: [PATCH] Readd panic blinking in 2.6

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


Attachments:
(No filename) (259.00 B)
signature.asc (189.00 B)
Digital signature
Download all attachments

2004-09-30 08:01:11

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] Readd panic blinking in 2.6

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

2004-09-30 08:42:49

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] Readd panic blinking in 2.6

>
> 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

2004-09-30 09:06:22

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: [PATCH] Readd panic blinking in 2.6

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.

--
Vojtech Pavlik
SuSE Labs, SuSE CR