Skip to contentSkip to conversation

preset loops_per_jiffy for faster booting

PATCH In review Tim Bird 8 participants 29 messages Jul 9, 2004 – Jul 13, 2004 Documentation

lore
Share & tools
lore

Tim Bird

author
#0 lore raw

PATCH 1/1 preset loops_per_jiffy for faster booting

Here is a patch which allows developers or users to preset the value for loops per jiffy. This avoids the overhead of performing the calibration at boot time. This saves about 250 milliseconds on many non-x86 architectures, and about 25 milliseconds on x86. (The amount of time saved depends on the value of HZ, and NOT on the processor speed.)

Besides allowing a value to be compiled in, the patch also allows the value to be specified on the kernel command line with the syntax: "lpj=xxx" This is available whether the code is configured with the option or not.

Finally, this code adds a new FASTBOOT menu to the kernel config system, where we (CE Linux Forum developers) would like to add this and other config options which can be used to reduce kernel bootup time.

This technique (of disabling calibration for loops_per_jiffy) is used by many embedded developers for reducing bootup time.

More commentary on this issue is available at:
http://tree.celinuxforum.org/pubwiki/moin.cgi/PresetLPJ

Comments are welcome.

Thanks.

=============================
Tim Bird
Bootup Time Working Group Chair, CE Linux Forum
Senior Staff Engineer, Sony Electronics
E-mail: [email protected]
=============================

preset-lpj.patch:

Signed-off-by Tim Bird <[email protected]> for CELF

---

 Kconfig |   35 ++++++++++++++++++++++++++++
 main.c  |   78 +++++++++++++++++++++++++++++++++++++++++-----------------------
 2 files changed, 86 insertions(+), 27 deletions(-)
init/Kconfig +35 -0
diff -ruN -x CVS -x '.#*' -X dontdiff tag_LINUX_2_6_7/init/Kconfig branch_PRESET_LPJ/init/Kconfig
@@ -218,6 +218,41 @@
 	  This option enables access to kernel configuration file and build
 	  information through /proc/config.gz.
 
+menuconfig FASTBOOT
+	bool "Fast boot options"
+	help
+	  Say Y here to enable faster booting of the Linux kernel.  If you
+	  say Y here, you may be asked to provide hardcoded values for some
+	  parameters that the kernel usually determines automatically.
+
+	  If unsure, say N.
+
+config USE_PRESET_LPJ
+	bool "Use preset loops_per_jiffy" if FASTBOOT
+	help
+	  Say Y here to use a preset value for loops_per_jiffy.  This
+	  is a value used internally in the kernel for busywait delays.
+	  Calculating this value at boot time can take up to 250 ms.
+	  Saying Y here, and specifying the value (next) will save
+	  this time during boot up.
+
+	  If unsure, say N.
+
+config PRESET_LPJ
+	int "Preset loops_per_jiffy" if USE_PRESET_LPJ
+	help
+	  This is the number of loops used by delay() to achieve a single
+	  jiffy of delay inside the kernel.  It is roughly BogoMips * 5000.
+	  To determine the correct value for your kernel, first turn off
+	  the fast booting option, compile and boot the kernel on your
+	  target hardware, then see what value is printed during the
+	  kernel boot.  Use that value here.
+
+	  If unsure, don't enable the "Use preset loops_per_jiffy" option.
+	  An incorrect value will cause delays in the kernel to be
+	  incorrect.  Although unlikely, in the extreme case this might
+	  damage your hardware.
+
 
 menuconfig EMBEDDED
 	bool "Configure standard kernel features (for small systems)"
init/main.c +51 -27

97 lines not on this page · raw message

#1 lore raw
Quoted 4 lines · Tim Bird

Finally, this code adds a new FASTBOOT menu to the kernel config system, where we (CE Linux Forum developers) would like to add this and other config options which can be used to reduce kernel bootup time.

<snip>

init/Kconfig line 226

Quoted 6 lines
init/Kconfig +6 -0
+menuconfig FASTBOOT
+	bool "Fast boot options"
+	help
+	  Say Y here to enable faster booting of the Linux kernel.  If you
+	  say Y here, you may be asked to provide hardcoded values for some
+	  parameters that the kernel usually determines automatically.

If FASTBOOT is intended to be merely a container for individual related options, this help text seems misleading. FASTBOOT=y alone will have no effect on the kernel. It's just a gateway to other more specific options. Something like this may be better:

Say Y here to select among various options that can decrease kernel boot time. These options commonly involve providing hardcoded values for some parameters that the kernel usually determines automatically.

This option is useful primarily on embedded systems.

If unsure, say N.

init/Kconfig line 249

Quoted 9 lines
init/Kconfig +9 -0
+config PRESET_LPJ
+	int "Preset loops_per_jiffy" if USE_PRESET_LPJ
+	help
+	  This is the number of loops used by delay() to achieve a single
+	  jiffy of delay inside the kernel.  It is roughly BogoMips * 5000.
+	  To determine the correct value for your kernel, first turn off
+	  the fast booting option, compile and boot the kernel on your
+	  target hardware, then see what value is printed during the
+	  kernel boot.  Use that value here.

Perhaps mention the new lpj= parameter is an alternative:

loops_per_jiffy can also be set via the "lpj=" kernel command line parameter.

--Adam

#2 lore raw
In reply to Tim Bird
Quoted 6 lines · Tim Bird

Here is a patch which allows developers or users to preset the value for loops per jiffy. This avoids the overhead of performing the calibration at boot time. This saves about 250 milliseconds on many non-x86 architectures, and about 25 milliseconds on x86. (The amount of time saved depends on the value of HZ, and NOT on the processor speed.)

Here's an alternate patch (compile tested only) which is slightly simpler, slightly more flexible, and fixes a small bug in the original.

The simplification centers around removing USE_PRESET_LPJ and interpeting a preset value of 0 as a signal to autodetect. This eliminates ifdefs in the code and avoids giving magic significance to the loops_per_jiffy LSb. Additionally, the user can always disable the preset by using "lpj=0" which would allow booting a kernel that crashes due to a bogus preset. The only problem I can think of with this approach is if there is a system out there so slow that lpj=0 is actually a valid setting.

The final change is to fix a small bug in the original patch:
loops_per_jiffy was no longer initialized each time calibrate_delay() was invoked. This is potentially an issue on SMP systems since calibrate_delay() will be invoked for each CPU. One related thing to keep in mind is that on an SMP system, using an lpj preset will result in the same lpj setting on each CPU. On sane systems this shouldn't be a problem, but if there's a machine out there with unequal CPUs it will be a problem. Perhaps this is worth mentioning in the help text as well.

While we're on the topic: Should FASTBOOT perhaps depend on EMBEDDED? I can imagine a user with a massively MP system perhaps finding this option useful, so maybe not.

--Adam

init/main.c renamed from init/main.c.orig +49 -32
@@ -167,6 +167,15 @@
 	return 0;
 }
 
+static unsigned long preset_lpj = CONFIG_PRESET_LPJ;
+static int __init lpj_setup(char *str)
+{
+	preset_lpj = simple_strtoul(str,NULL,10);
+	return 1;
+}
+
+__setup("lpj=", lpj_setup);
+
 /* this should be approx 2 Bo*oMips to start (note initial shift), and will
    still work even if initially too large, it will just take slightly longer */
 unsigned long loops_per_jiffy = (1<<12);

@@ -183,40 +192,48 @@
 	unsigned long ticks, loopbit;
 	int lps_precision = LPS_PREC;
 
