[PATCH] add "notime" boot option

[PATCH] add "notime" boot option

[Randy Dunlap]

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();

Re: [PATCH] add "notime" boot option

[Dave Jones]

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

Re: [PATCH] add "notime" boot option

[Randy Dunlap]

On Tue, 22 May 2007 15:40:30 -0400 Dave Jones wrote:

> 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.

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 line
+			Format: <bool>  (1=enable, 0=disable)
+or	printk.time=	Show timing data prefixed to each printk message line
+			Format: <bool>  (1/Y/y=enable, 0/N/n=disable)
 
 	tipar.timeout=	[HW,PPT]
 			Set communications timeout in tenths of a second
--- linux-2622-rc2.orig/kernel/printk.c
+++ linux-2622-rc2/kernel/printk.c
@@ -449,17 +449,19 @@ static int printk_time = 1;
 #else
 static int printk_time = 0;
 #endif
-module_param(printk_time, int, S_IRUGO | S_IWUSR);
+module_param_named(time, printk_time, bool, S_IRUGO | S_IWUSR);
 
 static int __init printk_time_setup(char *str)
 {
-	if (*str)
+	if (!*str)
 		return 0;
-	printk_time = 1;
+	if (*str == '1')
+		printk_time = 1;
+	else if (*str == '0')
+		printk_time = 0;
 	return 1;
 }
-
-__setup("time", printk_time_setup);
+__setup("time=", printk_time_setup);
 
 __attribute__((weak)) unsigned long long printk_clock(void)
 {

Re: [PATCH] add "notime" boot option

[Michael Tokarev]

Randy Dunlap wrote:
> 
> Add "notime" boot option to prevent timing data from being printed on
> each printk message line.

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

Re: [PATCH] add "notime" boot option

[Randy Dunlap]

On Wed, 23 May 2007 15:03:01 +0400 Michael Tokarev wrote:

> Randy Dunlap wrote:
> > 
> > Add "notime" boot option to prevent timing data from being printed on
> > each printk message line.
> 
> 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?

"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 ***

Re: [PATCH] add "notime" boot option

[Rene Herman]

On 05/23/2007 07:04 PM, Randy Dunlap wrote:

>> 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?
> 
> "nologtime" is OK with me.  or does it confuse people in a
> different way?  Anyone else?

The CONFIG option is called CONFIG_PRINTK_TIME. How about "noprintktime"? At 
least nicely to the point...

Rene.

Re: [PATCH] add "notime" boot option

[Randy Dunlap]

On Wed, 23 May 2007 21:15:20 +0200 Rene Herman wrote:

> On 05/23/2007 07:04 PM, Randy Dunlap wrote:
> 
> >> 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?
> > 
> > "nologtime" is OK with me.  or does it confuse people in a
> > different way?  Anyone else?
> 
> The CONFIG option is called CONFIG_PRINTK_TIME. How about "noprintktime"? At 
> least nicely to the point...

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 ***

Re: [PATCH] add "notime" boot option

[Rene Herman]

On 05/23/2007 10:55 PM, Randy Dunlap wrote:

>>>> 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
>>> 
>>> "nologtime" is OK with me.  or does it confuse people in a different
>>> way?  Anyone else?
>> 
>> The CONFIG option is called CONFIG_PRINTK_TIME. How about "noprintktime"? At 
>> least nicely to the point...
> 
> 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.

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.

Re: [PATCH] add "notime" boot option

[Randy Dunlap]

Rene Herman wrote:
> On 05/23/2007 10:55 PM, Randy Dunlap wrote:
> 
>>>>> 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
>>>>
>>>> "nologtime" is OK with me.  or does it confuse people in a different
>>>> way?  Anyone else?
>>>
>>> The CONFIG option is called CONFIG_PRINTK_TIME. How about 
>>> "noprintktime"? At least nicely to the point...
>>
>> 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.
> 
> 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...

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 ***

Re: [PATCH] add "notime" boot option

[Rene Herman]

On 05/24/2007 07:08 AM, Randy Dunlap wrote:

> Rene Herman wrote:

>> 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...
> 
> But CONFIG_PRINTK_TIME is what controls its "default" (build-time) value.
>  I.e., users can control that.

Yes, but a (full) kernel recompile is a bit of a hard-hitting switch, 
certainly on the older machines where I actually have it enabled...

> 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.  ;)

