* [PATCH 1/2] x86, Clean up smp_num_siblings calculation
2014-06-20 17:27 [PATCH 0/2] Fixes for smp_num_siblings calculation Prarit Bhargava
@ 2014-06-20 17:27 ` Prarit Bhargava
2014-06-20 17:27 ` [PATCH 2/2] x86, disable ht flag when hyperthreading is disabled Prarit Bhargava
1 sibling, 0 replies; 3+ messages in thread
From: Prarit Bhargava @ 2014-06-20 17:27 UTC (permalink / raw)
To: linux-kernel
Cc: Prarit Bhargava, Oren Twaig, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, x86, Borislav Petkov, Paul Gortmaker,
Andrew Morton, Andi Kleen, Dave Jones, Torsten Kaiser,
Jan Beulich, Jan Kiszka, Toshi Kani, Andrew Jones
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
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH 2/2] x86, disable ht flag when hyperthreading is disabled
2014-06-20 17:27 [PATCH 0/2] Fixes for smp_num_siblings calculation Prarit Bhargava
2014-06-20 17:27 ` [PATCH 1/2] x86, Clean up " Prarit Bhargava
@ 2014-06-20 17:27 ` Prarit Bhargava
1 sibling, 0 replies; 3+ messages in thread
From: Prarit Bhargava @ 2014-06-20 17:27 UTC (permalink / raw)
To: linux-kernel
Cc: Prarit Bhargava, Oren Twaig, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, x86, Borislav Petkov, Paul Gortmaker,
Andrew Morton, Andi Kleen, Dave Jones, Torsten Kaiser,
Jan Beulich, Jan Kiszka, Toshi Kani, Andrew Jones
The ht flag indicates that a processor is capable of having sibling threads.
This has long been confusing for users who confuse the ht flag as
indicating that hyperthreading is enabled on their systems even though
it may have been disabled in BIOS.
This patch removes the hyperthreading flag when no cpu thread siblings are
found, ie) smp_num_siblings == 1.
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/smpboot.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index e5ab30b..2eaadf0 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -371,6 +371,7 @@ void set_cpu_sibling_map(int cpu)
cpumask_set_cpu(cpu, cpu_core_mask(cpu));
c->booted_cores = 1;
smp_num_siblings = 1;
+ clear_cpu_cap(c, X86_FEATURE_HT);
return;
}
@@ -419,6 +420,9 @@ void set_cpu_sibling_map(int cpu)
/* 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));
+
+ if (cpu_has_ht && smp_num_siblings == 1)
+ clear_cpu_cap(c, X86_FEATURE_HT);
}
/* maps the cpu to the sched domain representing multi-core */
--
1.7.9.3
^ permalink raw reply [flat|nested] 3+ messages in thread