From owner-freebsd-stable@FreeBSD.ORG Tue Feb 24 05:58:02 2009 Return-Path: Delivered-To: stable@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 9878C106564A; Tue, 24 Feb 2009 05:58:02 +0000 (UTC) (envelope-from sobomax@FreeBSD.org) Received: from sippysoft.com (gk1.360sip.com [72.236.70.240]) by mx1.freebsd.org (Postfix) with ESMTP id 6576C8FC13; Tue, 24 Feb 2009 05:58:02 +0000 (UTC) (envelope-from sobomax@FreeBSD.org) Received: from [192.168.1.38] (S0106001372fd1e07.vs.shawcable.net [70.71.171.106]) (authenticated bits=0) by sippysoft.com (8.13.8/8.13.8) with ESMTP id n1O5vwLj049290 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 23 Feb 2009 21:57:59 -0800 (PST) (envelope-from sobomax@FreeBSD.org) Message-ID: <49A38C54.7070601@FreeBSD.org> Date: Mon, 23 Feb 2009 21:57:40 -0800 From: Maxim Sobolev Organization: Sippy Software, Inc. User-Agent: Thunderbird 2.0.0.19 (Windows/20081209) MIME-Version: 1.0 To: Robert Watson References: <200902231652.n1NGqMxH047731@post.behrens.de> <49A2DE9D.4090902@FreeBSD.org> <49A2ED6A.9040202@FreeBSD.org> <49A30456.5010400@FreeBSD.org> In-Reply-To: Content-Type: multipart/mixed; boundary="------------080303000001020500010202" Cc: Frank Behrens , Jeff Roberson , "current@freebsd.org" , stable@FreeBSD.org Subject: Re: The machdep.hyperthreading_allowed & ULE weirdness in 7.1 X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 24 Feb 2009 05:58:03 -0000 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--