login
Search
From: Randy Dunlap <randy.dunlap@oracle.com>
Add "notime" boot option to prevent timing data from being printed on
each printk message line.
We've seen a few cases of 'time' data locking problems (possibly
involving netconsole or net drivers). If a kernel is built with
CONFIG_PRINTK_TIME=y, it may have a boot locking hang. Booting
with "notime" may (i.e., could) prevent the lock hang and allow the
kernel to boot successfully, without requiring a kernel rebuild.
Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
---
Documentation/kernel-parameters.txt | 2 ++
kernel/printk.c | 10 +++++++++-
2 files changed, 11 insertions(+), 1 deletion(-)
--- linux-2622-rc2.orig/Documentation/kernel-parameters.txt
+++ linux-2622-rc2/Documentation/kernel-parameters.txt
@@ -1227,6 +1227,8 @@ and is between 256 and 4096 characters.
nosync [HW,M68K] Disables sync negotiation for all devices.
+ notime Do not print timing data on each printk message line.
+
notsc [BUGS=IA-32] Disable Time Stamp Counter
nousb [USB] Disable the USB subsystem
--- linux-2622-rc2.orig/kernel/printk.c
+++ linux-2622-rc2/kernel/printk.c
@@ -458,9 +458,17 @@ static int __init printk_time_setup(char
printk_time = 1;
return 1;
}
-
__setup("time", printk_time_setup);
+static int __init printk_notime_setup(char *str)
+{
+ if (*str)
+ return 0;
+ printk_time = 0;
+ return 1;
+}
+__setup("notime", printk_notime_setup);
+
__attribute__((weak)) unsigned long long printk_clock(void)
{
return sched_clock();
-
On Tue, May 22, 2007 at 12:09:38PM -0700, Randy Dunlap wrote:
> --- linux-2622-rc2.orig/kernel/printk.c
> +++ linux-2622-rc2/kernel/printk.c
> @@ -458,9 +458,17 @@ static int __init printk_time_setup(char
> printk_time = 1;
> return 1;
> }
> -
> __setup("time", printk_time_setup);
>
> +static int __init printk_notime_setup(char *str)
> +{
> + if (*str)
> + return 0;
> + printk_time = 0;
> + return 1;
> +}
> +__setup("notime", printk_notime_setup);
> +
> __attribute__((weak)) unsigned long long printk_clock(void)
> {
> return sched_clock();
Not disagreeing with the patch, but I wonder if it'd be a net win
if we had a helper that would replace the zillion instances
of "set this variable to a 0/1 depending if its prefixed with 'no'"
with 1-2 lines.
Dave
--
http://www.codemonkey.org.uk
-
I don't know about that, but I had another version of this patch that removed the use of __setup() and just used the module_param() that is already there (but with the param renamed to "time" and changed to a bool), so that the usage on the command line is: linux printk.time=<bool> I like this alternate version pretty well, but it causes more change and a usage that is not well-known. Patch is below. The __setup() declaration and its function helper are not removed yet, but they could be, and just leave the value-setting job to the module_param() code. All tested. --- From: Randy Dunlap <randy.dunlap@oracle.com> Allow printk_time to be enabled or disabled at boot time. Previously it could be enabled only, but not disabled. Change printk_time from an int to a bool since that's what it is. Make its logical (exposed) name just be "time". Note: Changes kernel boot option syntax from "time" to "time=value". Since printk_time is declared as a module_param, it can also be changed at run-time by modifying /sys/module/printk/parameters/time to a value of 1/Y/y to enabled it or 0/N/n to disable it. Since printk_time is declared as a module_param, its value can also be set at boot-time by using linux printk.time=<bool> If we are willing to drop the shorter "time=value" syntax, we could also drop the entire printk_time_setup() function and the __setup() for it. Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com> --- Documentation/kernel-parameters.txt | 5 ++++- kernel/printk.c | 12 +++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) --- linux-2622-rc2.orig/Documentation/kernel-parameters.txt +++ linux-2622-rc2/Documentation/kernel-parameters.txt @@ -1824,7 +1824,10 @@ and is between 256 and 4096 characters. thash_entries= [KNL,NET] Set number of hash buckets for TCP connection - time Show timing data prefixed to each printk message line + time= Show timing data prefixed to each printk message ...
That's a good source of confusion. To me, "notime" means something like "don't bother calculating time", instead of the proposed behavior. Can't it be something like 'nologts' (no log timestamps) or nots or notimestamps or nologtime instead? /mjt -
"nologtime" is OK with me. or does it confuse people in a different way? Anyone else? --- ~Randy *** Remember to use Documentation/SubmitChecklist when testing your code *** -
The CONFIG option is called CONFIG_PRINTK_TIME. How about "noprintktime"? At least nicely to the point... Rene. -
Actually I'm concerned about total kernel command line length, so using option names that are "long" when short will do is not good IMO. I.e., I can easily overflow a 255-byte command line length buffer, so Shorter is Better. However, "when short will do" does not mean making them unreadable or like they mean something else, like "nopkt". --- ~Randy *** Remember to use Documentation/SubmitChecklist when testing your code *** -
Okay. I would by the way not be against turning the timestamping off by default and turning it _on_ with a "timestamps" or "logtime" or whatever option. The information is sometimes handy for seeing the (clustering of) event times so I've been compiling it in for a while on some boxes but in the majority case for me it's noise taking up printk real estate... Rene. -
But CONFIG_PRINTK_TIME is what controls its "default" (build-time) value. I.e., users can control that. I would be OK with removing that config option and only being able to enable it, but I doubt that this would have much support. ;) -- ~Randy *** Remember to use Documentation/SubmitChecklist when testing your code *** -
Yes, but a (full) kernel recompile is a bit of a hard-hitting switch, Fine by me. Rene. -
Yes I only now looked. Sorry, didn't realise that was how it already worked. CONFIG_PRINTK_TIME seems a little silly in that case but _someone_ thought it was worth spending an option on, so hey... Rene. -
Wonderful, now we have _three_ options to control time... time=<bool> printk.time=<bool> notime=<bool> Jan -- -
Currently the middle one is "printk.printk_time". And the first and last are just "time" and "notime", without a bool value. And as I wrote later, I'd be happy to remove #s 1 and 3 and use only the middle one, renaming the param so that it _is_ printk.time. -- ~Randy *** Remember to use Documentation/SubmitChecklist when testing your code *** -
You get my (+) vote for that. Jan -- -
From: Randy Dunlap <randy.dunlap@oracle.com> Andrew, please drop add-notime-boot-option.patch and use this patch instead... Allow printk_time to be enabled or disabled at boot time. Previously it could be enabled only, but not disabled. Change printk_time from an int to a bool since that's what it is. Make its logical (exposed) name just be "time" (was "printk_time"). Note: Changes kernel boot option syntax from "time" to "printk.time=value". Since printk_time is declared as a module_param, it can also be changed at run-time by modifying /sys/module/printk/parameters/time to a value of 1/Y/y to enabled it or 0/N/n to disable it. Since printk_time is declared as a module_param, its value can also be set at boot-time by using linux printk.time=<bool> Drop the "time" boot option and its __setup() functions. Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com> --- Documentation/kernel-parameters.txt | 5 +++-- kernel/printk.c | 12 +----------- 2 files changed, 4 insertions(+), 13 deletions(-) --- linux-2622-rc2g5.orig/Documentation/kernel-parameters.txt +++ linux-2622-rc2g5/Documentation/kernel-parameters.txt @@ -1406,6 +1406,9 @@ and is between 256 and 4096 characters. autoconfiguration. Ranges are in pairs (memory base and size). + printk.time= Show timing data prefixed to each printk message line + Format: <bool> (1/Y/y=enable, 0/N/n=disable) + profile= [KNL] Enable kernel profiling via /proc/profile Format: [schedule,]<number> Param: "schedule" - profile schedule points. @@ -1805,8 +1808,6 @@ and is between 256 and 4096 characters. thash_entries= [KNL,NET] Set number of hash buckets for TCP connection - time Show timing data prefixed to each printk message line - clocksource= [GENERIC_TIME] Override the default clocksource Override the default clocksource and use the clocksource with the name specified. --- linux-2622-rc2g5.orig/kernel/printk.c +++ ...
On Thu, 24 May 2007 17:15:35 -0700 That's going to break a *lot* of people's setups - timestamping appears to be very popular. We will get sad emails from people. If we're going to do this then I'm afraid we should retain the `time=' thing for a while and add a this-is-going-away printk to it. -
Reminds me of http://lkml.org/lkml/2007/5/1/163 (minus the ANSI sequences, though that WOULD have gotten their attention)... Jan -- -
[$ checkpatch-v2.pl ~/patch/printk-time-onoff.patch Your patch has no obvious style problems and is ready for submission.] From: Randy Dunlap <randy.dunlap@oracle.com> Allow printk_time to be enabled or disabled at boot time. Previously it could be enabled only, but not disabled. Change printk_time from an int to a bool since that's what it is. Make its logical (exposed) name just be "time" (was "printk_time"). Note: Changes kernel boot option syntax from "time" to "printk.time=value". Since printk_time is declared as a module_param, it can also be changed at run-time by modifying /sys/module/printk/parameters/time to a value of 1/Y/y to enabled it or 0/N/n to disable it. Since printk_time is declared as a module_param, its value can also be set at boot-time by using linux printk.time=<bool> If the "time" boot option is used, print a message that it is deprecated and will be removed. Note its planned removal in feature-removal-schedule.txt. Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com> --- Documentation/feature-removal-schedule.txt | 7 +++++++ Documentation/kernel-parameters.txt | 4 ++++ kernel/printk.c | 5 ++++- 3 files changed, 15 insertions(+), 1 deletion(-) --- linux-2622-rc3.orig/Documentation/kernel-parameters.txt +++ linux-2622-rc3/Documentation/kernel-parameters.txt @@ -1426,6 +1426,9 @@ and is between 256 and 4096 characters. autoconfiguration. Ranges are in pairs (memory base and size). + printk.time= Show timing data prefixed to each printk message line + Format: <bool> (1/Y/y=enable, 0/N/n=disable) + profile= [KNL] Enable kernel profiling via /proc/profile Format: [schedule,]<number> Param: "schedule" - profile schedule points. @@ -1826,6 +1829,7 @@ and is between 256 and 4096 characters. Set number of hash buckets for TCP connection time Show timing data prefixed to each printk message line + [deprecated, see 'printk.time'] ...