-	loops_per_jiffy = (1<<12);
+	if (preset_lpj) {
+		loops_per_jiffy = preset_lpj;
+		printk("Calibrating delay loop (skipped)... \n");
+	} else {
+		loops_per_jiffy = (1<<12);
+
+		printk("Calibrating delay loop... ");
+		while ((loops_per_jiffy <<= 1) != 0) {
+			/* wait for "start of" clock tick */
+			ticks = jiffies;
+			while (ticks == jiffies)
+				/* nothing */;
+			/* Go .. */
+			ticks = jiffies;
+			__delay(loops_per_jiffy);
+			ticks = jiffies - ticks;
+			if (ticks)
+				break;
+		}
+
+	/* Do a binary approximation to get loops_per_jiffy set to equal one clock
+	   (up to lps_precision bits) */
+		loops_per_jiffy >>= 1;
+		loopbit = loops_per_jiffy;
+		while ( lps_precision-- && (loopbit >>= 1) ) {
+			loops_per_jiffy |= loopbit;
+			ticks = jiffies;
+			while (ticks == jiffies);
+			ticks = jiffies;
+			__delay(loops_per_jiffy);
+			if (jiffies != ticks)	/* longer than 1 tick */
+				loops_per_jiffy &= ~loopbit;
+		}
+
+	/* Round the value and print it */	
+		printk("%lu.%02lu BogoMIPS\n",
+			loops_per_jiffy/(500000/HZ),
+			(loops_per_jiffy/(5000/HZ)) % 100);
+		printk("Set 'Preset loops_per_jiffy'=%lu for preset lpj.\n",
+			loops_per_jiffy);
 
-	printk("Calibrating delay loop... ");
-	while ((loops_per_jiffy <<= 1) != 0) {
-		/* wait for "start of" clock tick */
-		ticks = jiffies;
-		while (ticks == jiffies)
-			/* nothing */;
-		/* Go .. */
-		ticks = jiffies;
-		__delay(loops_per_jiffy);
-		ticks = jiffies - ticks;
-		if (ticks)
-			break;
 	}
-
-/* Do a binary approximation to get loops_per_jiffy set to equal one clock
-   (up to lps_precision bits) */
-	loops_per_jiffy >>= 1;
-	loopbit = loops_per_jiffy;
-	while ( lps_precision-- && (loopbit >>= 1) ) {
-		loops_per_jiffy |= loopbit;
-		ticks = jiffies;
-		while (ticks == jiffies);
-		ticks = jiffies;
-		__delay(loops_per_jiffy);
-		if (jiffies != ticks)	/* longer than 1 tick */
-			loops_per_jiffy &= ~loopbit;
-	}
-
-/* Round the value and print it */	
-	printk("%lu.%02lu BogoMIPS\n",
-		loops_per_jiffy/(500000/HZ),
-		(loops_per_jiffy/(5000/HZ)) % 100);
 }
 
 static int __init debug_kernel(char *str)
init/Kconfig renamed from init/Kconfig.orig +34 -0
@@ -218,6 +218,40 @@
 	  This option enables access to kernel configuration file and build
 	  information through /proc/config.gz.
 
+menuconfig FASTBOOT
+	bool "Fast boot options"
+	help
+	  Say Y here to select among various options that can decrease
+	  kernel boot time. These options commonly involve providing
+	  hardcoded values for some parameters that the kernel usually
+	  determines automatically.
+	
+	  This option is useful primarily on embedded systems.
+	
+	  If unsure, say N.
+
+config PRESET_LPJ
+	int "Preset loops_per_jiffy" if FASTBOOT
+	default 0
+	help
+	  This is the number of loops used by delay() to achieve a single
+	  jiffy of delay inside the kernel.  It is normally calculated at
+	  boot time, but that calculation can take up to 250 ms per CPU.
+	  Specifying a constant value here will eliminate that delay.
+	  
+	  loops_per_jiffy is roughly BogoMips * 5000. To determine the correct
+	  value for your kernel, first turn off the fast booting option,
+	  compile and boot the kernel on your target hardware, then see what
+	  value is printed during the kernel boot.  Use that value here.
+
+	  A value of 0 results in the normal autodetect behavior at boot.
+	  
+	  The kernel command line parameter "lpj=" can be used to override
+	  the value configured here.
+
+	  If unsure, set this to 0.  An incorrect value will cause delays in
+	  the kernel to be incorrect.  Although unlikely, in the extreme case
+	  this might damage your hardware.
 
 menuconfig EMBEDDED
 	bool "Configure standard kernel features (for small systems)"
author
#3 lore raw
Quoted 2 lines · Adam Kropelin

Here's an alternate patch (compile tested only) which is slightly simpler, slightly more flexible, and fixes a small bug in the original.

These are great improvements. Thanks.

Quoted 2 lines

The simplification centers around removing USE_PRESET_LPJ and
interpeting a preset value of 0 as a signal to autodetect. This eliminates ifdefs in the code and avoids giving magic significance to the loops_per_jiffy LSb.

Yeah. When I originally wrote it, I thought using the LSb was cute, and avoided an extra variable. Your way is simpler and provides the extra feature of disabling the preset from the command line. This, and the elimination of ifdefs is quite nice.

Quoted 5 lines

Additionally, the user can always disable
the preset by using "lpj=0" which would allow booting a kernel that crashes due to a bogus preset. The only problem I can think of with this approach is if there is a system out there so slow that lpj=0 is actually a valid setting.

It's hard to imagine this case, but that would merely result in a calibration, right? For a machine that slow, calibrating the delay is the least of their worries. :-)

Quoted 9 lines

The final change is to fix a small bug in the original patch:
loops_per_jiffy was no longer initialized each time calibrate_delay() was invoked. This is potentially an issue on SMP systems since calibrate_delay() will be invoked for each CPU. One related thing to keep in mind is that on an SMP system, using an lpj preset will result in the same lpj setting on each CPU. On sane systems this shouldn't be a problem, but if there's a machine out there with unequal CPUs it will be a problem. Perhaps this is worth mentioning in the help text as well.

I hadn't considered this. (Too much "embedded" on the brain.) By help text, do you mean the config text, or something on the wiki page, or some other file (in Documentation?).

On the subject of help text, is there a Documentation file I should modify or someone I should notify about the addition of a new kernel command line option?

Quoted 4 lines

While we're on the topic: Should FASTBOOT perhaps depend on EMBEDDED? I can imagine a user with a massively MP system perhaps finding this option useful, so maybe not.

I had it there originally, then changed my mind. I know some server guys are interested in fastboot. This particular change might not be that interesting, but some of the other changes we are thinking of might not be specific to just embedded.

I will do some runtime testing on your patch, but I probably won't be able to report back until Monday.

Thanks very much!
-- Tim

=============================
Tim Bird
Architecture Group Co-Chair, CE Linux Forum
Senior Staff Engineer, Sony Electronics
E-mail: [email protected]
=============================

#4 lore raw
Quoted 7 lines · Tim Bird
Quoted 3 lines · Adam Kropelin

crashes due to a bogus preset. The only problem I can think of with this approach is if there is a system out there so slow that lpj=0 is actually a valid setting.

It's hard to imagine this case, but that would merely result in a calibration, right? For a machine that slow, calibrating the delay is the least of their worries. :-)

Good point. ;)

Quoted 4 lines
Quoted 1 line

be a problem. Perhaps this is worth mentioning in the help text as well.

I hadn't considered this. (Too much "embedded" on the brain.) By help text, do you mean the config text, or something on the wiki page, or some other file (in Documentation?).

I meant the config text.

Quoted 3 lines

On the subject of help text, is there a Documentation file I should modify or someone I should notify about the addition of a new kernel command line option?

Indeed, there is: Documentation/kernel-parameters.txt. You're right; we should update it.

Quoted 8 lines
Quoted 3 lines

While we're on the topic: Should FASTBOOT perhaps depend on EMBEDDED? I can imagine a user with a massively MP system perhaps finding this option useful, so maybe not.

I had it there originally, then changed my mind. I know some server guys are interested in fastboot. This particular change might not be that interesting, but some of the other changes we are thinking of might not be specific to just embedded.

Ok, let's leave it as-is, then.

Quoted 2 lines

I will do some runtime testing on your patch, but I probably won't be able to report back until Monday.

I've made an updated patch below that incorporates the items we just discussed as well as a couple of things Andreas Dilger sent to me privately.

Quoted 1 line

Thanks very much!

No problem. This was a nice little self-contained project to play with this evening.

Signed-off-by Adam Kropelin <[email protected]>
init/main.c renamed from init/main.c.orig +50 -32
@@ -167,6 +167,15 @@
 	return 0;
 }
 
+static unsigned long preset_lpj = CONFIG_PRESET_LPJ;
+static int __init lpj_setup(char *str)
+{
+	preset_lpj = simple_strtoul(str,NULL,0);
+	return 1;
+}
+
+__setup("lpj=", lpj_setup);
+
 /* this should be approx 2 Bo*oMips to start (note initial shift), and will
    still work even if initially too large, it will just take slightly longer */
 unsigned long loops_per_jiffy = (1<<12);

@@ -183,40 +192,49 @@
 	unsigned long ticks, loopbit;
 	int lps_precision = LPS_PREC;
 
