In-Reply-To: <[email protected]>
On Mon, 27 Feb 2006 14:32:05, Chris Wright wrote:
> -stable review patch. If anyone has any objections, please let us know.
> ------------------
>
> early_cpu_detect only runs on the BP, but this code needs to run
> on all CPUs. This will fix problems with the powernow-k8 driver
> on dual core systems and general misdetection of AMD dual core.
>
> Looks like a mismerge somewhere. Also add a warning comment.
>
> Signed-off-by: Andi Kleen <[email protected]>
> Signed-off-by: Chris Wright <[email protected]>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
> ---
>
> arch/i386/kernel/cpu/common.c | 17 ++++++++++-------
> 1 files changed, 10 insertions(+), 7 deletions(-)
>
> --- linux-2.6.15.4.orig/arch/i386/kernel/cpu/common.c
> +++ linux-2.6.15.4/arch/i386/kernel/cpu/common.c
> @@ -207,7 +207,10 @@ static int __devinit have_cpuid_p(void)
>
> /* Do minimum CPU detection early.
> Fields really needed: vendor, cpuid_level, family, model, mask, cache alignment.
> - The others are not touched to avoid unwanted side effects. */
> + The others are not touched to avoid unwanted side effects.
> +
> + WARNING: this function is only called on the BP. Don't add code here
> + that is supposed to run on all CPUs. */
> static void __init early_cpu_detect(void)
> {
> struct cpuinfo_x86 *c = &boot_cpu_data;
> @@ -239,12 +242,6 @@ static void __init early_cpu_detect(void
> if (cap0 & (1<<19))
> c->x86_cache_alignment = ((misc >> 8) & 0xff) * 8;
> }
> -
> - early_intel_workaround(c);
==> This has been in this location since at least 2.6.9. If it's also needed
in generic_identify(), fine, but it should be left here too. I think this
patch in 2.6.16-rc is wrong. The comment for this function says it really
needs accurate cache alignment information, presumably because it's needed
for early boot setup, and without this it won't be accurate.
> -
> -#ifdef CONFIG_X86_HT
> - phys_proc_id[smp_processor_id()] = (cpuid_ebx(1) >> 24) & 0xff;
> -#endif
> }
>
> void __devinit generic_identify(struct cpuinfo_x86 * c)
> @@ -292,6 +289,12 @@ void __devinit generic_identify(struct c
> get_model_name(c); /* Default name */
> }
> }
> +
> + early_intel_workaround(c);
> +
> +#ifdef CONFIG_X86_HT
> + phys_proc_id[smp_processor_id()] = (cpuid_ebx(1) >> 24) & 0xff;
> +#endif
> }
>
> static void __devinit squash_the_stupid_serial_number(struct cpuinfo_x86 *c)
>
--
Chuck
"Equations are the Devil's sentences." --Stephen Colbert
> ==> This has been in this location since at least 2.6.9. If it's also
> needed in generic_identify(), fine, but it should be left here too. I
> think this patch in 2.6.16-rc is wrong. The comment for this function says
> it really needs accurate cache alignment information, presumably because
> it's needed for early boot setup, and without this it won't be accurate.
Actually it's ok because early_cpu_detect() is called from setup_arch()
which is way before kmem_cache_init() which is the first user of the
alignment information.
So I think the patch is fine. You're right the comment could need updating
though.
Thanks for the review.
-Andi