[PATCH] Always print panic message on current console

[PATCH] Always print panic message on current console

[Bernhard Walle]

The kernel offers with TIOCL_GETKMSGREDIRECT ioctl() the possibility to
redirect the kernel messages to a specific console.

However, since it's not possible to switch to the kernel message console after
a panic(), it would be nice if the kernel would print the panic message on the
current console.


Signed-off-by: Bernhard Walle <bernhard@bwalle.de>
---
 kernel/panic.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/kernel/panic.c b/kernel/panic.c
index bcdef26..f9950e3 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -22,6 +22,7 @@
 #include <linux/init.h>
 #include <linux/nmi.h>
 #include <linux/dmi.h>
+#include <linux/tty.h>
 
 int panic_on_oops;
 static unsigned long tainted_mask;
@@ -65,6 +66,9 @@ NORET_TYPE void panic(const char * fmt, ...)
 	 */
 	preempt_disable();
 
+	/* don't redirect the panic message to some hidden console */
+	kmsg_redirect = 0;
+
 	bust_spinlocks(1);
 	va_start(args, fmt);
 	vsnprintf(buf, sizeof(buf), fmt, args);
-- 
1.6.0.4

Re: [PATCH] Always print panic message on current console

[Andrew Morton]

On Sun, 27 Sep 2009 14:36:54 +0200
Bernhard Walle <bernhard@bwalle.de> wrote:

> The kernel offers with TIOCL_GETKMSGREDIRECT ioctl() the possibility to
> redirect the kernel messages to a specific console.
> 
> However, since it's not possible to switch to the kernel message console after
> a panic(), it would be nice if the kernel would print the panic message on the
> current console.
> 
> 
> Signed-off-by: Bernhard Walle <bernhard@bwalle.de>
> ---
>  kernel/panic.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/panic.c b/kernel/panic.c
> index bcdef26..f9950e3 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -22,6 +22,7 @@
>  #include <linux/init.h>
>  #include <linux/nmi.h>
>  #include <linux/dmi.h>
> +#include <linux/tty.h>

It's odd that kmsg_redirect is declared in tty.h when it's purely a
vt.c thing.  Why not vt.h?

>  int panic_on_oops;
>  static unsigned long tainted_mask;
> @@ -65,6 +66,9 @@ NORET_TYPE void panic(const char * fmt, ...)
>  	 */
>  	preempt_disable();
>  
> +	/* don't redirect the panic message to some hidden console */
> +	kmsg_redirect = 0;
> +
>  	bust_spinlocks(1);
>  	va_start(args, fmt);
>  	vsnprintf(buf, sizeof(buf), fmt, args);

Methinks CONFIG_HW_CONSOLE=n kernels won't link after making this
change.

I'd suggest something like

#ifdef CONFIG_HW_CONSOLE
extern void vt_set_kmsg_redirect(int vt);
#else
static inline void vt_set_kmsg_redirect(int vt)
{
}
#endif


Another possible way of doing this would be for vt.c to hook itself
into panic_notifier_list.  That's nice and clean from a
separation-of-subsystems POV but isn't really conceptually correct - it
assumes that other entries on panic_notifier_list don't print messages
to the console.

Re: [PATCH] Always print panic message on current console

[Bernhard Walle]

Hi,

Andrew Morton <akpm@linux-foundation.org>:
>>
>> diff --git a/kernel/panic.c b/kernel/panic.c
>> index bcdef26..f9950e3 100644
>> --- a/kernel/panic.c
>> +++ b/kernel/panic.c
>> @@ -22,6 +22,7 @@
>> #include <linux/init.h>
>> #include <linux/nmi.h>
>> #include <linux/dmi.h>
>> +#include <linux/tty.h>
>
> It's odd that kmsg_redirect is declared in tty.h when it's purely a
> vt.c thing.  Why not vt.h?

Sorry, I don't know. It was there before my patch. :-)

>> int panic_on_oops;
>> static unsigned long tainted_mask;
>> @@ -65,6 +66,9 @@ NORET_TYPE void panic(const char * fmt, ...)
>> 	 */
>> 	preempt_disable();
>>
>> +	/* don't redirect the panic message to some hidden console */
>> +	kmsg_redirect = 0;
>> +
>> 	bust_spinlocks(1);
>> 	va_start(args, fmt);
>> 	vsnprintf(buf, sizeof(buf), fmt, args);
>
> Methinks CONFIG_HW_CONSOLE=n kernels won't link after making this
> change.
>
> I'd suggest something like
>
> #ifdef CONFIG_HW_CONSOLE
> extern void vt_set_kmsg_redirect(int vt);
> #else
> static inline void vt_set_kmsg_redirect(int vt)
> {
> }
> #endif

Sounds good. I will send a new patch for this.

> Another possible way of doing this would be for vt.c to hook itself
> into panic_notifier_list.  That's nice and clean from a
> separation-of-subsystems POV but isn't really conceptually correct - it
> assumes that other entries on panic_notifier_list don't print messages
> to the console.

I also thought about that, but I found it a bit oversized just for setting
a variable (in the panic case). Also, the panic() function does other things
(like blinking the keyboard LEDs) very direct, without abstraction. But
however, if you or other people think that this is the way to go, I have
no objections and will create a patch.


Regards,
Bernhard