-	loops_per_jiffy = (1<<12);
+	if (preset_lpj) {
+		loops_per_jiffy = preset_lpj;
+		printk("Calibrating delay loop (skipped)... ");
+	} else {
+		loops_per_jiffy = (1<<12);
+
+		printk("Calibrating delay loop... ");
+		while ((loops_per_jiffy <<= 1) != 0) {
+			/* wait for "start of" clock tick */
+			ticks = jiffies;
+			while (ticks == jiffies)
+				/* nothing */;
+			/* Go .. */
+			ticks = jiffies;
+			__delay(loops_per_jiffy);
+			ticks = jiffies - ticks;
+			if (ticks)
+				break;
+		}
+
+		/* Do a binary approximation to get loops_per_jiffy set to
+		   equal one clock (up to lps_precision bits) */
+		loops_per_jiffy >>= 1;
+		loopbit = loops_per_jiffy;
+		while ( lps_precision-- && (loopbit >>= 1) ) {
+			loops_per_jiffy |= loopbit;
+			ticks = jiffies;
+			while (ticks == jiffies)
+				/* nothing */;
+			ticks = jiffies;
+			__delay(loops_per_jiffy);
+			if (jiffies != ticks)	/* longer than 1 tick */
+				loops_per_jiffy &= ~loopbit;
+		}
+
+		/* Round the value and print it */	
+		printk("%lu.%02lu BogoMIPS\n",
+			loops_per_jiffy/(500000/HZ),
+			(loops_per_jiffy/(5000/HZ)) % 100);
+		printk("Set 'Preset loops_per_jiffy'=%lu for preset lpj.\n",
+			loops_per_jiffy);
 
-	printk("Calibrating delay loop... ");
-	while ((loops_per_jiffy <<= 1) != 0) {
-		/* wait for "start of" clock tick */
-		ticks = jiffies;
-		while (ticks == jiffies)
-			/* nothing */;
-		/* Go .. */
-		ticks = jiffies;
-		__delay(loops_per_jiffy);
-		ticks = jiffies - ticks;
-		if (ticks)
-			break;
 	}
-
-/* Do a binary approximation to get loops_per_jiffy set to equal one clock
-   (up to lps_precision bits) */
-	loops_per_jiffy >>= 1;
-	loopbit = loops_per_jiffy;
-	while ( lps_precision-- && (loopbit >>= 1) ) {
-		loops_per_jiffy |= loopbit;
-		ticks = jiffies;
-		while (ticks == jiffies);
-		ticks = jiffies;
-		__delay(loops_per_jiffy);
-		if (jiffies != ticks)	/* longer than 1 tick */
-			loops_per_jiffy &= ~loopbit;
-	}
-
-/* Round the value and print it */	
-	printk("%lu.%02lu BogoMIPS\n",
-		loops_per_jiffy/(500000/HZ),
-		(loops_per_jiffy/(5000/HZ)) % 100);
 }
 
 static int __init debug_kernel(char *str)
init/Kconfig renamed from init/Kconfig.orig +38 -0
@@ -218,6 +218,44 @@
 	  This option enables access to kernel configuration file and build
 	  information through /proc/config.gz.
 
+menuconfig FASTBOOT
+	bool "Fast boot options"
+	help
+	  Say Y here to select among various options that can decrease
+	  kernel boot time. These options commonly involve providing
+	  hardcoded values for some parameters that the kernel usually
+	  determines automatically.
+	
+	  This option is useful primarily on embedded systems.
+	
+	  If unsure, say N.
+
+config PRESET_LPJ
+	int "Preset loops_per_jiffy" if FASTBOOT
+	default 0
+	help
+	  This is the number of loops used by delay() to achieve a single
+	  jiffy of delay inside the kernel.  It is normally calculated at
+	  boot time, but that calculation can take up to 250 ms per CPU.
+	  Specifying a constant value here will eliminate that delay.
+	  
+	  loops_per_jiffy is roughly BogoMips * 5000. To determine the correct
+	  value for your kernel, first turn off the fast booting option,
+	  compile and boot the kernel on your target hardware, then see what
+	  value is printed during the kernel boot.  Use that value here.
+
+	  A value of 0 results in the normal autodetect behavior at boot.
+	  
+	  The kernel command line parameter "lpj=" can be used to override
+	  the value configured here.
+	  
+	  Note that on SMP systems the preset will be applied to all CPUs
+	  which will cause problems if for some reason your CPUs need
+	  significantly divergent settings.
+
+	  If unsure, set this to 0. An incorrect value will cause delays in
+	  the kernel to be incorrect.  Although unlikely, in the extreme case
+	  this might damage your hardware.
 
 menuconfig EMBEDDED
 	bool "Configure standard kernel features (for small systems)"
Documentation/kernel-parameters.txt renamed from Documentation/kernel-parameters.txt.orig +6 -0
@@ -576,6 +576,12 @@
 				so, the driver will manage that printer.
 				See also header of drivers/char/lp.c.
 
+	lpj=n		[KNL]
+			Sets loops_per_jiffy to given constant, thus avoiding
+			time-consuming boot-time autodetection.
+			0 enables autodetection (default). See Kconfig help text
+			for PRESET_LPJ for details.
+
 	ltpc=		[NET]
 			Format: <io>,<irq>,<dma>
#5 lore raw

init/main.c line 197

