* [PATCH] printk: add halt_delay parameter for printk delay in halt phase
@ 2009-06-02 9:54 Dave Young
2009-06-02 10:56 ` Ingo Molnar
2009-06-11 14:43 ` Pavel Machek
0 siblings, 2 replies; 6+ messages in thread
From: Dave Young @ 2009-06-02 9:54 UTC (permalink / raw)
To: Linux Kernel Mailing List
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 <hidave.darkstar@gmail.com>
---
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] printk: add halt_delay parameter for printk delay in halt phase
2009-06-02 9:54 [PATCH] printk: add halt_delay parameter for printk delay in halt phase Dave Young
@ 2009-06-02 10:56 ` Ingo Molnar
2009-06-02 13:27 ` Dave Young
2009-06-11 14:43 ` Pavel Machek
1 sibling, 1 reply; 6+ messages in thread
From: Ingo Molnar @ 2009-06-02 10:56 UTC (permalink / raw)
To: Dave Young; +Cc: Linux Kernel Mailing List
* Dave Young <hidave.darkstar@gmail.com> 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 <hidave.darkstar@gmail.com>
> ---
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] printk: add halt_delay parameter for printk delay in halt phase
2009-06-02 10:56 ` Ingo Molnar
@ 2009-06-02 13:27 ` Dave Young
2009-06-02 14:22 ` Ingo Molnar
0 siblings, 1 reply; 6+ messages in thread
From: Dave Young @ 2009-06-02 13:27 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Linux Kernel Mailing List
On Tue, Jun 2, 2009 at 6:56 PM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Dave Young <hidave.darkstar@gmail.com> 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 <hidave.darkstar@gmail.com>
>> ---
>> 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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] printk: add halt_delay parameter for printk delay in halt phase
2009-06-02 13:27 ` Dave Young
@ 2009-06-02 14:22 ` Ingo Molnar
0 siblings, 0 replies; 6+ messages in thread
From: Ingo Molnar @ 2009-06-02 14:22 UTC (permalink / raw)
To: Dave Young; +Cc: Linux Kernel Mailing List
* Dave Young <hidave.darkstar@gmail.com> wrote:
> On Tue, Jun 2, 2009 at 6:56 PM, Ingo Molnar <mingo@elte.hu> wrote:
> >
> > * Dave Young <hidave.darkstar@gmail.com> 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 <hidave.darkstar@gmail.com>
> >> ---
> >> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] printk: add halt_delay parameter for printk delay in halt phase
2009-06-02 9:54 [PATCH] printk: add halt_delay parameter for printk delay in halt phase Dave Young
2009-06-02 10:56 ` Ingo Molnar
@ 2009-06-11 14:43 ` Pavel Machek
2009-06-12 1:21 ` Dave Young
1 sibling, 1 reply; 6+ messages in thread
From: Pavel Machek @ 2009-06-11 14:43 UTC (permalink / raw)
To: Dave Young; +Cc: Linux Kernel Mailing List
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] printk: add halt_delay parameter for printk delay in halt phase
2009-06-11 14:43 ` Pavel Machek
@ 2009-06-12 1:21 ` Dave Young
0 siblings, 0 replies; 6+ messages in thread
From: Dave Young @ 2009-06-12 1:21 UTC (permalink / raw)
To: Pavel Machek; +Cc: Linux Kernel Mailing List
On Thu, Jun 11, 2009 at 10:43 PM, Pavel Machek<pavel@ucw.cz> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-06-12 1:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-06-02 9:54 [PATCH] printk: add halt_delay parameter for printk delay in halt phase Dave Young
2009-06-02 10:56 ` Ingo Molnar
2009-06-02 13:27 ` Dave Young
2009-06-02 14:22 ` Ingo Molnar
2009-06-11 14:43 ` Pavel Machek
2009-06-12 1:21 ` Dave Young
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox