2009-06-02 09:55:00

by Dave Young

[permalink] [raw]
Subject: [PATCH] printk: add halt_delay parameter for printk delay in halt phase

Add a halt_delay module parameter in printk.c used to read the printk
messages in halt/poweroff/restart phase, delay each printk messages
by halt_delay milliseconds. It is useful for debugging if there's no
other way to dump kernel messages that time.

halt_delay default value is 0, change it by:

echo xxx > /sys/module/printk/parameters/halt_delay

Signed-off-by: Dave Young <[email protected]>
---
kernel/printk.c | 21 +++++++++++++++++++++
lib/Kconfig.debug | 10 ++++++++++
2 files changed, 31 insertions(+)

--- linux-2.6.orig/kernel/printk.c 2009-04-09 16:23:03.000000000 +0800
+++ linux-2.6/kernel/printk.c 2009-06-02 17:45:54.000000000 +0800
@@ -250,6 +250,26 @@ static inline void boot_delay_msec(void)
}
#endif

+#ifdef CONFIG_HALT_PRINTK_DELAY
+/* msecs delay after each halt/poweroff/restart phase printk */
+static unsigned int halt_delay;
+
+static inline void halt_delay_msec(void)
+{
+ if (halt_delay == 0 || !(system_state == SYSTEM_HALT
+ || system_state == SYSTEM_POWER_OFF
+ || system_state == SYSTEM_RESTART))
+ return;
+
+ mdelay(halt_delay);
+}
+
+module_param(halt_delay, int, S_IRUGO | S_IWUSR);
+#else
+static inline void halt_delay_msec(void)
+{
+}
+#endif
/*
* Commands to do_syslog:
*
@@ -649,6 +669,7 @@ asmlinkage int vprintk(const char *fmt,
char *p;

boot_delay_msec();
+ halt_delay_msec();

preempt_disable();
/* This stops the holder of console_sem just where we want him */
--- linux-2.6.orig/lib/Kconfig.debug 2009-05-12 11:27:22.000000000 +0800
+++ linux-2.6/lib/Kconfig.debug 2009-06-02 17:50:15.000000000 +0800
@@ -647,6 +647,16 @@ config BOOT_PRINTK_DELAY
BOOT_PRINTK_DELAY also may cause DETECT_SOFTLOCKUP to detect
what it believes to be lockup conditions.

+config HALT_PRINTK_DELAY
+ bool "Delay halt/poweroff/restart printk message by N milliseconds"
+ depends on DEBUG_KERNEL && PRINTK && SYSFS
+ default n
+ help
+ This build option allows you to read kernel messages in
+ halt/poweroff/restart phase by inserting a short delay after
+ each one. The delay is specified in milliseconds on the
+ module parameter: /sys/module/printk/halt_delay.
+
config RCU_TORTURE_TEST
tristate "torture tests for RCU"
depends on DEBUG_KERNEL


2009-06-02 10:56:25

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] printk: add halt_delay parameter for printk delay in halt phase


* Dave Young <[email protected]> wrote:

> Add a halt_delay module parameter in printk.c used to read the printk
> messages in halt/poweroff/restart phase, delay each printk messages
> by halt_delay milliseconds. It is useful for debugging if there's no
> other way to dump kernel messages that time.

nice idea! We frequently have kernel-death warnings/messages that
scroll off too fast and which cannot be captured.

> halt_delay default value is 0, change it by:
>
> echo xxx > /sys/module/printk/parameters/halt_delay
>
> Signed-off-by: Dave Young <[email protected]>
> ---
> kernel/printk.c | 21 +++++++++++++++++++++
> lib/Kconfig.debug | 10 ++++++++++
> 2 files changed, 31 insertions(+)
>
> --- linux-2.6.orig/kernel/printk.c 2009-04-09 16:23:03.000000000 +0800
> +++ linux-2.6/kernel/printk.c 2009-06-02 17:45:54.000000000 +0800
> @@ -250,6 +250,26 @@ static inline void boot_delay_msec(void)
> }
> #endif
>
> +#ifdef CONFIG_HALT_PRINTK_DELAY