Quoted 3 lines · Adam Kropelin
init/main.c renamed from init/main.c.orig +3 -0
+	if (preset_lpj) {
+		loops_per_jiffy = preset_lpj;
+		printk("Calibrating delay loop (skipped)... ");

Suggest a "\n" at the end of that. Maybe add the precomputed value to help bring incorrect presets to someone's attention, something like:

+ printk("BogoMIPS preset to %lu.%02lu\n",
+ loops_per_jiffy/(500000/HZ),
+ (loops_per_jiffy/(5000/HZ)) % 100);

Quoted 3 lines

+ If unsure, set this to 0. An incorrect value will cause delays in
+ the kernel to be incorrect. Although unlikely, in the extreme case
+ this might damage your hardware.

I suppose it may result in unpredictable I/O errors, in case we want to warn against that.

Signature · 2 lines
Todd Poynor
MontaVista Software
#6 lore raw
Quoted 7 lines · Todd Poynor

init/main.c line 197

Quoted 3 lines · Adam Kropelin
init/main.c renamed from init/main.c.orig +3 -0
+	if (preset_lpj) {
+		loops_per_jiffy = preset_lpj;
+		printk("Calibrating delay loop (skipped)... ");

Suggest a "\n" at the end of that.

Indeed. I propogated that bug from the original patch. I'll fix it.

init/main.c line 233

Quoted 6 lines

Maybe add the precomputed value to
help bring incorrect presets to someone's attention, something like:

init/main.c renamed from init/main.c.orig +3 -0
+		printk("BogoMIPS preset to %lu.%02lu\n",
+			loops_per_jiffy/(500000/HZ),
+			(loops_per_jiffy/(5000/HZ)) % 100);

Will do.

Quoted 6 lines
Quoted 3 lines

+ If unsure, set this to 0. An incorrect value will cause delays in
+ the kernel to be incorrect. Although unlikely, in the extreme case
+ this might damage your hardware.

I suppose it may result in unpredictable I/O errors, in case we want to warn against that.

Easy enough to throw it in.

Another patch is forthcoming.

--Adam

#7 lore raw
In reply to Adam Kropelin
Quoted 6 lines · Adam Kropelin
Quoted 1 line · Tim Bird

Adam Kropelin wrote:

init/Kconfig renamed from init/Kconfig.orig +3 -0
+		printk("Set 'Preset loops_per_jiffy'=%lu for preset lpj.\n",
+			loops_per_jiffy);
 
+config PRESET_LPJ

init/Kconfig line 243

Quoted 10 lines
init/Kconfig renamed from init/Kconfig.orig +10 -0
+	int "Preset loops_per_jiffy" if FASTBOOT
+	default 0
+	help
+	  This is the number of loops used by delay() to achieve a single
+	  jiffy of delay inside the kernel.  It is normally calculated at
+	  boot time, but that calculation can take up to 250 ms per CPU.
+	  Specifying a constant value here will eliminate that delay.
+
+	  loops_per_jiffy is roughly BogoMips * 5000. To determine the correct
+	  value for your kernel, first turn off the fast booting option,

                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

init/Kconfig line 245

Quoted 2 lines
init/Kconfig renamed from init/Kconfig.orig +2 -0
+	  compile and boot the kernel on your target hardware, then see what
+	  value is printed during the kernel boot.  Use that value here.

This is no longer true, it will always print the value if lpj=0.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

#8 lore raw
Quoted 18 lines · Geert Uytterhoeven
Quoted 6 lines · Adam Kropelin
Quoted 1 line · Tim Bird

Adam Kropelin wrote:

init/Kconfig renamed from init/Kconfig.orig +3 -0
+		printk("Set 'Preset loops_per_jiffy'=%lu for preset lpj.\n",
+			loops_per_jiffy);
 
+config PRESET_LPJ

init/Kconfig line 243

Quoted 10 lines
init/Kconfig renamed from init/Kconfig.orig +10 -0
+	int "Preset loops_per_jiffy" if FASTBOOT
+	default 0
+	help
+	  This is the number of loops used by delay() to achieve a single
+	  jiffy of delay inside the kernel.  It is normally calculated at
+	  boot time, but that calculation can take up to 250 ms per CPU.
+	  Specifying a constant value here will eliminate that delay.
+
+	  loops_per_jiffy is roughly BogoMips * 5000. To determine the correct
+	  value for your kernel, first turn off the fast booting option,

                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Quoted 4 lines

init/Kconfig line 245

Quoted 2 lines
init/Kconfig renamed from init/Kconfig.orig +2 -0
+	  compile and boot the kernel on your target hardware, then see what
+	  value is printed during the kernel boot.  Use that value here.

This is no longer true, it will always print the value if lpj=0.

True enough. I'll fix that along with the things Todd noticed.

--Adam

#9 lore raw
In reply to Tim Bird

Here's an updated patch which incorporates suggestions from Todd Poynor and Geert Uytterhoeven.

Signed-off-by Adam Kropelin <[email protected]>
init/main.c renamed from init/main.c.orig +53 -32
@@ -167,6 +167,15 @@
 	return 0;
 }
 
+static unsigned long preset_lpj = CONFIG_PRESET_LPJ;
+static int __init lpj_setup(char *str)
+{
+	preset_lpj = simple_strtoul(str,NULL,0);
+	return 1;
+}
+
+__setup("lpj=", lpj_setup);
+
 /* this should be approx 2 Bo*oMips to start (note initial shift), and will
    still work even if initially too large, it will just take slightly longer */
 unsigned long loops_per_jiffy = (1<<12);

@@ -183,40 +192,52 @@
 	unsigned long ticks, loopbit;
 	int lps_precision = LPS_PREC;
 
-	loops_per_jiffy = (1<<12);
-
-	printk("Calibrating delay loop... ");
-	while ((loops_per_jiffy <<= 1) != 0) {
-		/* wait for "start of" clock tick */
-		ticks = jiffies;
-		while (ticks == jiffies)
-			/* nothing */;
-		/* Go .. */
-		ticks = jiffies;
-		__delay(loops_per_jiffy);
-		ticks = jiffies - ticks;
-		if (ticks)
-			break;
-	}
-
-/* Do a binary approximation to get loops_per_jiffy set to equal one clock
-   (up to lps_precision bits) */
-	loops_per_jiffy >>= 1;
-	loopbit = loops_per_jiffy;
-	while ( lps_precision-- && (loopbit >>= 1) ) {
-		loops_per_jiffy |= loopbit;
-		ticks = jiffies;
-		while (ticks == jiffies);
-		ticks = jiffies;
-		__delay(loops_per_jiffy);
-		if (jiffies != ticks)	/* longer than 1 tick */
-			loops_per_jiffy &= ~loopbit;
+	if (preset_lpj) {
+		loops_per_jiffy = preset_lpj;
+		printk("Calibrating delay loop (skipped)... "
+			"%lu.%02lu BogoMIPS preset\n",
+			loops_per_jiffy/(500000/HZ),
+			(loops_per_jiffy/(5000/HZ)) % 100);
+	} else {
+		loops_per_jiffy = (1<<12);
+
+		printk("Calibrating delay loop... ");
+		while ((loops_per_jiffy <<= 1) != 0) {
+			/* wait for "start of" clock tick */
+			ticks = jiffies;
+			while (ticks == jiffies)
+				/* nothing */;
+			/* Go .. */
+			ticks = jiffies;
+			__delay(loops_per_jiffy);
+			ticks = jiffies - ticks;
+			if (ticks)
+				break;
+		}
+
+		/* Do a binary approximation to get loops_per_jiffy set to
+		   equal one clock (up to lps_precision bits) */
+		loops_per_jiffy >>= 1;
+		loopbit = loops_per_jiffy;
+		while ( lps_precision-- && (loopbit >>= 1) ) {
+			loops_per_jiffy |= loopbit;
+			ticks = jiffies;
+			while (ticks == jiffies)
+				/* nothing */;
+			ticks = jiffies;
+			__delay(loops_per_jiffy);
+			if (jiffies != ticks)	/* longer than 1 tick */
+				loops_per_jiffy &= ~loopbit;
+		}
+
+	    /* Round the value and print it */	
+	    printk("%lu.%02lu BogoMIPS\n",
+		    loops_per_jiffy/(500000/HZ),
+		    (loops_per_jiffy/(5000/HZ)) % 100);
+	    printk("Set 'Preset loops_per_jiffy'=%lu for preset lpj.\n",
+		    loops_per_jiffy);
 	}
 
-/* Round the value and print it */	
-	printk("%lu.%02lu BogoMIPS\n",
-		loops_per_jiffy/(500000/HZ),
-		(loops_per_jiffy/(5000/HZ)) % 100);
 }
 
 static int __init debug_kernel(char *str)
init/Kconfig renamed from init/Kconfig.orig +39 -0
@@ -218,6 +218,45 @@
 	  This option enables access to kernel configuration file and build
 	  information through /proc/config.gz.
 
+menuconfig FASTBOOT
+	bool "Fast boot options"
+	help
+	  Say Y here to select among various options that can decrease
+	  kernel boot time. These options commonly involve providing
+	  hardcoded values for some parameters that the kernel usually
+	  determines automatically.
+	
+	  This option is useful primarily on embedded systems.
+	
+	  If unsure, say N.
+
+config PRESET_LPJ
+	int "Preset loops_per_jiffy" if FASTBOOT
+	default 0
+	help
+	  This is the number of loops used by delay() to achieve a single
+	  jiffy of delay inside the kernel.  It is normally calculated at
+	  boot time, but that calculation can take up to 250 ms per CPU.
+	  Specifying a constant value here will eliminate that delay.
+
+	  A value of 0 results in the normal autodetect behavior.
+
+	  loops_per_jiffy is roughly BogoMips * 5000. To determine the correct
+	  value for your kernel, first set this option to 0, compile and boot
+	  the kernel on your target hardware, then see what value is printed
+	  during the kernel boot.  Use that value here.
+
+	  The kernel command line parameter "lpj=" can be used to override
+	  the value configured here.
+
+	  Note that on SMP systems the preset will be applied to all CPUs
+	  which will cause problems if for some reason your CPUs need
+	  significantly divergent settings.
+
+	  If unsure, set this to 0. An incorrect value will cause delays in
+	  the kernel to be wrong, leading to unpredictable I/O errors and
+	  other breakage.  Although unlikely, in the extreme case this might
+	  damage your hardware.
 
 menuconfig EMBEDDED
 	bool "Configure standard kernel features (for small systems)"
Documentation/kernel-parameters.txt renamed from Documentation/kernel-parameters.txt.orig +6 -0
@@ -576,6 +576,12 @@
 				so, the driver will manage that printer.
 				See also header of drivers/char/lp.c.
 
+	lpj=n		[KNL]
+			Sets loops_per_jiffy to given constant, thus avoiding
+			time-consuming boot-time autodetection.
+			0 enables autodetection (default). See Kconfig help text
+			for PRESET_LPJ for details.
+
 	ltpc=		[NET]
 			Format: <io>,<irq>,<dma>
#10 lore raw
Quoted 2 lines · Adam Kropelin

Here's an updated patch which incorporates suggestions from Todd Poynor and Geert Uytterhoeven.

<snip>

Review

Quoted 7 lines
(quoted diff) +6 -0
+	    /* Round the value and print it */	
+	    printk("%lu.%02lu BogoMIPS\n",
+		    loops_per_jiffy/(500000/HZ),
+		    (loops_per_jiffy/(5000/HZ)) % 100);
+	    printk("Set 'Preset loops_per_jiffy'=%lu for preset lpj.\n",
+		    loops_per_jiffy);
 	}

Argh. Here's one with the right tabbing.

Signed-off-by Adam Kropelin <[email protected]>
init/main.c renamed from init/main.c.orig +53 -32
@@ -167,6 +167,15 @@
 	return 0;
 }
 
+static unsigned long preset_lpj = CONFIG_PRESET_LPJ;
+static int __init lpj_setup(char *str)
+{
+	preset_lpj = simple_strtoul(str,NULL,0);
+	return 1;
+}
+
+__setup("lpj=", lpj_setup);
+
 /* this should be approx 2 Bo*oMips to start (note initial shift), and will
    still work even if initially too large, it will just take slightly longer */
 unsigned long loops_per_jiffy = (1<<12);

@@ -183,40 +192,52 @@
 	unsigned long ticks, loopbit;
 	int lps_precision = LPS_PREC;
 
