From owner-cvs-all@FreeBSD.ORG Fri Sep 21 19:27:30 2007 Return-Path: Delivered-To: cvs-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A22A516A46B; Fri, 21 Sep 2007 19:27:30 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from speedfactory.net (mail6.speedfactory.net [66.23.216.219]) by mx1.freebsd.org (Postfix) with ESMTP id 1042113C465; Fri, 21 Sep 2007 19:27:29 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from server.baldwin.cx (unverified [66.23.211.162]) by speedfactory.net (SurgeMail 3.8p) with ESMTP id 210657247-1834499 for multiple; Fri, 21 Sep 2007 15:25:48 -0400 Received: from localhost.corp.yahoo.com (john@localhost [127.0.0.1]) (authenticated bits=0) by server.baldwin.cx (8.13.8/8.13.8) with ESMTP id l8LJRAHb063226; Fri, 21 Sep 2007 15:27:14 -0400 (EDT) (envelope-from jhb@freebsd.org) From: John Baldwin To: Attilio Rao Date: Fri, 21 Sep 2007 14:36:14 -0400 User-Agent: KMail/1.9.6 References: <200709112254.l8BMsB7P074637@repoman.freebsd.org> In-Reply-To: <200709112254.l8BMsB7P074637@repoman.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200709211436.15444.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH authentication, not delayed by milter-greylist-2.0.2 (server.baldwin.cx [127.0.0.1]); Fri, 21 Sep 2007 15:27:14 -0400 (EDT) X-Virus-Scanned: ClamAV 0.88.3/4357/Fri Sep 21 05:55:46 2007 on server.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-4.4 required=4.2 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.1.3 X-Spam-Checker-Version: SpamAssassin 3.1.3 (2006-06-01) on server.baldwin.cx Cc: cvs-src@freebsd.org, src-committers@freebsd.org, cvs-all@freebsd.org Subject: Re: cvs commit: src/sys/amd64/amd64 local_apic.c src/sys/i386/acpica madt.c src/sys/i386/i386 local_apic.c src/sys/kern subr_smp.c src/sys/sun4v/mdesc mdesc_init.c X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 21 Sep 2007 19:27:30 -0000 On Tuesday 11 September 2007 06:54:09 pm Attilio Rao wrote: > attilio 2007-09-11 22:54:09 UTC > > FreeBSD src repository > > Modified files: > sys/amd64/amd64 local_apic.c > sys/i386/acpica madt.c > sys/i386/i386 local_apic.c > sys/kern subr_smp.c > sys/sun4v/mdesc mdesc_init.c > Log: > This is a follow-up, cleaning-up commit about recent changes involving > topology foo functions. > Working at the patch for topology problems in ia32/amd64 evicted some > problems regarding functions ordering in the SI_SUB_CPU family of > SYSINIT'ed subsystems. > In order to avoid problems with new modified to involved functions, a > correct ordering is not semantically specified for SI_SUB_CPU functions > (for a larger view of the issue please visit: > http://lists.freebsd.org/pipermail/freebsd-current/2007-July/075409.html ) Could you clarify exactly what ordering this does? AFAICT, nothing in cpu_startup() requires the APIC to be up and running. If there were a dependency, then that would be something for the MD architecture code to fix. IIUC, the problem was that you had 3 sysinit functions like this: A - SI_ORDER_FIRST, cpu_startup() B - SI_ORDER_FIRST, apic_init() C - SI_ORDER_SECOND, mp_start() Now mp_start() requires both A and B to have run on x86. What Peter did originally was to move B to SI_ORDER_SECOND because he found that B needed A to run. That broke because C could end up running before B. However, the actual patch that went into CVS left A and B as is and instead made them independent of each other so B no longer depends on A. So, I don't think you've solved an actual problem. Conceptually the way SI_SUB_CPU worked before was that the MD code had SI_ORDER_FIRST to get ready before it was asked to start SMP at SI_ORDER_SECOND. Now the MD code gets SI_ORDER_FIRST and _SECOND even though it doesn't need it. You should at least put the x86 apic routines back to SI_ORDER_FIRST becuase they do _not_ depend on cpu_startup(). The madt.c change in this commit is plain wrong however and should be reverted. If it was correct it would have needed to be done in amd64's madt.c as well. The sun4v change is bogus as well as mdesc_init() doesn't depend on cpu_startup() on sun4v at all, and it doesn't matter what order they run in. Basically, I think at the least you should revert all the MD changes, and the change to subr_smp.c is basically a NOP. -- John Baldwin