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>