-	loops_per_jiffy = (1<<12);
-
-	printk("Calibrating delay loop... ");
-	while ((loops_per_jiffy <<= 1) != 0) {
-		/* wait for "start of" clock tick */
-		ticks = jiffies;
-		while (ticks == jiffies)
-			/* nothing */;
-		/* Go .. */
-		ticks = jiffies;
-		__delay(loops_per_jiffy);
-		ticks = jiffies - ticks;
-		if (ticks)
-			break;
-	}
-
-/* Do a binary approximation to get loops_per_jiffy set to equal one clock
-   (up to lps_precision bits) */
-	loops_per_jiffy >>= 1;
-	loopbit = loops_per_jiffy;
-	while ( lps_precision-- && (loopbit >>= 1) ) {
-		loops_per_jiffy |= loopbit;
-		ticks = jiffies;
-		while (ticks == jiffies);
-		ticks = jiffies;
-		__delay(loops_per_jiffy);
-		if (jiffies != ticks)	/* longer than 1 tick */
-			loops_per_jiffy &= ~loopbit;
+	if (preset_lpj) {
+		loops_per_jiffy = preset_lpj;
+		printk("Calibrating delay loop (skipped)... "
+			"%lu.%02lu BogoMIPS preset\n",
+			loops_per_jiffy/(500000/HZ),
+			(loops_per_jiffy/(5000/HZ)) % 100);
+	} else {
+		loops_per_jiffy = (1<<12);
+
+		printk("Calibrating delay loop... ");
+		while ((loops_per_jiffy <<= 1) != 0) {
+			/* wait for "start of" clock tick */
+			ticks = jiffies;
+			while (ticks == jiffies)
+				/* nothing */;
+			/* Go .. */
+			ticks = jiffies;
+			__delay(loops_per_jiffy);
+			ticks = jiffies - ticks;
+			if (ticks)
+				break;
+		}
+
+		/* Do a binary approximation to get loops_per_jiffy set to
+		   equal one clock (up to lps_precision bits) */
+		loops_per_jiffy >>= 1;
+		loopbit = loops_per_jiffy;
+		while ( lps_precision-- && (loopbit >>= 1) ) {
+			loops_per_jiffy |= loopbit;
+			ticks = jiffies;
+			while (ticks == jiffies)
+				/* nothing */;
+			ticks = jiffies;
+			__delay(loops_per_jiffy);
+			if (jiffies != ticks)	/* longer than 1 tick */
+				loops_per_jiffy &= ~loopbit;
+		}
+
+		/* Round the value and print it */	
+		printk("%lu.%02lu BogoMIPS\n",
+			loops_per_jiffy/(500000/HZ),
+			(loops_per_jiffy/(5000/HZ)) % 100);
+		printk("Set 'Preset loops_per_jiffy'=%lu for preset lpj.\n",
+			loops_per_jiffy);
 	}
 
-/* Round the value and print it */	
-	printk("%lu.%02lu BogoMIPS\n",
-		loops_per_jiffy/(500000/HZ),
-		(loops_per_jiffy/(5000/HZ)) % 100);
 }
 
 static int __init debug_kernel(char *str)
init/Kconfig renamed from init/Kconfig.orig +39 -0
@@ -218,6 +218,45 @@
 	  This option enables access to kernel configuration file and build
 	  information through /proc/config.gz.
 
+menuconfig FASTBOOT
+	bool "Fast boot options"
+	help
+	  Say Y here to select among various options that can decrease
+	  kernel boot time. These options commonly involve providing
+	  hardcoded values for some parameters that the kernel usually
+	  determines automatically.
+	
+	  This option is useful primarily on embedded systems.
+	
+	  If unsure, say N.
+
+config PRESET_LPJ
+	int "Preset loops_per_jiffy" if FASTBOOT
+	default 0
+	help
+	  This is the number of loops used by delay() to achieve a single
+	  jiffy of delay inside the kernel.  It is normally calculated at
+	  boot time, but that calculation can take up to 250 ms per CPU.
+	  Specifying a constant value here will eliminate that delay.
+
+	  A value of 0 results in the normal autodetect behavior.
+
+	  loops_per_jiffy is roughly BogoMips * 5000. To determine the correct
+	  value for your kernel, first set this option to 0, compile and boot
+	  the kernel on your target hardware, then see what value is printed
+	  during the kernel boot.  Use that value here.
+
+	  The kernel command line parameter "lpj=" can be used to override
+	  the value configured here.
+
+	  Note that on SMP systems the preset will be applied to all CPUs
+	  which will cause problems if for some reason your CPUs need
+	  significantly divergent settings.
+
+	  If unsure, set this to 0. An incorrect value will cause delays in
+	  the kernel to be wrong, leading to unpredictable I/O errors and
+	  other breakage.  Although unlikely, in the extreme case this might
+	  damage your hardware.
 
 menuconfig EMBEDDED
 	bool "Configure standard kernel features (for small systems)"
Documentation/kernel-parameters.txt renamed from Documentation/kernel-parameters.txt.orig +6 -0
@@ -576,6 +576,12 @@
 				so, the driver will manage that printer.
 				See also header of drivers/char/lp.c.
 
+	lpj=n		[KNL]
+			Sets loops_per_jiffy to given constant, thus avoiding
+			time-consuming boot-time autodetection.
+			0 enables autodetection (default). See Kconfig help text
+			for PRESET_LPJ for details.
+
 	ltpc=		[NET]
 			Format: <io>,<irq>,<dma>
#11 lore raw
Quoted 9 lines · Adam Kropelin

+         Note that on SMP systems the preset will be applied to all CPUs +         which will cause problems if for some reason your CPUs need +         significantly divergent settings.
+
+         If unsure, set this to 0. An incorrect value will cause delays in +         the kernel to be wrong, leading to unpredictable I/O errors and +         other breakage.  Although unlikely, in the extreme case this might +         damage your hardware.

Note that it may also not work correctly on laptops that switch frequency when working on battery/AC. Also one needs to be careful when changing timesource (pit, tsc, pm, hpet). And always look out for timer code changes in next version of kernel. Does 250 ms worth all this pain?

Signature · 1 line
Dmitry
#12 lore raw
Quoted 10 lines · Dmitry Torokhov
Quoted 8 lines · Adam Kropelin

+ Note that on SMP systems the preset will be applied to all CPUs
+ which will cause problems if for some reason your CPUs need
+ significantly divergent settings.
+
+ If unsure, set this to 0. An incorrect value will cause delays in
+ the kernel to be wrong, leading to unpredictable I/O errors and
+ other breakage. Although unlikely, in the extreme case this might
+ damage your hardware.

Quoted 4 lines

Note that it may also not work correctly on laptops that switch frequency when working on battery/AC. Also one needs to be careful when changing timesource (pit, tsc, pm, hpet). And always look out for timer code changes in next version of kernel.

Certainly many demons lurk around the corner, but embedded guys are used to that.

Quoted 1 line

Does 250 ms worth all this pain?

On a desktop box, almost certainly not. On a massive SMP machine, maybe. On an embedded system that is required to boot in a ridiculously short time, absolutely.

--Adam

#13 lore raw
Quoted 6 lines · Adam Kropelin
Quoted 2 lines

Does 250 ms worth all this pain?

On a desktop box, almost certainly not. On a massive SMP machine, maybe. On an embedded system that is required to boot in a ridiculously short time, absolutely.

Yes, it's pretty small beer, but we do recognise that although the number of development teams which use features like this is small, the number of systems is large, so the features are correspondingly more important.

One of the services which we kernel developers provide the downstream kernel users is the hosting and maintenance of their code so they don't have to carry important stuff off-stream, and when the changes are this small and simple, I don't see a problem with merging them, even if none of "us" will use the feature.

wrt this particular patch:

a) I don't see much point in making it configurable. Just add the boot option and be done with it. The few hundred bytes of extra code will be dropped from core anyway.

The main reason for this is that most people won't turn on the config option, so your new code could get accidentally broken quite easily. Plus it removes some ifdefs.

b) Coding style consistency, please:

Replace things like

		/* Do a binary approximation to get loops_per_jiffy set to
		   equal one clock (up to lps_precision bits) */

with

		/*
		 * Do a binary approximation to get loops_per_jiffy set to
		 * equal one clock (up to lps_precision bits)
		 */


	while ( lps_precision-- && (loopbit >>= 1) ) {

should be

	while (lps_precision-- && (loopbit >>= 1)) {

Yes, some of the code was already like that, but let's regularise these things as we go.

#14 lore raw
Quoted 7 lines · Andrew Morton
Quoted 6 lines · Adam Kropelin
Quoted 2 lines

Does 250 ms worth all this pain?

On a desktop box, almost certainly not. On a massive SMP machine, maybe. On an embedded system that is required to boot in a ridiculously short time, absolutely.

Andrew, thanks for the review...

Quoted 9 lines

wrt this particular patch:

a) I don't see much point in making it configurable. Just add the boot option and be done with it. The few hundred bytes of extra code will be dropped from core anyway.

The main reason for this is that most people won't turn on the config option, so your new code could get accidentally broken quite easily. Plus it removes some ifdefs.

Actually, there are no ifdefs at all (unless there are some auto-generated ones I don't know about). The only function of the config option is to allow a preset without using the command line parameter. I agree that the command line parameter alone is enough. I'm not an actual user of this code, though, so perhaps Tim or someone else can comment if the config option serves a useful purpose for them.

Quoted 1 line

b) Coding style consistency, please:

Will do. I actually purposely avoided making "gratuitous" changes to existing code, but I definitely appreciate consistency so I'll happily clean things up a bit.

Thanks for your feedback. Here's a patch implementing your suggestions.

Signed-off-by Adam Kropelin <[email protected]>
init/main.c renamed from init/main.c.orig +75 -43
@@ -167,15 +167,28 @@
 	return 0;
 }
 
