Date: Mon, 9 Apr 2012 17:58:07 +0100 From: Attilio Rao <attilio@freebsd.org> To: John Baldwin <jhb@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: <CAJ-FndB_oWjd3VsJ%2BD9p3987MdfCXuAUHXX-mkjLmmRnnTireQ@mail.gmail.com> In-Reply-To: <201204091234.43106.jhb@freebsd.org> References: <201204062119.q36LJTKR026564@svn.freebsd.org> <20120409154510.GA2253@a91-153-116-96.elisa-laajakaista.fi> <201204091234.43106.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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: >> > =C2=A0 Fix interrupt load balancing regression, introduced in revision >> > =C2=A0 222813, that left all un-pinned interrupts assigned to CPU 0. >> > >> > =C2=A0 sys/x86/x86/intr_machdep.c: >> > =C2=A0 =C2=A0 In intr_shuffle_irqs(), remove CPU_SETOF() call that ini= tialized >> > =C2=A0 =C2=A0 the "intr_cpus" cpuset to only contain CPU0. >> > >> > =C2=A0 =C2=A0 This initialization is too late and nullifies the result= s of calls >> > =C2=A0 =C2=A0 the intr_add_cpu() that occur much earlier in the boot p= rocess. >> > =C2=A0 =C2=A0 Since "intr_cpus" is statically initialized to the empty= set, and >> > =C2=A0 =C2=A0 all processors, including the BSP, already add themselve= s to >> > =C2=A0 =C2=A0 "intr_cpus" no special initialization for the BSP is nec= essary. >> >> 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: >> =C2=A0 =C2=A0 =C2=A0ID: 0x01000000 =C2=A0 VER: 0x00050014 LDR: 0x0000000= 0 DFR: 0xffffffff >> =C2=A0 lint0: 0x00010700 lint1: 0x00000400 TPR: 0x00000000 SVR: 0x000001= ff >> =C2=A0 timer: 0x000100ef therm: 0x00010000 err: 0x000000f0 pmc: 0x000104= 00 >> >> The system boots with r233960. >> >> Some information: >> >> CPU: Intel(R) Pentium(R) 4 CPU 2.60GHz (2605.96-MHz 686-class CPU) >> =C2=A0 Origin =3D "GenuineIntel" =C2=A0Id =3D 0xf29 =C2=A0Family =3D f = =C2=A0Model =3D 2 =C2=A0Stepping =3D >> 9 >> > Features=3D0xbfebfbff<FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PG= E,MCA,CMOV,PAT,PSE36,CLFLUSH,DTS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE> >> =C2=A0 Features2=3D0x4400<CNXT-ID,xTPR> >> real memory =C2=A0=3D 2147483648 (2048 MB) >> avail memory =3D 2085228544 (1988 MB) >> Event timer "LAPIC" quality 400 >> ACPI APIC Table: <A M I =C2=A0OEMAPIC > >> FreeBSD/SMP: Multiprocessor System Detected: 2 CPUs >> FreeBSD/SMP: 1 package(s) x 1 core(s) x 2 HTT threads >> =C2=A0cpu0 (BSP): APIC ID: =C2=A00 >> =C2=A0cpu1 (AP/HT): APIC ID: =C2=A01 > > I suspect in your case intr_add_cpu() is never called. =C2=A0I think Atti= lio 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(). =C2=A0This= used to > work because bit 0 was assigned statically. =C2=A0Also, 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. Thanks, Attilio Index: sys/i386/i386/mp_machdep.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- sys/i386/i386/mp_machdep.c (revisione 234046) +++ sys/i386/i386/mp_machdep.c (copia locale) @@ -831,6 +831,8 @@ set_interrupt_apic_ids(void) apic_id =3D cpu_apic_ids[i]; if (apic_id =3D=3D -1) continue; + + /* BSP should be already added by cpu_startup(). */ if (cpu_info[apic_id].cpu_bsp) continue; if (cpu_info[apic_id].cpu_disabled) Index: sys/i386/i386/machdep.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- sys/i386/i386/machdep.c (revisione 234046) +++ sys/i386/i386/machdep.c (copia locale) @@ -336,6 +336,11 @@ cpu_startup(dummy) #ifndef XEN cpu_setregs(); #endif + + /* + * Add BSP interrupt bitmask. + */ + intr_add_cpu(0); } /* Index: sys/amd64/amd64/mp_machdep.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- sys/amd64/amd64/mp_machdep.c (revisione 234046) +++ sys/amd64/amd64/mp_machdep.c (copia locale) @@ -797,6 +797,8 @@ set_interrupt_apic_ids(void) apic_id =3D cpu_apic_ids[i]; if (apic_id =3D=3D -1) continue; + + /* BSP should be already added by cpu_startup(). */ if (cpu_info[apic_id].cpu_bsp) continue; if (cpu_info[apic_id].cpu_disabled) Index: sys/amd64/amd64/machdep.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- sys/amd64/amd64/machdep.c (revisione 234046) +++ sys/amd64/amd64/machdep.c (copia locale) @@ -295,6 +295,11 @@ cpu_startup(dummy) vm_pager_bufferinit(); cpu_setregs(); + + /* + * Add BSP interrupt bitmask. + */ + intr_add_cpu(0); } /*
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-FndB_oWjd3VsJ%2BD9p3987MdfCXuAUHXX-mkjLmmRnnTireQ>