Date: Tue, 10 Apr 2012 13:41:18 +0200 From: Marius Strobl <marius@alchemy.franken.de> To: Attilio Rao <attilio@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, John Baldwin <jhb@freebsd.org> Subject: Re: svn commit: r234074 - in head/sys: amd64/amd64 i386/i386 Message-ID: <20120410114118.GB93449@alchemy.franken.de> In-Reply-To: <CAJ-FndDzh50_Xb%2B08EnhAVBnu1vFLo0d5MX%2BQwAOTcq_ewwDJQ@mail.gmail.com> References: <201204092241.q39MfJZn081610@svn.freebsd.org> <20120409230949.GB68111@alchemy.franken.de> <CAJ-FndDzh50_Xb%2B08EnhAVBnu1vFLo0d5MX%2BQwAOTcq_ewwDJQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Apr 10, 2012 at 01:03:56AM +0100, Attilio Rao wrote: > Il 10 aprile 2012 00:09, Marius Strobl <marius@alchemy.franken.de> ha scritto: > > On Mon, Apr 09, 2012 at 10:41:19PM +0000, Attilio Rao wrote: > >> Author: attilio > >> Date: Mon Apr ??9 22:41:19 2012 > >> New Revision: 234074 > >> URL: http://svn.freebsd.org/changeset/base/234074 > >> > >> Log: > >> ?? BSP is not added to the mask of valid target CPUs for interrupts > >> ?? in set_apic_interrupt_ids(). Besides, set_apic_interrupts_ids() is not > >> ?? called in the !SMP case too. > >> ?? Fix this by: > >> ?? - Adding the BSP as an interrupt target directly in cpu_startup(). > >> ?? - Remove an obsolete optimization where the BSP are skipped in > >> ?? ?? set_apic_interrupt_ids(). > >> > >> ?? Reported by: ?? ?? ?? ??jh > >> ?? Reviewed by: ?? ?? ?? ??jhb > >> ?? MFC after: ??3 days > >> ?? X-MFC: ?? ?? ?? ?? ?? ?? ??r233961 > >> ?? Pointy hat to: ?? ?? ??me > >> > >> Modified: > >> ?? head/sys/amd64/amd64/machdep.c > >> ?? head/sys/amd64/amd64/mp_machdep.c > >> ?? head/sys/i386/i386/machdep.c > >> ?? head/sys/i386/i386/mp_machdep.c > >> > >> Modified: head/sys/amd64/amd64/machdep.c > >> ============================================================================== > >> --- head/sys/amd64/amd64/machdep.c ?? ??Mon Apr ??9 22:01:43 2012 ?? ?? ?? ??(r234073) > >> +++ head/sys/amd64/amd64/machdep.c ?? ??Mon Apr ??9 22:41:19 2012 ?? ?? ?? ??(r234074) > >> @@ -295,6 +295,11 @@ cpu_startup(dummy) > >> ?? ?? ?? vm_pager_bufferinit(); > >> > >> ?? ?? ?? cpu_setregs(); > >> + > >> + ?? ?? /* > >> + ?? ?? ??* Add BSP as an interrupt target. > >> + ?? ?? ??*/ > >> + ?? ?? intr_add_cpu(0); > >> ??} > > > > If I'm not mistaken, intr_add_cpu() is under #ifdef SMP, so it should be > > here as well. > > You are right, sorry, I did forgot to test without SMP. > I think we still need intr_add_cpu() on cpu_startup() because of the > case smp_disabled = 1. > I think the attached patch should make its dirty job, opinion? I currently fail to see why the latter approach would be necessary, i.e. IMO wrapping the intr_add_cpu() calls in cpu_startup() should be sufficient. In case the kernel is compiled without SMP support, interrupt balancing support isn't available in the first place and the BSP is always the only available target (see the UP version of intr_next_cpu() at the end of x86/x86/intr_machdep.c), so there's no need to add the BSP as a valid target. If an SMP kernel is run on a UP machine or with SMP disabled, interrupt balancing support is available but the intr_add_cpu() calls in cpu_startup() will add the BSP as (the only) target, so everything should be fine. Maybe you can elaborate on why you think an SMP kernel with SMP disabled needs special handling. Marius
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120410114118.GB93449>