From owner-svn-src-all@FreeBSD.ORG Tue Apr 10 13:25:48 2012 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B829E106564A; Tue, 10 Apr 2012 13:25:48 +0000 (UTC) (envelope-from marius@alchemy.franken.de) Received: from alchemy.franken.de (alchemy.franken.de [194.94.249.214]) by mx1.freebsd.org (Postfix) with ESMTP id 5F05B8FC12; Tue, 10 Apr 2012 13:25:48 +0000 (UTC) Received: from alchemy.franken.de (localhost [127.0.0.1]) by alchemy.franken.de (8.14.4/8.14.4/ALCHEMY.FRANKEN.DE) with ESMTP id q3ADPkl7032854; Tue, 10 Apr 2012 15:25:47 +0200 (CEST) (envelope-from marius@alchemy.franken.de) Received: (from marius@localhost) by alchemy.franken.de (8.14.4/8.14.4/Submit) id q3ADPkv6032853; Tue, 10 Apr 2012 15:25:46 +0200 (CEST) (envelope-from marius) Date: Tue, 10 Apr 2012 15:25:46 +0200 From: Marius Strobl To: Attilio Rao Message-ID: <20120410132546.GF93449@alchemy.franken.de> References: <201204092241.q39MfJZn081610@svn.freebsd.org> <20120409230949.GB68111@alchemy.franken.de> <20120410114118.GB93449@alchemy.franken.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.3i Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, John Baldwin Subject: Re: svn commit: r234074 - in head/sys: amd64/amd64 i386/i386 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 10 Apr 2012 13:25:48 -0000 On Tue, Apr 10, 2012 at 01:55:31PM +0100, Attilio Rao wrote: > Il 10 aprile 2012 12:41, Marius Strobl ha scritto: > > On Tue, Apr 10, 2012 at 01:03:56AM +0100, Attilio Rao wrote: > >> Il 10 aprile 2012 00:09, Marius Strobl 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. > > I do not understand what you mean. Well, so we are at least in the same boat :) > Right now there is a compile time issue where for !SMP kernel it won't > compile and this is what I'm trying to fix now, so I don't understand > what do you mean here. > AFAICT the below patch should take care of UP in both the compile-time and run-time cases. Marius Index: amd64/amd64/machdep.c =================================================================== --- amd64/amd64/machdep.c (revision 234095) +++ amd64/amd64/machdep.c (working copy) @@ -296,10 +296,12 @@ cpu_startup(dummy) cpu_setregs(); +#ifdef SMP /* * Add BSP as an interrupt target. */ intr_add_cpu(0); +#endif } /* Index: i386/i386/machdep.c =================================================================== --- i386/i386/machdep.c (revision 234095) +++ i386/i386/machdep.c (working copy) @@ -337,10 +337,12 @@ cpu_startup(dummy) cpu_setregs(); #endif +#ifdef SMP /* * Add BSP as an interrupt target. */ intr_add_cpu(0); +#endif } /*