[PATCH 1/2] x86, Clean up smp_num_siblings calculation

[Prarit Bhargava <prarit@redhat.com>]

I have a system on which I have disabled threading in the BIOS, and I am booting
the kernel with the option "idle=poll".

The kernel displays

process: WARNING: polling idle and HT enabled, performance may degrade

which is incorrect -- I've already disabled HT.

This warning is issued here:

void select_idle_routine(const struct cpuinfo_x86 *c)
{
        if (boot_option_idle_override == IDLE_POLL && smp_num_siblings > 1)
                pr_warn_once("WARNING: polling idle and HT enabled, performance may degrade\n");

>From my understanding of the other areas of kernel that use smp_num_siblings,
the value is supposed to be the actual number of threads per core, and
this value of smp_num_siblings is incorrect.  In theory, it should be 1 but it
is reported as 2.  When I looked into how smp_num_siblings is calculated I
found the following call sequence in the kernel:

start_kernel ->
        check_bugs ->
                identify_boot_cpu ->
                                identify_cpu ->
                                        c_init = init_intel
                                                init_intel ->
                                                        detect_extended_topology
                                                        (sets value)

                                        OR

                                        c_init = init_amd
                                                init_amd -> amd_detect_cmp
                                                             -> amd_get_topology
                                                                (sets value)
                                                         -> detect_ht()
                                        ...		    (sets value)
                                        detect_ht()
                                        (also sets value)

ie) it is confusing to figure out where smp_num_siblings is set.

It should be noted that nothing in the identify_cpu() path or the cpu_up()
path requires smp_num_siblings to be set, prior to the final call to
detect_ht().

For x86 boxes without X86_FEATURE_XTOPOLOGY, smp_num_siblings is set to a
value read in a CPUID call in detect_ht().  This value is the *factory
defined* value in all cases; even if HT is disabled in BIOS the value
still returns 2 if the CPU supports HT.  AMD also reports the factory
defined value in all cases.

For Intel x86 boxes with X86_FEATURE_XTOPOLOGY, smp_num_siblings is set to a
value read from the 0xb leaf of CPUID.  This value is also the *factory
defined* value in all cases.

For new-ish AMD x86 boxes, smp_num_siblings is also set to the *factory*
defined value.

That is, even with threading disabled in BIOSes on these systems,

crash> p smp_num_siblings
smp_num_siblings = $1 = 0x2

smp_num_siblings should be calculated a single time on cpu 0 to determine
whether or not the system is multi-threaded or not.  We can easily do
this by examining the boot cpu's cpu_sibling_mask after the mask has been
setup in the boot up code path.

After the patch, on a system with HT enabled,

crash> p smp_num_siblings
smp_num_siblings = $1 = 0x2

On a system with HT disabled,

crash> p smp_num_siblings
smp_num_siblings = $1 = 0x1

Other uses of smp_num_siblings involve oprofile (used after boot), and
the perf code which is done well after the initial cpus are brought online.

[v2]: After comment from Oren Twaig, rework to single patch.
Unfortunately there was no easy way to take into account the various
settings of smp_num_siblings and fix it in two patches.

Cc: Oren Twaig <oren@scalemp.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: Borislav Petkov <bp@suse.de>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Dave Jones <davej@redhat.com>
Cc: Torsten Kaiser <just.for.lkml@googlemail.com>
Cc: Jan Beulich <JBeulich@suse.com>
Cc: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Toshi Kani <toshi.kani@hp.com>
Cc: Andrew Jones <drjones@redhat.com>
Signed-off-by: Prarit Bhargava <prarit@redhat.com>
---
 arch/x86/kernel/cpu/amd.c      |    1 -
 arch/x86/kernel/cpu/common.c   |   23 +++++++++++------------
 arch/x86/kernel/cpu/topology.c |    2 +-
 arch/x86/kernel/smpboot.c      |   10 +++++++---
 4 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index ce8b8ff..6aca2b6 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -304,7 +304,6 @@ static void amd_get_topology(struct cpuinfo_x86 *c)
 		node_id = ecx & 7;
 
 		/* get compute unit information */
