Date: Mon, 23 Feb 2009 21:57:40 -0800 From: Maxim Sobolev <sobomax@FreeBSD.org> To: Robert Watson <rwatson@FreeBSD.org> Cc: Frank Behrens <frank@ilse.behrens.de>, Jeff Roberson <jeff@FreeBSD.org>, "current@freebsd.org" <current@FreeBSD.org>, stable@FreeBSD.org Subject: Re: The machdep.hyperthreading_allowed & ULE weirdness in 7.1 Message-ID: <49A38C54.7070601@FreeBSD.org> In-Reply-To: <alpine.BSF.2.00.0902232026180.92010@fledge.watson.org> References: <alpine.BSF.2.00.0902231000300.98609@fledge.watson.org> <200902231652.n1NGqMxH047731@post.behrens.de> <49A2DE9D.4090902@FreeBSD.org> <alpine.BSF.2.00.0902231801450.92010@fledge.watson.org> <49A2ED6A.9040202@FreeBSD.org> <alpine.BSF.2.00.0902231853040.92010@fledge.watson.org> <49A30456.5010400@FreeBSD.org> <alpine.BSF.2.00.0902232026180.92010@fledge.watson.org>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------080303000001020500010202
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Robert Watson wrote:
>> So, are you suggesting that we should disable
>> machdep.hyperthreading_allowed with ULE in 7.x and current to avoid
>> confusion?
>
> Possibly even without ULE.
I've verified - the tunable/sysctl works just fine with SCHED_4BSD in
7.1, so that I am not sure it's worth ripping it off.
Instead, I've re-implemented the feature differently - by disabling HT
cores at detection time when SCHED_ULE is compiled in and
machdep.hyperthreading_allowed is set to 0 in loader.conf. Patch for 7.1
is attached. Apart from the fact that it's not possible to change the
setting at run-time, this version should be even better by not starting
up HT cores in the first place.
I would appreciate if somebody familiar with the code in question can
review the patch, particularly part that moves assign_cpu_ids() and
start_all_aps() calls little bit further in the cpu_mp_start(). I need
them there so that I can use hyperthreading_cpus in assign_cpu_ids().
Thanks in advance.
-Maxim
--------------080303000001020500010202
Content-Type: text/plain;
name="machdep.hyperthreading_allowed.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="machdep.hyperthreading_allowed.diff"
--- i386/i386/mp_machdep.c 2009-01-19 07:34:49.000000000 -0800
+++ /tmp/mp_machdep.c 2009-02-23 21:38:18.000000000 -0800
@@ -209,6 +210,7 @@
int cpu_present:1;
int cpu_bsp:1;
int cpu_disabled:1;
+ int cpu_hyperthread:1;
} static cpu_info[MAX_APIC_ID + 1];
int cpu_apic_ids[MAXCPU];
@@ -405,11 +407,6 @@
("BSP's APIC ID doesn't match boot_cpu_id"));
cpu_apic_ids[0] = boot_cpu_id;
- assign_cpu_ids();
-
- /* Start each Application Processor */
- start_all_aps();
-
/* Setup the initial logical CPUs info. */
logical_cpus = logical_cpus_mask = 0;
if (cpu_feature & CPUID_HTT)
@@ -457,6 +454,11 @@
hyperthreading_cpus = logical_cpus;
}
+ assign_cpu_ids();
+
+ /* Start each Application Processor */
+ start_all_aps();
+
set_interrupt_apic_ids();
/* Last, setup the cpu topology now that we have probed CPUs */
@@ -471,18 +473,26 @@
cpu_mp_announce(void)
{
int i, x;
+ const char *hyperthread;
/* List CPUs */
printf(" cpu0 (BSP): APIC ID: %2d\n", boot_cpu_id);
for (i = 1, x = 0; x <= MAX_APIC_ID; x++) {
if (!cpu_info[x].cpu_present || cpu_info[x].cpu_bsp)
continue;
+ if (cpu_info[x].cpu_hyperthread) {
+ hyperthread = "/HT";
+ } else {
+ hyperthread = "";
+ }
if (cpu_info[x].cpu_disabled)
- printf(" cpu (AP): APIC ID: %2d (disabled)\n", x);
+ printf(" cpu (AP%s): APIC ID: %2d (disabled)\n",
+ hyperthread, x);
else {
KASSERT(i < mp_ncpus,
("mp_ncpus and actual cpus are out of whack"));
- printf(" cpu%d (AP): APIC ID: %2d\n", i++, x);
+ printf(" cpu%d (AP%s): APIC ID: %2d\n", i++,
+ hyperthread, x);
}
}
}
@@ -691,11 +701,24 @@
{
u_int i;
+ TUNABLE_INT_FETCH("machdep.hyperthreading_allowed",
+ &hyperthreading_allowed);
+
/* Check for explicitly disabled CPUs. */
for (i = 0; i <= MAX_APIC_ID; i++) {
if (!cpu_info[i].cpu_present || cpu_info[i].cpu_bsp)
continue;
+ if (hyperthreading_cpus > 1 && i % hyperthreading_cpus != 0) {
+ cpu_info[i].cpu_hyperthread = 1;
+#if defined(SCHED_ULE)
+ if (hyperthreading_allowed == 0) {
+ cpu_info[i].cpu_disabled = 1;
+ continue;
+ }
+#endif
+ }
+
/* Don't use this CPU if it has been disabled by a tunable. */
if (resource_disabled("lapic", i)) {
cpu_info[i].cpu_disabled = 1;
@@ -1410,6 +1433,12 @@
if (error || !req->newptr)
return (error);
+#ifdef SCHED_ULE
+ if (allowed != hyperthreading_allowed)
+ return (ENOTSUP);
+ return (error);
+#endif
+
if (allowed)
hlt_cpus_mask &= ~hyperthreading_cpus_mask;
else
@@ -1454,8 +1483,6 @@
* of hlt_logical_cpus.
*/
if (hyperthreading_cpus_mask) {
- TUNABLE_INT_FETCH("machdep.hyperthreading_allowed",
- &hyperthreading_allowed);
SYSCTL_ADD_PROC(&logical_cpu_clist,
SYSCTL_STATIC_CHILDREN(_machdep), OID_AUTO,
"hyperthreading_allowed", CTLTYPE_INT|CTLFLAG_RW,
--------------080303000001020500010202--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?49A38C54.7070601>