svn commit: r233961 - head/sys/x86/x86

[John Baldwin]

On Monday, April 09, 2012 12:58:07 pm Attilio Rao wrote:
> Il 09 aprile 2012 17:34, John Baldwin <jhb at freebsd.org> ha scritto:
> > On Monday, April 09, 2012 11:45:11 am Jaakko Heinonen wrote:
> >>
> >> Hi,
> >>
> >> On 2012-04-06, Justin T. Gibbs wrote:
> >> >   Fix interrupt load balancing regression, introduced in revision
> >> >   222813, that left all un-pinned interrupts assigned to CPU 0.
> >> >
> >> >   sys/x86/x86/intr_machdep.c:
> >> >     In intr_shuffle_irqs(), remove CPU_SETOF() call that initialized
> >> >     the "intr_cpus" cpuset to only contain CPU0.
> >> >
> >> >     This initialization is too late and nullifies the results of calls
> >> >     the intr_add_cpu() that occur much earlier in the boot process.
> >> >     Since "intr_cpus" is statically initialized to the empty set, and
> >> >     all processors, including the BSP, already add themselves to
> >> >     "intr_cpus" no special initialization for the BSP is necessary.
> >>
> >> My Pentium 4 system hangs on boot after this commit. These are the last
> >> lines from a verbose boot:
> >>
> >> SMP: AP CPU #1 Launched!
> >> cpu1 AP:
> >>      ID: 0x01000000   VER: 0x00050014 LDR: 0x00000000 DFR: 0xffffffff
> >>   lint0: 0x00010700 lint1: 0x00000400 TPR: 0x00000000 SVR: 0x000001ff
> >>   timer: 0x000100ef therm: 0x00010000 err: 0x000000f0 pmc: 0x00010400
> >>
> >> The system boots with r233960.
> >>
> >> Some information:
> >>
> >> CPU: Intel(R) Pentium(R) 4 CPU 2.60GHz (2605.96-MHz 686-class CPU)
> >>   Origin = "GenuineIntel"  Id = 0xf29  Family = f  Model = 2  Stepping =
> >> 9
> >>
> > Features=0xbfebfbff<FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CLFLUSH,DTS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE>
> >>   Features2=0x4400<CNXT-ID,xTPR>
> >> real memory  = 2147483648 (2048 MB)
> >> avail memory = 2085228544 (1988 MB)
> >> Event timer "LAPIC" quality 400
> >> ACPI APIC Table: <A M I  OEMAPIC >
> >> FreeBSD/SMP: Multiprocessor System Detected: 2 CPUs
> >> FreeBSD/SMP: 1 package(s) x 1 core(s) x 2 HTT threads
> >>  cpu0 (BSP): APIC ID:  0
> >>  cpu1 (AP/HT): APIC ID:  1
> >
> > I suspect in your case intr_add_cpu() is never called.  I think Attilio is not
> > correct in that it is not called for the BSP.
> >
> > Yes, it is not called for the BSP in set_interrupt_apic_ids().  This used to
> > work because bit 0 was assigned statically.  Also, in a UP machine
> > set_interrupt_apic_ids() is not called at all.
> 
> But why there is a front-end check for the BSP in set_interrupt_apic_ids()?
> 
> Anyway, I think a better fix would be like the attached patch.

This would be fine.  What I would really prefer is to not need the sysinit at
all and be able to do something like the original pre-cpuset code:

static cpuset_t intr_cpus = CPU_INITIAILIZER(0);

Also, with the cpuset variant, I think we could remove the special case check
for the BSP from set_apic_interrupt_ids() as it doesn't hurt to set it
multiple times.   IIRC, the pre-cpuset code kept a separate count which is
why that would have been harmful.

-- 
John Baldwin