On 20 April 2011 22:29, Sergey Kandaurov <[email protected]> wrote:
> On 20 April 2011 21:02, Ian Smith <[email protected]> wrote:
>> On Wed, 13 Apr 2011, Ian Smith wrote:
>> > On Tue, 12 Apr 2011, Daniel Gerzo wrote:
>> > > On 11.4.2011 6:08, Ian Smith wrote:
>> [..]
>> > > > Are those kern.cp_times values as they came, or did you remove
>> trailing
>> > > > zeroes? Reason I ask is that on my Thinkpad T23, single-core
>> 1133/733
>> > > > MHz, sysctl kern.cp_time shows the usual 5 values, but kern.cp_times
>> has
>> > > > the same 5 values for cpu0, but then 5 zeroes for each of cpu1
>> through
>> > > > cpu31, on 8.2-PRE about early January. I need to update the script
>> to
>> > > > remove surplus data for non-existing cpus, but wonder if the extra
>> data
>> > > > also appeared on your 12 core box?
>> > >
>> > > I haven't removed anything, it's a pure copy&paste.
>> >
>> > Thanks. I'll check the single-cpu case again after updating to 8.2-R
>>
>> Ok, still a problem on at least my i386 single core Thinkpad T23 at
>> 8.2-R, since 8.0 I think, certainly evident in a sysctl -a at 8.1-R
>>
>> FreeBSD t23.smithi.id.au 8.2-RELEASE FreeBSD 8.2-RELEASE #1: Thu Apr 14
>> 21:45:47 EST 2011 [email protected]:/usr/obj/usr/src/sys/GENERIC i386
>>
>> Verbose dmesg: http://smithi.id.au/t23_dmesg_boot-v.8.2-R.txt
>> sysctl -a: http://smithi.id.au/t23_sysctl-a_8.2-R.txt
>>
>> kern.ccpu: 0
>> <cpu count="1" mask="0x1">0</cpu>
>> kern.smp.forward_signal_enabled: 1
>> kern.smp.topology: 0
>> kern.smp.cpus: 1
>> kern.smp.disabled: 0
>> kern.smp.active: 0
>> kern.smp.maxcpus: 32
>> kern.smp.maxid: 31 <<<<<<<
>> hw.ncpu: 1
>>
>> kern.cp_times: 38548 1 120437 195677 9660939 0 0 0 0 0 0 0 0 0 0 0 0 0 0
>> 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
>> 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
>> 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
>> 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
>>
>> /usr/src/sys/kern/kern_clock.c:
>> return SYSCTL_OUT(req, 0, sizeof(long) * CPUSTATES * (mp_maxid + 1));
>>
>> Consumers of kern.cp_times like powerd, top, dtrace? and others have to
>> loop over 32 cpus, all but one non-existent, and there seem to be many
>> places in the kernel doing eg: for (cpu = 0; cpu <= mp_maxid; cpu++) {
>> and while CPU_FOREACH / CPU_ABSENT will skip over them, seems wasteful
>> at best on machines least likely to have cycles to spare.
>>
>> eg: powerd parses kern.cp_times to count cpus, wasting cycles adding
>> up the 31 'empty' cpus. I haven't explored other userland consumers.
>>
>> Clearly kern.smp.maxid (ie mp_maxid) should be 0, not 31. On i386,
>> non-APIC i386 at least, mp_maxid is not set to (mp_ncpus - 1) as on some
>> other archs .. after having being initialised to (MAXCPU - 1) in
>> /sys/i386/i386/mp_machdep.c it's never updated for non-smp machines.
>>
>> I haven't chased all of these rabbits down all of their holes by any
>> means, but it seems that making /sys/i386/i386/mp_machdep.c do what it
>> says it's gonna do ('with an id of 0') should help. Paste, tabs lost:
>>
>> int
>> cpu_mp_probe(void)
>> {
>> /*
>> * Always record BSP in CPU map so that the mbuf init code works
>> * correctly.
>> */
>> all_cpus = 1;
>> if (mp_ncpus == 0) {
>> /*
>> * No CPUs were found, so this must be a UP system. Setup
>> * the variables to represent a system with a single CPU
>> * with an id of 0.
>> */
>> mp_ncpus = 1;
>> + mp_maxid = 0;
>> return (0);
>> }
>>
>> /* At least one CPU was found. */
>> if (mp_ncpus == 1) {
>> /*
>> * One CPU was found, so this must be a UP system with
>> * an I/O APIC.
>> */
>> + mp_maxid = 0;
>> return (0);
>> }
>>
>> /* At least two CPUs were found. */
>> return (1);
>> }
>>
>> Note that the second added line above already exists in
>> /sys/amd64/amd64/mp_machdep.c, maybe to fix a similar problem, though
>> that should only apply to 'a UP system with an I/O APIC'. Maybe better
>> could be to fix this in cpu_mp_probe's caller, /sys/kern/subr_smp.c:
>>
>> static void
>> mp_start(void *dummy)
>> {
>> mtx_init(&smp_ipi_mtx, "smp rendezvous", NULL, MTX_SPIN);
>>
>> /* Probe for MP hardware. */
>> if (smp_disabled != 0 || cpu_mp_probe() == 0) {
>> mp_ncpus = 1;
>> + mp_maxid = 0;
>> all_cpus = PCPU_GET(cpumask);
>> return;
>> }
>>
>> cpu_mp_start();
>> printf("FreeBSD/SMP: Multiprocessor System Detected: %d CPUs\n",
>> mp_ncpus);
>> cpu_mp_announce();
>> }
>>
>> I'm probably a long way off base for a solution, but think I've located
>> the problem. Thoughts? Is this a known issue? Might any developers
>> actually still have a single-cpu i386 system to check this on? :)
>>
>> Very happy to test any patches etc.
>>
>
> Ouch.
> Looks like that affects a system with 2 cores as well.
> Intel Core2 E7200, 8.2-R i386 SMP:
>
> kern.smp.forward_signal_enabled: 1
> kern.smp.topology: 0
> kern.smp.cpus: 2
> kern.smp.disabled: 0
> kern.smp.active: 1
> kern.smp.maxcpus: 32
> kern.smp.maxid: 31
>
> kern.cp_times: 867360 171 429180 70114 170549535 1385294 306 176659 82618
> 170270900 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
> 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
> 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
> 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
> 0 0 0 0 0 0 0
Perhaps that was so from the begging: the problem is seen also on
6.4 i386 SMP 8 core. Here it was just set to (MAXCPU-1).
The buggy mp_maxid was fixed in HEAD with r215009, though not merged.
The patch works on my 8.2 Core2 SMP i386 system.
--
wbr,
pluknet
Index: x86/x86/mptable.c
===================================================================
--- x86/x86/mptable.c (revision 215008)
+++ x86/x86/mptable.c (revision 215009)
@@ -389,7 +389,8 @@
apic_register_enumerator(&mptable_enumerator);
}
-SYSINIT(mptable_register, SI_SUB_MPTBL, SI_ORDER_FIRST, mptable_register, NULL);
+SYSINIT(mptable_register, SI_SUB_TUNABLES - 1, SI_ORDER_FIRST, mptable_register,
+ NULL);
/*
* Call the handler routine for each entry in the MP config table.
Index: x86/x86/local_apic.c
===================================================================
--- x86/x86/local_apic.c (revision 215008)
+++ x86/x86/local_apic.c (revision 215009)
@@ -1285,7 +1285,7 @@
if (resource_disabled("apic", 0))
return;
- /* First, probe all the enumerators to find the best match. */
+ /* Probe all the enumerators to find the best match. */
best_enum = NULL;
best = 0;
SLIST_FOREACH(enumerator, &enumerators, apic_next) {
@@ -1321,13 +1321,12 @@
}
#endif
- /* Second, probe the CPU's in the system. */
+ /* Probe the CPU's in the system. */
retval = best_enum->apic_probe_cpus();
if (retval != 0)
printf("%s: Failed to probe CPUs: returned %d\n",
best_enum->apic_name, retval);
-#ifdef __amd64__
}
SYSINIT(apic_init, SI_SUB_TUNABLES - 1, SI_ORDER_SECOND, apic_init, NULL);
@@ -1342,19 +1341,14 @@
if (best_enum == NULL)
return;
-#endif
- /* Third, initialize the local APIC. */
+
+ /* Initialize the local APIC. */
retval = best_enum->apic_setup_local();
if (retval != 0)
printf("%s: Failed to setup the local APIC: returned %d\n",
best_enum->apic_name, retval);
}
-#ifdef __amd64__
-SYSINIT(apic_setup_local, SI_SUB_CPU, SI_ORDER_SECOND, apic_setup_local,
- NULL);
-#else
-SYSINIT(apic_init, SI_SUB_CPU, SI_ORDER_SECOND, apic_init, NULL);
-#endif
+SYSINIT(apic_setup_local, SI_SUB_CPU, SI_ORDER_SECOND, apic_setup_local, NULL);
/*
* Setup the I/O APICs.
Index: i386/acpica/madt.c
===================================================================
--- i386/acpica/madt.c (revision 215008)
+++ i386/acpica/madt.c (revision 215009)
@@ -203,7 +203,7 @@
apic_register_enumerator(&madt_enumerator);
}
-SYSINIT(madt_register, SI_SUB_CPU - 1, SI_ORDER_SECOND, madt_register, NULL);
+SYSINIT(madt_register, SI_SUB_TUNABLES - 1, SI_ORDER_FIRST, madt_register, NULL);
/*
* Call the handler routine for each entry in the MADT table.
Index: i386/i386/mp_machdep.c
===================================================================
--- i386/i386/mp_machdep.c (revision 215008)
+++ i386/i386/mp_machdep.c (revision 215009)
@@ -465,8 +465,10 @@
boot_cpu_id = apic_id;
cpu_info[apic_id].cpu_bsp = 1;
}
- if (mp_ncpus < MAXCPU)
+ if (mp_ncpus < MAXCPU) {
mp_ncpus++;
+ mp_maxid = mp_ncpus - 1;
+ }
if (bootverbose)
printf("SMP: Added CPU %d (%s)\n", apic_id, boot_cpu ? "BSP" :
"AP");
@@ -476,7 +478,19 @@
cpu_mp_setmaxid(void)
{
- mp_maxid = MAXCPU - 1;
+ /*
+ * mp_maxid should be already set by calls to cpu_add().
+ * Just sanity check its value here.
+ */
+ if (mp_ncpus == 0)
+ KASSERT(mp_maxid == 0,
+ ("%s: mp_ncpus is zero, but mp_maxid is not", __func__));
+ else if (mp_ncpus == 1)
+ mp_maxid = 0;
+ else
+ KASSERT(mp_maxid >= mp_ncpus - 1,
+ ("%s: counters out of sync: max %d, count %d", __func__,
+ mp_maxid, mp_ncpus));
}
int
@@ -504,6 +518,7 @@
* One CPU was found, so this must be a UP system with
* an I/O APIC.
*/
+ mp_maxid = 0;
return (0);
}
Index: i386/xen/mptable.c
===================================================================
--- i386/xen/mptable.c (revision 215008)
+++ i386/xen/mptable.c (revision 215009)
@@ -109,7 +109,7 @@
apic_register_enumerator(&mptable_enumerator);
}
-SYSINIT(mptable_register, SI_SUB_CPU - 1, SI_ORDER_FIRST, mptable_register,
+SYSINIT(mptable_register, SI_SUB_TUNABLES - 1, SI_ORDER_FIRST, mptable_register,
NULL);
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "[email protected]"