(this #ifdef is ugly - see below.)

> +/* msecs delay after each halt/poweroff/restart phase printk */
> +static unsigned int halt_delay;
> +
> +static inline void halt_delay_msec(void)
> +{
> + if (halt_delay == 0 || !(system_state == SYSTEM_HALT
> + || system_state == SYSTEM_POWER_OFF
> + || system_state == SYSTEM_RESTART))
> + return;
> +
> + mdelay(halt_delay);

>
> boot_delay_msec();
> + halt_delay_msec();


i think it should be done in boot_delay_msec() and the function
should be renamed to print_delay_msec() or so.

> preempt_disable();
> /* This stops the holder of console_sem just where we want him */
> --- linux-2.6.orig/lib/Kconfig.debug 2009-05-12 11:27:22.000000000 +0800
> +++ linux-2.6/lib/Kconfig.debug 2009-06-02 17:50:15.000000000 +0800
> @@ -647,6 +647,16 @@ config BOOT_PRINTK_DELAY
> BOOT_PRINTK_DELAY also may cause DETECT_SOFTLOCKUP to detect
> what it believes to be lockup conditions.
>
> +config HALT_PRINTK_DELAY
> + bool "Delay halt/poweroff/restart printk message by N milliseconds"
> + depends on DEBUG_KERNEL && PRINTK && SYSFS
> + default n
> + help
> + This build option allows you to read kernel messages in
> + halt/poweroff/restart phase by inserting a short delay after
> + each one. The delay is specified in milliseconds on the
> + module parameter: /sys/module/printk/halt_delay.

No need for a Kconfig option - this should be an unconditional
feature like boot-delay.

This will further simplify the patch and will get rid of that ugly
#ifdef.

Ingo

2009-06-02 13:27:17

by Dave Young

[permalink] [raw]
Subject: Re: [PATCH] printk: add halt_delay parameter for printk delay in halt phase

On Tue, Jun 2, 2009 at 6:56 PM, Ingo Molnar <[email protected]> wrote:
>
> * Dave Young <[email protected]> wrote:
>
>> Add a halt_delay module parameter in printk.c used to read the printk
>> messages in halt/poweroff/restart phase, delay each printk messages
>> by halt_delay milliseconds. It is useful for debugging if there's no
>> other way to dump kernel messages that time.
>
> nice idea! We frequently have kernel-death warnings/messages that
> scroll off too fast and which cannot be captured.

Thanks! It is for this case indeed.

>
>> halt_delay default value is 0, change it by:
>>
>> echo xxx > /sys/module/printk/parameters/halt_delay
>>
>> Signed-off-by: Dave Young <[email protected]>
>> ---
>> kernel/printk.c   |   21 +++++++++++++++++++++
>> lib/Kconfig.debug |   10 ++++++++++
>> 2 files changed, 31 insertions(+)
>>
>> --- linux-2.6.orig/kernel/printk.c    2009-04-09 16:23:03.000000000 +0800
>> +++ linux-2.6/kernel/printk.c 2009-06-02 17:45:54.000000000 +0800
>> @@ -250,6 +250,26 @@ static inline void boot_delay_msec(void)
>>  }
>>  #endif
>>
>> +#ifdef CONFIG_HALT_PRINTK_DELAY
>
> (this #ifdef is ugly - see below.)
>
>> +/* msecs delay after each halt/poweroff/restart phase printk */
>> +static unsigned int halt_delay;
>> +
>> +static inline void halt_delay_msec(void)
>> +{
>> +     if (halt_delay == 0 || !(system_state == SYSTEM_HALT
>> +                             || system_state == SYSTEM_POWER_OFF
>> +                             || system_state == SYSTEM_RESTART))
>> +             return;
>> +
>> +     mdelay(halt_delay);
>
>>
>>       boot_delay_msec();
>> +     halt_delay_msec();
>
>
> i think it should be done in boot_delay_msec() and the function
> should be renamed to print_delay_msec() or so.

I have two concerns:

1. boot_delay use a busy looping with lpj preset because it's too
early that mdelay is probably not ready yet. Furthermore the delay is
not accurate. For halt issue we can just use mdelay()

2. boot_delay is set by kernel boot paramenter, but IMHO, for halt
delay use sysfs file is more convenient.

>
>>       preempt_disable();
>>       /* This stops the holder of console_sem just where we want him */
>> --- linux-2.6.orig/lib/Kconfig.debug  2009-05-12 11:27:22.000000000 +0800
>> +++ linux-2.6/lib/Kconfig.debug       2009-06-02 17:50:15.000000000 +0800
>> @@ -647,6 +647,16 @@ config BOOT_PRINTK_DELAY
>>         BOOT_PRINTK_DELAY also may cause DETECT_SOFTLOCKUP to detect
>>         what it believes to be lockup conditions.
>>
>> +config HALT_PRINTK_DELAY
>> +     bool "Delay halt/poweroff/restart printk message by N milliseconds"
>> +     depends on DEBUG_KERNEL && PRINTK && SYSFS
>> +     default n
>> +     help
>> +       This build option allows you to read kernel messages in
>> +       halt/poweroff/restart phase by inserting a short delay after
>> +       each one.  The delay is specified in milliseconds on the
>> +       module parameter: /sys/module/printk/halt_delay.
>
> No need for a Kconfig option - this should be an unconditional
> feature like boot-delay.
>
> This will further simplify the patch and will get rid of that ugly
> #ifdef.

What do you think removing the Kconfig option, then unconditionally
use halt_delay functions?

>
>        Ingo
>



--
Regards
dave

2009-06-02 14:23:27

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] printk: add halt_delay parameter for printk delay in halt phase


* Dave Young <[email protected]> wrote:

> On Tue, Jun 2, 2009 at 6:56 PM, Ingo Molnar <[email protected]> wrote:
> >
> > * Dave Young <[email protected]> wrote:
> >
> >> Add a halt_delay module parameter in printk.c used to read the printk
> >> messages in halt/poweroff/restart phase, delay each printk messages
> >> by halt_delay milliseconds. It is useful for debugging if there's no
> >> other way to dump kernel messages that time.
> >
> > nice idea! We frequently have kernel-death warnings/messages that
> > scroll off too fast and which cannot be captured.
>
> Thanks! It is for this case indeed.
>
> >
> >> halt_delay default value is 0, change it by:
> >>
> >> echo xxx > /sys/module/printk/parameters/halt_delay
> >>
> >> Signed-off-by: Dave Young <[email protected]>
> >> ---
> >> kernel/printk.c ? | ? 21 +++++++++++++++++++++
> >> lib/Kconfig.debug | ? 10 ++++++++++
> >> 2 files changed, 31 insertions(+)
> >>
> >> --- linux-2.6.orig/kernel/printk.c ? ?2009-04-09 16:23:03.000000000 +0800
> >> +++ linux-2.6/kernel/printk.c 2009-06-02 17:45:54.000000000 +0800
> >> @@ -250,6 +250,26 @@ static inline void boot_delay_msec(void)
> >> ?}
> >> ?#endif
> >>
> >> +#ifdef CONFIG_HALT_PRINTK_DELAY
> >
> > (this #ifdef is ugly - see below.)
> >
> >> +/* msecs delay after each halt/poweroff/restart phase printk */
> >> +static unsigned int halt_delay;
> >> +
> >> +static inline void halt_delay_msec(void)
> >> +{
> >> + ? ? if (halt_delay == 0 || !(system_state == SYSTEM_HALT
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? || system_state == SYSTEM_POWER_OFF
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? || system_state == SYSTEM_RESTART))
> >> + ? ? ? ? ? ? return;
> >> +
> >> + ? ? mdelay(halt_delay);
> >
> >>
> >> ? ? ? boot_delay_msec();
> >> + ? ? halt_delay_msec();
> >
> >
> > i think it should be done in boot_delay_msec() and the function
> > should be renamed to print_delay_msec() or so.
>
> I have two concerns:
>
> 1. boot_delay use a busy looping with lpj preset because it's too
> early that mdelay is probably not ready yet. Furthermore the delay is
> not accurate. For halt issue we can just use mdelay()
>
> 2. boot_delay is set by kernel boot paramenter, but IMHO, for halt
> delay use sysfs file is more convenient.
>
> >
> >> ? ? ? preempt_disable();
> >> ? ? ? /* This stops the holder of console_sem just where we want him */
> >> --- linux-2.6.orig/lib/Kconfig.debug ?2009-05-12 11:27:22.000000000 +0800
> >> +++ linux-2.6/lib/Kconfig.debug ? ? ? 2009-06-02 17:50:15.000000000 +0800
> >> @@ -647,6 +647,16 @@ config BOOT_PRINTK_DELAY
> >> ? ? ? ? BOOT_PRINTK_DELAY also may cause DETECT_SOFTLOCKUP to detect
> >> ? ? ? ? what it believes to be lockup conditions.
> >>
> >> +config HALT_PRINTK_DELAY
> >> + ? ? bool "Delay halt/poweroff/restart printk message by N milliseconds"
> >> + ? ? depends on DEBUG_KERNEL && PRINTK && SYSFS
> >> + ? ? default n
> >> + ? ? help
> >> + ? ? ? This build option allows you to read kernel messages in
> >> + ? ? ? halt/poweroff/restart phase by inserting a short delay after
> >> + ? ? ? each one. ?The delay is specified in milliseconds on the
> >> + ? ? ? module parameter: /sys/module/printk/halt_delay.
> >
> > No need for a Kconfig option - this should be an unconditional
> > feature like boot-delay.
> >
> > This will further simplify the patch and will get rid of that ugly
> > #ifdef.
>
> What do you think removing the Kconfig option, then unconditionally
> use halt_delay functions?

that's fine too i think. The delay method assymetry between the two
functions indeed calls for them to be separate.

Ingo

2009-06-11 14:43:40

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH] printk: add halt_delay parameter for printk delay in halt phase

On Tue 2009-06-02 17:54:52, Dave Young wrote:
> Add a halt_delay module parameter in printk.c used to read the printk
> messages in halt/poweroff/restart phase, delay each printk messages
> by halt_delay milliseconds. It is useful for debugging if there's no
> other way to dump kernel messages that time.
>
> halt_delay default value is 0, change it by:
>
> echo xxx > /sys/module/printk/parameters/halt_delay

Well, stopping oopses from scrolling offscreen was very useful for me
in past.

> +static inline void halt_delay_msec(void)
> +{
> + if (halt_delay == 0 || !(system_state == SYSTEM_HALT
> + || system_state == SYSTEM_POWER_OFF
> + || system_state == SYSTEM_RESTART))
> + return;
> +
> + mdelay(halt_delay);

...but this will not trigger for oops, right?
Pavel

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2009-06-12 01:21:36

by Dave Young

[permalink] [raw]
Subject: Re: [PATCH] printk: add halt_delay parameter for printk delay in halt phase

On Thu, Jun 11, 2009 at 10:43 PM, Pavel Machek<[email protected]> wrote:
> On Tue 2009-06-02 17:54:52, Dave Young wrote:
>> Add a halt_delay module parameter in printk.c used to read the printk
>> messages in halt/poweroff/restart phase, delay each printk messages
>> by halt_delay milliseconds. It is useful for debugging if there's no
>> other way to dump kernel messages that time.
>>
>> halt_delay default value is 0, change it by:
>>
>> echo xxx > /sys/module/printk/parameters/halt_delay
>
> Well, stopping oopses from scrolling offscreen was very useful for me
> in past.
>
>> +static inline void halt_delay_msec(void)
>> +{
>> +     if (halt_delay == 0 || !(system_state == SYSTEM_HALT
>> +                             || system_state == SYSTEM_POWER_OFF
>> +                             || system_state == SYSTEM_RESTART))
>> +             return;
>> +
>> +     mdelay(halt_delay);
>
> ...but this will not trigger for oops, right?

Hi, Pavel, this is for halt phase only.

Andrew suggest to make it a general feature, thus we can enable printk
delay when we want. I will write a new patch for that.

>                                                                Pavel
>
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
>



--
Regards
dave