Fine by me.

Rene.

Re: [PATCH] add "notime" boot option

[Rene Herman]

On 05/24/2007 07:11 AM, Rene Herman wrote:

>>> 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.

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.

Re: [PATCH] add "notime" boot option

[Jan Engelhardt]

On May 22 2007 12:09, Randy Dunlap wrote:
>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.

Wonderful, now we have _three_ options to control time...

	time=<bool>
	printk.time=<bool>
	notime=<bool>


	Jan
-- 

Re: [PATCH] add "notime" boot option

[Randy Dunlap]

Jan Engelhardt wrote:
> On May 22 2007 12:09, Randy Dunlap wrote:
>> 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.
> 
> Wonderful, now we have _three_ options to control time...
> 
> 	time=<bool>
> 	printk.time=<bool>
> 	notime=<bool>

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 ***

Re: [PATCH] add "notime" boot option

[Jan Engelhardt]

On May 24 2007 09:24, Randy Dunlap wrote:
>> 
>> Wonderful, now we have _three_ options to control time...
>> 
>> time=<bool>
>> printk.time=<bool>
>> notime=<bool>
>
> 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.

You get my (+) vote for that.



	Jan
-- 

[PATCH] use printk.time option, drop time/notime

[Randy Dunlap]

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
+++ linux-2622-rc2g5/kernel/printk.c
@@ -449,17 +449,7 @@ static int printk_time = 1;
 #else
 static int printk_time = 0;
 #endif
-module_param(printk_time, int, S_IRUGO | S_IWUSR);
-
-static int __init printk_time_setup(char *str)
-{
-	if (*str)
-		return 0;
-	printk_time = 1;
-	return 1;
-}
-
-__setup("time", printk_time_setup);
+module_param_named(time, printk_time, bool, S_IRUGO | S_IWUSR);
 
 __attribute__((weak)) unsigned long long printk_clock(void)
 {

Re: [PATCH] use printk.time option, drop time/notime

[Andrew Morton]

On Thu, 24 May 2007 17:15:35 -0700
Randy Dunlap <randy.dunlap@oracle.com> wrote:

> 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".

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.

Re: [PATCH] use printk.time option, drop time/notime

[Jan Engelhardt]

On May 29 2007 13:07, Andrew Morton wrote:
>
>> 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".
>
>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
-- 

[PATCH v3] add printk.time option, deprecate 'time'

[Randy Dunlap]

On Tue, 29 May 2007 13:07:05 -0700 Andrew Morton wrote:

> 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.

[$ 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']
 
 	tipar.timeout=	[HW,PPT]
 			Set communications timeout in tenths of a second
--- linux-2622-rc3.orig/kernel/printk.c
+++ linux-2622-rc3/kernel/printk.c
@@ -449,13 +449,16 @@ static int printk_time = 1;
 #else
 static int printk_time = 0;
 #endif
-module_param(printk_time, int, S_IRUGO | S_IWUSR);
+module_param_named(time, printk_time, bool, S_IRUGO | S_IWUSR);
 
 static int __init printk_time_setup(char *str)
 {
 	if (*str)
 		return 0;
 	printk_time = 1;
+	printk(KERN_NOTICE "The 'time' option is deprecated and "
+		"is scheduled for removal in early 2008\n");
+	printk(KERN_NOTICE "Use 'printk.time=<value>' instead\n");
 	return 1;
 }
 
--- linux-2622-rc3.orig/Documentation/feature-removal-schedule.txt
+++ linux-2622-rc3/Documentation/feature-removal-schedule.txt
@@ -346,3 +346,10 @@ Who:  Tejun Heo <htejun@gmail.com>
 
 ---------------------------
 
+What:	'time' kernel boot parameter
+When:	January 2008
+Why:	replaced by 'printk.time=<value>' so that printk timestamps can be
+	enabled or disabled as needed
+Who:	Randy Dunlap <randy.dunlap@oracle.com>
+
+---------------------------