Re: [PATCH 2/2] allow printk delay after multi lines

[Dave Young]

On Tue, Feb 9, 2010 at 5:56 AM, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Sat, 6 Feb 2010 21:40:56 +0800
> Dave Young <hidave.darkstar@xxxxxxxxx> wrote:
>
>> printk delay help us to capture printk messages on some unconvenient senarios,
>> but it is still not easy to read.
>>
>> Add another sysctl variable printk_delay_per_lines to make it more readable.
>> We can set the lines according to screen height, then take pictures by camera.
>>
>> kmesg will delay printk_delay_per_lines * printk_delay_msecs milliseconds
>> after every printk_delay_per_lines lines when printk_delay is enabled.
>>
>> Setting the lines by proc/sysctl interface:
>> /proc/sys/kernel/printk_delay_per_lines
>>
>> Andrew, sorry, I have not find time to cleanup the kernel.h sysctl variables.
>> If I'm free I will try to do it.
>>
>> The value range from 1 - 100, default value is 1
>>
>> ...
>>
>> --- linux-2.6.orig/include/linux/kernel.h   2010-02-02 13:38:09.537495564 +0800
>> +++ linux-2.6/include/linux/kernel.h Â2010-02-02 13:40:47.657480122 +0800
>> @@ -246,6 +246,7 @@ extern bool printk_timed_ratelimit(unsig
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âunsigned int interval_msec);
>>
>> Âextern int printk_delay_msec;
>> +extern int printk_delay_per_lines;
>>
>> Â/*
>> Â * Print a one-time message (analogous to WARN_ONCE() et al):
>> --- linux-2.6.orig/kernel/printk.c  Â2010-02-02 13:39:19.446657319 +0800
>> +++ linux-2.6/kernel/printk.c 2010-02-02 13:40:47.660813615 +0800
>> @@ -656,16 +656,26 @@ static int new_text_line = 1;
>> Âstatic char printk_buf[1024];
>>
>> Âint printk_delay_msec __read_mostly;
>> +int printk_delay_per_lines __read_mostly;
>>
>> Âstatic inline void printk_delay(void)
>> Â{
>> Â Â Â if (unlikely(printk_delay_msec)) {
>> - Â Â Â Â Â Â int m = printk_delay_msec;
>> + Â Â Â Â Â Â static int m, l;
>>
>> + Â Â Â Â Â Â if (!l)
>> + Â Â Â Â Â Â Â Â Â Â l = printk_delay_per_lines;
>> +
>> + Â Â Â Â Â Â if (--l) {
>> + Â Â Â Â Â Â Â Â Â Â m += printk_delay_msec;
>> + Â Â Â Â Â Â Â Â Â Â return;
>> + Â Â Â Â Â Â }
>> + Â Â Â Â Â Â m += printk_delay_msec;
>> Â Â Â Â Â Â Â while (m--) {
>> Â Â Â Â Â Â Â Â Â Â Â mdelay(1);
>> Â Â Â Â Â Â Â Â Â Â Â touch_nmi_watchdog();
>> Â Â Â Â Â Â Â }
>> + Â Â Â Â Â Â m = 0;
>> Â Â Â }
>> Â}
>
> - The default value is zero, not 1. ÂAnd zero will be treated as 4G.
> ÂThat's a bug.

Will fix.

>
> - This feature would be a lot more useful if the user could specify
> Âprintk_delay_per_lines on the boot command line. ÂDitto
> Âprintk_delay_msec. ÂSo you can stop the important mesages from
> Âscrolling off. Â(I think there's already a way to do that, but I'm
> Âtoo lazy to go remember what it was).

I think it's the similar "boot_delay". It's a little different as
calibration is not done on the early booting phase, so busy loop
is used instead to delay. It's hard to make boot delay accurate.

Maybe delay_per_lines can be done while booting as well,
but it should be another patch,  isn't it?

>
> - The permitted range of 1-100 for printk_delay_per_lines seems
> Âarbitrary and unneeded. ÂWhy shouldn't I be able to set it to 10,000?
> ÂI see no harm in permitting that.
>
> - If the user sets printk_delay_per_lines=N, the kernel will pause
> Âfor N*printk_delay_msec every N lines. ÂThis is odd, and unintuitive.
> ÂAnd it'll really hurt if I set printk_delay_per_lines=10000!
>
> ÂI'd expect the kernel to pause for printk_delay_msec every N lines,
> Âand I think that would be better.

Fine to me as well. Thanks for your comments.

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