-/* this should be approx 2 Bo*oMips to start (note initial shift), and will
-   still work even if initially too large, it will just take slightly longer */
+static unsigned long preset_lpj;
+static int __init lpj_setup(char *str)
+{
+	preset_lpj = simple_strtoul(str,NULL,0);
+	return 1;
+}
+
+__setup("lpj=", lpj_setup);
+
+/*
+ * This should be approx 2 Bo*oMips to start (note initial shift), and will
+ * still work even if initially too large, it will just take slightly longer
+ */
 unsigned long loops_per_jiffy = (1<<12);
 
 EXPORT_SYMBOL(loops_per_jiffy);
 
-/* This is the number of bits of precision for the loops_per_jiffy.  Each
-   bit takes on average 1.5/HZ seconds.  This (like the original) is a little
-   better than 1% */
+/*
+ * This is the number of bits of precision for the loops_per_jiffy.  Each
+ * bit takes on average 1.5/HZ seconds.  This (like the original) is a little
+ * better than 1%
+ */
 #define LPS_PREC 8
 
 void __devinit calibrate_delay(void)

@@ -183,40 +196,54 @@
 	unsigned long ticks, loopbit;
 	int lps_precision = LPS_PREC;
 
-	loops_per_jiffy = (1<<12);
+	if (preset_lpj) {
+		loops_per_jiffy = preset_lpj;
+		printk("Calibrating delay loop (skipped)... "
+			"%lu.%02lu BogoMIPS preset\n",
+			loops_per_jiffy/(500000/HZ),
+			(loops_per_jiffy/(5000/HZ)) % 100);
+	} else {
+		loops_per_jiffy = (1<<12);
+
+		printk("Calibrating delay loop... ");
+		while ((loops_per_jiffy <<= 1) != 0) {
+			/* wait for "start of" clock tick */
+			ticks = jiffies;
+			while (ticks == jiffies)
+				/* nothing */;
+			/* Go .. */
+			ticks = jiffies;
+			__delay(loops_per_jiffy);
+			ticks = jiffies - ticks;
+			if (ticks)
+				break;
+		}
 
-	printk("Calibrating delay loop... ");
-	while ((loops_per_jiffy <<= 1) != 0) {
-		/* wait for "start of" clock tick */
-		ticks = jiffies;
-		while (ticks == jiffies)
-			/* nothing */;
-		/* Go .. */
-		ticks = jiffies;
-		__delay(loops_per_jiffy);
-		ticks = jiffies - ticks;
-		if (ticks)
-			break;
-	}
+		/*
+		 * Do a binary approximation to get loops_per_jiffy set to
+		 * equal one clock (up to lps_precision bits)
+		 */
+		loops_per_jiffy >>= 1;
+		loopbit = loops_per_jiffy;
+		while (lps_precision-- && (loopbit >>= 1)) {
+			loops_per_jiffy |= loopbit;
+			ticks = jiffies;
+			while (ticks == jiffies)
+				/* nothing */;
+			ticks = jiffies;
+			__delay(loops_per_jiffy);
+			if (jiffies != ticks)	/* longer than 1 tick */
+				loops_per_jiffy &= ~loopbit;
+		}
 
-/* Do a binary approximation to get loops_per_jiffy set to equal one clock
-   (up to lps_precision bits) */
-	loops_per_jiffy >>= 1;
-	loopbit = loops_per_jiffy;
-	while ( lps_precision-- && (loopbit >>= 1) ) {
-		loops_per_jiffy |= loopbit;
-		ticks = jiffies;
-		while (ticks == jiffies);
-		ticks = jiffies;
-		__delay(loops_per_jiffy);
-		if (jiffies != ticks)	/* longer than 1 tick */
-			loops_per_jiffy &= ~loopbit;
+		/* Round the value and print it */	
+		printk("%lu.%02lu BogoMIPS\n",
+			loops_per_jiffy/(500000/HZ),
+			(loops_per_jiffy/(5000/HZ)) % 100);
+		printk("Set 'Preset loops_per_jiffy'=%lu for preset lpj.\n",
+			loops_per_jiffy);
 	}
 
-/* Round the value and print it */	
-	printk("%lu.%02lu BogoMIPS\n",
-		loops_per_jiffy/(500000/HZ),
-		(loops_per_jiffy/(5000/HZ)) % 100);
 }
 
 static int __init debug_kernel(char *str)

@@ -238,8 +265,10 @@
 __setup("debug", debug_kernel);
 __setup("quiet", quiet_kernel);
 
