Messages in this thread | | | Date | Mon, 12 Jul 2004 15:41:42 -0700 | From | Tim Bird <> | Subject | Re: [PATCH] preset loops_per_jiffy for faster booting |
| |
Adam Kropelin wrote: > On Mon, Jul 12, 2004 at 11:52:44AM -0700, Tim Bird wrote: > >>Adam Kropelin wrote: >> >>>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.
Here's what Andrew Morton said: > 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: tim.bird@am.sony.com ============================= - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |