[RFC patch] disable the assumption of apic_is_clustered_box for cores >= 6

[RFC patch] disable the assumption of apic_is_clustered_box for cores >= 6

[Luming Yu]

[-- Attachment #1: Type: text/plain, Size: 1950 bytes --]

Hello,

We have to disable the assumption of apic_is_clustered_box that assumes high
4bits of 8 bit APIC ID was cluster id, which is just the function's guess.
On a normal 4-socket system with >6cores Processor and HT enabled, we hit the
limitation of the guess and failed. Have to disable the guess on any
cores > 6 system.

Without this patch, we can't use good TSC on any SMP system with 48+ logical CPU

**The patch is enclosed in text attachment*
**Using web client to send the patch* *
**below is for review, please apply attached  patch*/

Thanks,
Luming

 apic/apic.c |   15 ++++++++++++++-
 cpu/intel.c |    2 +-
 2 files changed, 15 insertions(+), 2 deletions(-)

--- linux-2.6.30-rc6/arch/x86/kernel/apic/apic.c.0	2009-06-29
19:54:15.000000000 -0600
+++ linux-2.6.30-rc6/arch/x86/kernel/apic/apic.c	2009-06-29
20:14:15.000000000 -0600
@@ -2119,6 +2119,16 @@
 #endif	/* CONFIG_PM */

 #ifdef CONFIG_X86_64
+extern int __cpuinit intel_num_cpu_cores(struct cpuinfo_x86 *c);
+
+__cpuinit int __detect_cores(void)
+{
+	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
+		boot_cpu_data.x86_max_cores =
+			intel_num_cpu_cores(&boot_cpu_data);
+	return boot_cpu_data.x86_max_cores;
+}
+
 /*
  * apic_is_clustered_box() -- Check if we can expect good TSC
  *
@@ -2190,7 +2200,10 @@
 	 * May have to revisit this when multi-core + hyperthreaded CPUs come
 	 * out, but AFAIK this will work even for them.
 	 */
-	return (clusters > 2);
+	if (clusters > 2)
+		return (__detect_cores() >= 6) ? 0 : 1;
+	else
+		return 0;
 }
 #endif

--- linux-2.6.30-rc6/arch/x86/kernel/cpu/intel.c.0	2009-06-29
19:53:29.000000000 -0600
+++ linux-2.6.30-rc6/arch/x86/kernel/cpu/intel.c	2009-06-29
08:46:12.000000000 -0600
@@ -250,7 +250,7 @@
 /*
  * find out the number of processor cores on the die
  */
-static int __cpuinit intel_num_cpu_cores(struct cpuinfo_x86 *c)
+int __cpuinit intel_num_cpu_cores(struct cpuinfo_x86 *c)
 {
 	unsigned int eax, ebx, ecx, edx;

[-- Attachment #2: 1.patch --]
[-- Type: application/octet-stream, Size: 1281 bytes --]

--- linux-2.6.30-rc6/arch/x86/kernel/apic/apic.c.0	2009-06-29 19:54:15.000000000 -0600
+++ linux-2.6.30-rc6/arch/x86/kernel/apic/apic.c	2009-06-29 20:14:15.000000000 -0600
@@ -2119,6 +2119,16 @@
 #endif	/* CONFIG_PM */
 
 #ifdef CONFIG_X86_64
+extern int __cpuinit intel_num_cpu_cores(struct cpuinfo_x86 *c);
+
+__cpuinit int __detect_cores(void)
+{
+	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
+		boot_cpu_data.x86_max_cores = 
+			intel_num_cpu_cores(&boot_cpu_data);
+	return boot_cpu_data.x86_max_cores;
+}
+
 /*
  * apic_is_clustered_box() -- Check if we can expect good TSC
  *
@@ -2190,7 +2200,10 @@
 	 * May have to revisit this when multi-core + hyperthreaded CPUs come
 	 * out, but AFAIK this will work even for them.
 	 */
-	return (clusters > 2);
+	if (clusters > 2)
+		return (__detect_cores() >= 6) ? 0 : 1;
+	else
+		return 0;
 }
 #endif
 
--- linux-2.6.30-rc6/arch/x86/kernel/cpu/intel.c.0	2009-06-29 19:53:29.000000000 -0600
+++ linux-2.6.30-rc6/arch/x86/kernel/cpu/intel.c	2009-06-29 08:46:12.000000000 -0600
@@ -250,7 +250,7 @@
 /*
  * find out the number of processor cores on the die
  */
-static int __cpuinit intel_num_cpu_cores(struct cpuinfo_x86 *c)
+int __cpuinit intel_num_cpu_cores(struct cpuinfo_x86 *c)
 {
 	unsigned int eax, ebx, ecx, edx;
 

Re: [RFC patch] disable the assumption of apic_is_clustered_box for cores >= 6

[Suresh Siddha]

On Mon, 2009-06-29 at 19:53 -0700, Luming Yu wrote:
> Hello,
> 
> We have to disable the assumption of apic_is_clustered_box that assumes high
> 4bits of 8 bit APIC ID was cluster id, which is just the function's guess.
> On a normal 4-socket system with >6cores Processor and HT enabled, we hit the
> limitation of the guess and failed. Have to disable the guess on any
> cores > 6 system.
> 
> Without this patch, we can't use good TSC on any SMP system with 48+ logical CPU

This patch doesn't seem to be the right fix. BTW, it appears that
Yinghai's patch in mainline appear to address this issue already. Did
you check if the mainline still has the issue that you are trying to fix
here?

commit e0e42142bab96404de535cceb85d6533d5ad7942
Author: Yinghai Lu <yinghai@kernel.org>
Date:   Sun Apr 26 23:39:38 2009 -0700

    x86: Use dmi check in apic_is_clustered() on 64-bit to mark the TSC
unstable

thanks,
suresh

Re: [RFC patch] disable the assumption of apic_is_clustered_box for cores >= 6

[Luming Yu]

On Fri, Jul 3, 2009 at 3:04 AM, Suresh Siddha<suresh.b.siddha@intel.com> wrote:
> On Mon, 2009-06-29 at 19:53 -0700, Luming Yu wrote:
>> Hello,
>>
>> We have to disable the assumption of apic_is_clustered_box that assumes high
>> 4bits of 8 bit APIC ID was cluster id, which is just the function's guess.
>> On a normal 4-socket system with >6cores Processor and HT enabled, we hit the
>> limitation of the guess and failed. Have to disable the guess on any
>> cores > 6 system.
>>
>> Without this patch, we can't use good TSC on any SMP system with 48+ logical CPU
>
> This patch doesn't seem to be the right fix. BTW, it appears that
> Yinghai's patch in mainline appear to address this issue already. Did
> you check if the mainline still has the issue that you are trying to fix
> here?

Oh.. my testing kernel is still -rc6. The patch should fix the problem.
Thanks for the pointer. I believe I have heard of this patch, but don't know
why I didn't check that first before spending time on this fix,
although this patch didn't take
too much time, but does create some noise. I'm sorry about that.