Re: [PATCH] preset loops_per_jiffy for faster booting

[Tim Bird <tim.bird@am.sony.com>]

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