-		smp_num_siblings = ((ebx >> 8) & 3) + 1;
 		c->compute_unit_id = ebx & 0xff;
 		cores_per_cu += ((ebx >> 8) & 3);
 	} else if (cpu_has(c, X86_FEATURE_NODEID_MSR)) {
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index ef1b93f..2d832861 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -509,42 +509,41 @@ void detect_ht(struct cpuinfo_x86 *c)
 	u32 eax, ebx, ecx, edx;
 	int index_msb, core_bits;
 	static bool printed;
+	int threads_per_core;
 
 	if (!cpu_has(c, X86_FEATURE_HT))
 		return;
 
-	if (cpu_has(c, X86_FEATURE_CMP_LEGACY))
+	if (cpu_has(c, X86_FEATURE_CMP_LEGACY)) {
+		threads_per_core = 1;
 		goto out;
+	}
 
 	if (cpu_has(c, X86_FEATURE_XTOPOLOGY))
 		return;
 
 	cpuid(1, &eax, &ebx, &ecx, &edx);
 
-	smp_num_siblings = (ebx & 0xff0000) >> 16;
+	threads_per_core = (ebx & 0xff0000) >> 16;
 
-	if (smp_num_siblings == 1) {
-		printk_once(KERN_INFO "CPU0: Hyper-Threading is disabled\n");
+	if (threads_per_core <= 1) {
+		pr_info_once("CPU: Hyper-Threading is unsupported on this processor.\n");
 		goto out;
 	}
 
-	if (smp_num_siblings <= 1)
-		goto out;
-
-	index_msb = get_count_order(smp_num_siblings);
+	index_msb = get_count_order(threads_per_core);
 	c->phys_proc_id = apic->phys_pkg_id(c->initial_apicid, index_msb);
 
-	smp_num_siblings = smp_num_siblings / c->x86_max_cores;
+	threads_per_core = threads_per_core / c->x86_max_cores;
 
-	index_msb = get_count_order(smp_num_siblings);
+	index_msb = get_count_order(threads_per_core);
 
 	core_bits = get_count_order(c->x86_max_cores);
 
 	c->cpu_core_id = apic->phys_pkg_id(c->initial_apicid, index_msb) &
 				       ((1 << core_bits) - 1);
-
 out:
-	if (!printed && (c->x86_max_cores * smp_num_siblings) > 1) {
+	if (!printed && (c->x86_max_cores * threads_per_core) > 1) {
 		printk(KERN_INFO  "CPU: Physical Processor ID: %d\n",
 		       c->phys_proc_id);
 		printk(KERN_INFO  "CPU: Processor Core ID: %d\n",
diff --git a/arch/x86/kernel/cpu/topology.c b/arch/x86/kernel/cpu/topology.c
index 4c60eaf..a9b837e 100644
--- a/arch/x86/kernel/cpu/topology.c
+++ b/arch/x86/kernel/cpu/topology.c
@@ -55,7 +55,7 @@ void detect_extended_topology(struct cpuinfo_x86 *c)
 	/*
 	 * Populate HT related information from sub-leaf level 0.
 	 */
-	core_level_siblings = smp_num_siblings = LEVEL_MAX_SIBLINGS(ebx);
+	core_level_siblings = LEVEL_MAX_SIBLINGS(ebx);
 	core_plus_mask_width = ht_mask_width = BITS_SHIFT_NEXT_LEVEL(eax);
 
 	sub_index = 1;
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 5492798..e5ab30b 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -358,8 +358,7 @@ static bool match_mc(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o)
 
 void set_cpu_sibling_map(int cpu)
 {
-	bool has_smt = smp_num_siblings > 1;
-	bool has_mp = has_smt || boot_cpu_data.x86_max_cores > 1;
+	bool has_mp = boot_cpu_data.x86_max_cores > 1;
 	struct cpuinfo_x86 *c = &cpu_data(cpu);
 	struct cpuinfo_x86 *o;
 	int i;
@@ -371,13 +370,14 @@ void set_cpu_sibling_map(int cpu)
 		cpumask_set_cpu(cpu, cpu_llc_shared_mask(cpu));
 		cpumask_set_cpu(cpu, cpu_core_mask(cpu));
 		c->booted_cores = 1;
+		smp_num_siblings = 1;
 		return;
 	}
 
 	for_each_cpu(i, cpu_sibling_setup_mask) {
 		o = &cpu_data(i);
 
-		if ((i == cpu) || (has_smt && match_smt(c, o)))
+		if ((i == cpu) || match_smt(c, o))
 			link_mask(sibling, cpu, i);
 
 		if ((i == cpu) || (has_mp && match_llc(c, o)))
@@ -415,6 +415,10 @@ void set_cpu_sibling_map(int cpu)
 				c->booted_cores = cpu_data(i).booted_cores;
 		}
 	}
+
+	/* Only need to check this on the boot cpu, o/w it is disabled */
+	if (cpu == 0)
+		smp_num_siblings = cpumask_weight(cpu_sibling_mask(cpu));
 }
 
 /* maps the cpu to the sched domain representing multi-core */
-- 
1.7.9.3