-/* Unknown boot options get handed to init, unless they look like
-   failed parameters */
+/*
+ * Unknown boot options get handed to init, unless they look like
+ * failed parameters
+ */
 static int __init unknown_bootoption(char *param, char *val)
 {
 	/* Change NUL term back to "=", to make "param" the whole string. */

@@ -250,8 +279,10 @@
 	if (obsolete_checksetup(param))
 		return 0;
 
-	/* Preemptive maintenance for "why didn't my mispelled command
-           line work?" */
+	/*
+	 * Preemptive maintenance for "why didn't my mispelled command
+	 * line work?"
+	 */
 	if (strchr(param, '.') && (!val || strchr(param, '.') < val)) {
 		printk(KERN_ERR "Unknown boot option `%s': ignoring\n", param);
 		return 0;

@@ -289,7 +320,8 @@
 	unsigned int i;
 
 	execute_command = str;
-	/* In case LILO is going to boot us with default command line,
+	/*
+	 * In case LILO is going to boot us with default command line,
 	 * it prepends "auto" before the whole cmdline which makes
 	 * the shell think it should execute a script with such name.
 	 * So we ignore all arguments entered _before_ init=... [MJ]

@@ -483,9 +515,9 @@
 	check_bugs();
 
 	/* 
-	 *	We count on the initial thread going ok 
-	 *	Like idlers init is an unlocked kernel thread, which will
-	 *	make syscalls (and thus be locked).
+	 * We count on the initial thread going ok 
+	 * Like idlers init is an unlocked kernel thread, which will
+	 * make syscalls (and thus be locked).
 	 */
 	init_idle(current, smp_processor_id());
Documentation/kernel-parameters.txt renamed from Documentation/kernel-parameters.txt.orig +14 -0
@@ -576,6 +576,20 @@
 				so, the driver will manage that printer.
 				See also header of drivers/char/lp.c.
 
+	lpj=n		[KNL]
+			Sets loops_per_jiffy to given constant, thus avoiding
+			time-consuming boot-time autodetection (up to 250 ms per
+			CPU). 0 enables autodetection (default). To determine
+			the correct value for your kernel, boot with normal
+			autodetection and see what value is printed. Note that
+			on SMP systems the preset will be applied to all CPUs,
+			which is likely to cause problems if your CPUs need
+			significantly divergent settings. An incorrect value
+			will cause delays in the kernel to be wrong, leading to
+			unpredictable I/O errors and other breakage. Although
+			unlikely, in the extreme case this might damage your
+			hardware.
+
 	ltpc=		[NET]
 			Format: <io>,<irq>,<dma>
#15 lore raw
Quoted 3 lines · Adam Kropelin

Actually, there are no ifdefs at all (unless there are some
auto-generated ones I don't know about).

OK, I should have looked and not guessed ;)

Quoted 1 line

Thanks for your feedback. Here's a patch implementing your suggestions.

Thanks. Tim, could you please review-n-test this?

author
#16 lore raw
Quoted 6 lines · Andrew Morton
Quoted 2 lines · Adam Kropelin

Actually, there are no ifdefs at all (unless there are some auto-generated ones I don't know about).

Quoted 7 lines

OK, I should have looked and not guessed ;)

Quoted 1 line

Thanks for your feedback. Here's a patch implementing your suggestions.

Thanks. Tim, could you please review-n-test this?

Yes. I'll give it a full run-through today.

=============================
Tim Bird
Bootup Time Working Group Chair, CE Linux Forum
Senior Staff Engineer, Sony Electronics
E-mail: [email protected]
=============================

#17 lore raw
In reply to Adam Kropelin

init/main.c line 244

Quoted 6 lines · Adam Kropelin
init/main.c renamed from init/main.c.orig +6 -0
+		/* Round the value and print it */	
+		printk("%lu.%02lu BogoMIPS\n",
+			loops_per_jiffy/(500000/HZ),
+			(loops_per_jiffy/(5000/HZ)) % 100);
+		printk("Set 'Preset loops_per_jiffy'=%lu for preset lpj.\n",
+			loops_per_jiffy);

Do we need to encourage ordinary users to turn this option on? 99% of non-embedded market is much safer with that option off...

Signature · 1 line
Dmitry
#18 lore raw
Quoted 2 lines · Dmitry Torokhov

Do we need to encourage ordinary users to turn this option on? 99% of non-embedded market is much safer with that option off...

There are other boot params that gather similar if not higher percentages. profile= is one of those.

Also, keep in mind that in a not too distant future (and indeed today for some folks already) recompiling and fine-tuning the Linux kernel for your latest gizmo will not be as foreign as your statement may make it sound.

Karim

Signature · 3 lines
Author, Speaker, Developer, Consultant
Pushing Embedded and Real-Time Linux Systems Beyond the Limits
http://www.opersys.com || [email protected] || 1-866-677-4546
#19 lore raw
Quoted 8 lines · Karim Yaghmour
Quoted 2 lines · Dmitry Torokhov

Do we need to encourage ordinary users to turn this option on? 99% of non-embedded market is much safer with that option off...

There are other boot params that gather similar if not higher percentages. profile= is one of those.

I do not see anywhere in my boot log suggestion to activate profiling... Nor I see recommendadtion to use idebus=66... But i will see suggesion to set loops_per_jiffy.

Quoted 3 lines

Also, keep in mind that in a not too distant future (and indeed today for some folks already) recompiling and fine-tuning the Linux kernel for your latest gizmo will not be as foreign as your statement may make it sound.

Use of this particular option will require active tracking of kernel timer code to ensure that lpj that was valid for kernel x.y.z is still valid for kernel x.y.z+1. I do not beleive that normal user will ever care to do that even in very distant future. Still, given that message in the boot log, many will probably try the option.

I am no longer question presence of the code in the kernel, I just don't like the message...

Signature · 1 line
Dmitry
#20 lore raw
Quoted 3 lines · Dmitry Torokhov

I am no longer question presence of the code in the kernel, I just don't like
the message...

yup, we shouldn't have the friendly message.

#21 lore raw
Quoted 6 lines · Andrew Morton
Quoted 3 lines · Dmitry Torokhov

I am no longer question presence of the code in the kernel, I just don't like
the message...

yup, we shouldn't have the friendly message.

Just add the appropriate KERN_*, so it's not displayed by default, and people who want it can look it up in syslog.

Or what about a sysfs entry people can cat?

The alternative is to cat /proc/cpuinfo and grab a calculcator...

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

#22 lore raw
Quoted 11 lines · Geert Uytterhoeven
Quoted 6 lines · Andrew Morton
Quoted 3 lines · Dmitry Torokhov

I am no longer question presence of the code in the kernel, I just don't like
the message...

yup, we shouldn't have the friendly message.

Just add the appropriate KERN_*, so it's not displayed by default, and people who want it can look it up in syslog.

Oh crap, sorry. I just worked out what the darn printk is for. Yes, it's legit. Let's just print the bogomips and loops_per_jiffy on the same line.

#23 lore raw
Quoted 8 lines · Andrew Morton
Quoted 7 lines · Geert Uytterhoeven
Quoted 5 lines · Andrew Morton
Quoted 3 lines · Dmitry Torokhov

I am no longer question presence of the code in the kernel, I just don't like
the message...

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

yup, we shouldn't have the friendly message.

Just add the appropriate KERN_*, so it's not displayed by default, and people who want it can look it up in syslog.

Oh crap, sorry. I just worked out what the darn printk is for. Yes, it's legit. Let's just print the bogomips and loops_per_jiffy on the same line.

Here's a patch. It places the relevant information on the same line as bogomips and does so without encouraging anyone to fiddle with loops_per_jiffy and screw up their kernel.

Signed-off-by Adam Kropelin <[email protected]>
init/main.c renamed from init/main.c.orig +74 -43
@@ -167,15 +167,28 @@
 	return 0;
 }
 
-/* this should be approx 2 Bo*oMips to start (note initial shift), and will
-   still work even if initially too large, it will just take slightly longer */
+static unsigned long preset_lpj;
+static int __init lpj_setup(char *str)
+{
+	preset_lpj = simple_strtoul(str,NULL,0);
+	return 1;
+}
+
+__setup("lpj=", lpj_setup);
+
+/*
+ * This should be approx 2 Bo*oMips to start (note initial shift), and will
+ * still work even if initially too large, it will just take slightly longer
+ */
 unsigned long loops_per_jiffy = (1<<12);
 
 EXPORT_SYMBOL(loops_per_jiffy);
 
-/* This is the number of bits of precision for the loops_per_jiffy.  Each
-   bit takes on average 1.5/HZ seconds.  This (like the original) is a little
-   better than 1% */
+/*
+ * This is the number of bits of precision for the loops_per_jiffy.  Each
+ * bit takes on average 1.5/HZ seconds.  This (like the original) is a little
+ * better than 1%
+ */
 #define LPS_PREC 8
 
 void __devinit calibrate_delay(void)

@@ -183,40 +196,53 @@
 	unsigned long ticks, loopbit;
 	int lps_precision = LPS_PREC;
 
-	loops_per_jiffy = (1<<12);
+	if (preset_lpj) {
+		loops_per_jiffy = preset_lpj;
+		printk("Calibrating delay loop (skipped)... "
+			"%lu.%02lu BogoMIPS preset\n",
+			loops_per_jiffy/(500000/HZ),
+			(loops_per_jiffy/(5000/HZ)) % 100);
+	} else {
+		loops_per_jiffy = (1<<12);
+
+		printk("Calibrating delay loop... ");
+		while ((loops_per_jiffy <<= 1) != 0) {
+			/* wait for "start of" clock tick */
+			ticks = jiffies;
+			while (ticks == jiffies)
+				/* nothing */;
+			/* Go .. */
+			ticks = jiffies;
+			__delay(loops_per_jiffy);
+			ticks = jiffies - ticks;
+			if (ticks)
+				break;
+		}
 
-	printk("Calibrating delay loop... ");
-	while ((loops_per_jiffy <<= 1) != 0) {
-		/* wait for "start of" clock tick */
-		ticks = jiffies;
-		while (ticks == jiffies)
-			/* nothing */;
-		/* Go .. */
-		ticks = jiffies;
-		__delay(loops_per_jiffy);
-		ticks = jiffies - ticks;
-		if (ticks)
-			break;
-	}
+		/*
+		 * Do a binary approximation to get loops_per_jiffy set to
+		 * equal one clock (up to lps_precision bits)
+		 */
+		loops_per_jiffy >>= 1;
+		loopbit = loops_per_jiffy;
+		while (lps_precision-- && (loopbit >>= 1)) {
+			loops_per_jiffy |= loopbit;
+			ticks = jiffies;
+			while (ticks == jiffies)
+				/* nothing */;
+			ticks = jiffies;
+			__delay(loops_per_jiffy);
+			if (jiffies != ticks)	/* longer than 1 tick */
+				loops_per_jiffy &= ~loopbit;
+		}
 
-/* Do a binary approximation to get loops_per_jiffy set to equal one clock
-   (up to lps_precision bits) */
-	loops_per_jiffy >>= 1;
-	loopbit = loops_per_jiffy;
-	while ( lps_precision-- && (loopbit >>= 1) ) {
-		loops_per_jiffy |= loopbit;
-		ticks = jiffies;
-		while (ticks == jiffies);
-		ticks = jiffies;
-		__delay(loops_per_jiffy);
-		if (jiffies != ticks)	/* longer than 1 tick */
-			loops_per_jiffy &= ~loopbit;
+		/* Round the value and print it */	
+		printk("%lu.%02lu BogoMIPS (lpj=%lu)\n",
+			loops_per_jiffy/(500000/HZ),
+			(loops_per_jiffy/(5000/HZ)) % 100,
+			loops_per_jiffy);
 	}
 
-/* Round the value and print it */	
-	printk("%lu.%02lu BogoMIPS\n",
-		loops_per_jiffy/(500000/HZ),
-		(loops_per_jiffy/(5000/HZ)) % 100);
 }
 
 static int __init debug_kernel(char *str)

@@ -238,8 +264,10 @@
 __setup("debug", debug_kernel);
 __setup("quiet", quiet_kernel);
 
-/* Unknown boot options get handed to init, unless they look like
-   failed parameters */
+/*
+ * Unknown boot options get handed to init, unless they look like
+ * failed parameters
+ */
 static int __init unknown_bootoption(char *param, char *val)
 {
 	/* Change NUL term back to "=", to make "param" the whole string. */

@@ -250,8 +278,10 @@
 	if (obsolete_checksetup(param))
 		return 0;
 
-	/* Preemptive maintenance for "why didn't my mispelled command
-           line work?" */
+	/*
+	 * Preemptive maintenance for "why didn't my mispelled command
+	 * line work?"
+	 */
 	if (strchr(param, '.') && (!val || strchr(param, '.') < val)) {
 		printk(KERN_ERR "Unknown boot option `%s': ignoring\n", param);
 		return 0;

@@ -289,7 +319,8 @@
 	unsigned int i;
 
 	execute_command = str;
-	/* In case LILO is going to boot us with default command line,
+	/*
+	 * In case LILO is going to boot us with default command line,
 	 * it prepends "auto" before the whole cmdline which makes
 	 * the shell think it should execute a script with such name.
 	 * So we ignore all arguments entered _before_ init=... [MJ]

@@ -483,9 +514,9 @@
 	check_bugs();
 
 	/* 
-	 *	We count on the initial thread going ok 
-	 *	Like idlers init is an unlocked kernel thread, which will
-	 *	make syscalls (and thus be locked).
+	 * We count on the initial thread going ok 
+	 * Like idlers init is an unlocked kernel thread, which will
+	 * make syscalls (and thus be locked).
 	 */
 	init_idle(current, smp_processor_id());
Documentation/kernel-parameters.txt renamed from Documentation/kernel-parameters.txt.orig +14 -0
@@ -576,6 +576,20 @@
 				so, the driver will manage that printer.
 				See also header of drivers/char/lp.c.
 
+	lpj=n		[KNL]
+			Sets loops_per_jiffy to given constant, thus avoiding
+			time-consuming boot-time autodetection (up to 250 ms per
+			CPU). 0 enables autodetection (default). To determine
+			the correct value for your kernel, boot with normal
+			autodetection and see what value is printed. Note that
+			on SMP systems the preset will be applied to all CPUs,
+			which is likely to cause problems if your CPUs need
+			significantly divergent settings. An incorrect value
+			will cause delays in the kernel to be wrong, leading to
+			unpredictable I/O errors and other breakage. Although
+			unlikely, in the extreme case this might damage your
+			hardware.
+
 	ltpc=		[NET]
 			Format: <io>,<irq>,<dma>
author
#24 lore raw
Quoted 3 lines · Adam Kropelin

Here's a patch. It places the relevant information on the same line as bogomips and does so without encouraging anyone to fiddle with loops_per_jiffy and screw up their kernel.

The patch is missing the Kconfig piece. Is the wording the same as from your earlier patch?

=============================
Tim Bird
Architecture Group Co-Chair, CE Linux Forum
Senior Staff Engineer, Sony Electronics
E-mail: [email protected]
=============================

#25 lore raw
Quoted 7 lines · Tim Bird
Quoted 3 lines · Adam Kropelin

Here's a patch. It places the relevant information on the same line as bogomips and does so without encouraging anyone to fiddle with loops_per_jiffy and screw up their kernel.

The patch is missing the Kconfig piece. Is the wording the same as from your earlier patch?

Andrew requested that the config option be removed, so there are no longer any changes to Kconfig.

--Adam

author
#26 lore raw
Quoted 8 lines · Adam Kropelin
Quoted 6 lines · Tim Bird
Quoted 3 lines · Adam Kropelin

Here's a patch. It places the relevant information on the same line as bogomips and does so without encouraging anyone to fiddle with loops_per_jiffy and screw up their kernel.

Quoted 6 lines
Quoted 2 lines

The patch is missing the Kconfig piece. Is the wording the same as from your earlier patch?

Andrew requested that the config option be removed, so there are no longer any changes to Kconfig.

Oh - I missed the change where preset_lpj is no longer initialized from CONFIG_PRESET_LPJ. I see it now. I saw the comment from Andrew, but thought he was talking about the first version of the patch that had ifdefs.

Quoted 7 lines · Here's what Andrew Morton

a) I don't see much point in making it configurable. Just add the boot option and be done with it. The few hundred bytes of extra code will be dropped from core anyway.

The main reason for this is that most people won't turn on the config option, so your new code could get accidentally broken quite easily. Plus it removes some ifdefs.

I have one or two arguments in favor of keeping the config option. First, for embedded systems it is sometimes necessary to compile-in the value. Unfortunately, not all architectures allow the kernel command line to be compiled in. (It would be nice if they did, but that's something to fix separately.)

The first restructuring of the patch that you did, I think addresses the issues that Andrew raised. There are now no ifdefs, and since the core code doesn't drop away (and indeed can be turned on at boot time if needed), I think the code is not susceptible to accidental breakage like the first version was. I think Andrew is right that the preset code path won't get much testing outside of embedded circles, so it is important that it be as simple as possible.

The only *code* change from the configurable to the config-less version of the code is:

+static unsigned long preset_lpj = CONFIG_PRESET_LPJ;
vs.
+static unsigned long preset_lpj;

Secondly, I was hoping to get the FASTBOOT menu in the kernel, because CELF has a few more patches to submit that have the same theme as this one. (I realize this is not justification for making this option configurable, per se. If we need to push for this on a subsequent patch, that's OK.)

If we do put the config option back in, then there's a mistake in the wording that needs to be fixed. The statement "loops_per_jiffy is roughly BogoMips * 5000." This is only true when HZ is 100. This statement should just be removed - the rest of the wording works without it.

With regard to testing - everything worked as expected. I think the config option issue is the last thing to be resolved before recommending that the patch be applied.

=============================
Tim Bird
Architecture Group Co-Chair, CE Linux Forum
Senior Staff Engineer, Sony Electronics
E-mail: [email protected]
=============================

#27 lore raw

Hi,

Quoted 27 lines · Tim Bird
Quoted 11 lines · Adam Kropelin
Quoted 7 lines · Tim Bird
Quoted 3 lines · Adam Kropelin

Here's a patch. It places the relevant information on the same line as bogomips and does so without encouraging anyone to fiddle with loops_per_jiffy and screw up their kernel.

The patch is missing the Kconfig piece. Is the wording the same as from your earlier patch?

Andrew requested that the config option be removed, so there are no longer any changes to Kconfig.

Oh - I missed the change where preset_lpj is no longer initialized from CONFIG_PRESET_LPJ. I see it now. I saw the comment from Andrew, but thought he was talking about the first version of the patch that had ifdefs.

Quoted 7 lines · Here's what Andrew Morton

a) I don't see much point in making it configurable. Just add the boot option and be done with it. The few hundred bytes of extra code will
be dropped from core anyway.

The main reason for this is that most people won't turn on the config option, so your new code could get accidentally broken quite easily. Plus it removes some ifdefs.

Quoted 5 lines

I have one or two arguments in favor of keeping the config option. First, for embedded systems it is sometimes necessary to compile-in the value. Unfortunately, not all architectures allow the kernel command line to be compiled in. (It would be nice if they did, but that's something to fix separately.)

Many do (i.e. arm, h8300, mips, ppc, sh) and IMHO it is better to fix it separately where needed.

Quoted 11 lines

The first restructuring of the patch that you did, I think addresses the issues that Andrew raised. There are now no ifdefs, and since the core code doesn't drop away (and indeed can be turned on at boot time if needed), I think the code is not susceptible to accidental breakage like the first version was. I think Andrew is right that the preset code path won't get much testing outside of embedded circles, so it is important that it be as simple as possible.

The only *code* change from the configurable to the config-less version of the code is:

Review

Quoted 3 lines
(quoted diff) +2 -0
+static unsigned long preset_lpj = CONFIG_PRESET_LPJ;
   vs.
+static unsigned long preset_lpj;

with the latter statement 'preset_lpj' ends up in BSS instead of data section ;-)

Quoted 5 lines

Secondly, I was hoping to get the FASTBOOT menu in the kernel, because CELF has a few more patches to submit that have the same theme as this one. (I realize this is not justification for making this option configurable, per se. If we need to push for this on a subsequent patch, that's OK.)

Separate patch please if/when needed.

author
#28 lore raw
In reply to Andrew Morton
Quoted 3 lines · Andrew Morton
Quoted 1 line · Adam Kropelin
Quoted 1 line

Does 250 ms worth all this pain?

My requirement is to get to user space in 500 ms from product cold start. It would be very nice to do so in 250 ms, to give user space init more time. This requirement is pretty characteristic of many CE products.

Quoted 9 lines
Quoted 4 lines

On a desktop box, almost certainly not. On a massive SMP machine, maybe. On an embedded system that is required to boot in a ridiculously short time, absolutely.

Yes, it's pretty small beer, but we do recognise that although the number of development teams which use features like this is small, the number of systems is large, so the features are correspondingly more important.

Quoted 5 lines

One of the services which we kernel developers provide the downstream kernel users is the hosting and maintenance of their code so they don't have to carry important stuff off-stream, and when the changes are this small and simple, I don't see a problem with merging them, even if none of "us" will use the feature.

Thanks. Even if our features aren't mainstream, if we can make them unintrusive, it's nice to get them integrated. The help offered here, to make this as unintrusive and danger-free as possible, is really appreciated.

As to the number of developers and systems using this, I would wager that developers at almost every CE company will use this (with developers in the 100s), and that it will affect (at least) 100s of thousands of devices (possibly millions, depending on how Linux does in cell phones).

=============================
Tim Bird
Architecture Group Co-Chair, CE Linux Forum
Senior Staff Engineer, Sony Electronics
E-mail: [email protected]
=============================