Date: Mon, 9 Apr 2012 13:14:20 -0400 From: John Baldwin <jhb@freebsd.org> To: Attilio Rao <attilio@freebsd.org> Cc: svn-src-head@freebsd.org, Jaakko Heinonen <jh@freebsd.org>, svn-src-all@freebsd.org, "Justin T. Gibbs" <gibbs@freebsd.org>, src-committers@freebsd.org Subject: Re: svn commit: r233961 - head/sys/x86/x86 Message-ID: <201204091314.20775.jhb@freebsd.org> In-Reply-To: <CAJ-FndB_oWjd3VsJ%2BD9p3987MdfCXuAUHXX-mkjLmmRnnTireQ@mail.gmail.com> References: <201204062119.q36LJTKR026564@svn.freebsd.org> <201204091234.43106.jhb@freebsd.org> <CAJ-FndB_oWjd3VsJ%2BD9p3987MdfCXuAUHXX-mkjLmmRnnTireQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Monday, April 09, 2012 12:58:07 pm Attilio Rao wrote: > Il 09 aprile 2012 17:34, John Baldwin <jhb@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201204091314.20775.jhb>