From owner-svn-src-head@FreeBSD.ORG Mon May 25 15:09:33 2015 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 005D0109 for ; Mon, 25 May 2015 15:09:32 +0000 (UTC) (envelope-from ian@freebsd.org) Received: from outbound3.ore.mailhop.org (erouter6.ore.mailhop.org [54.187.213.119]) by mx1.freebsd.org (Postfix) with SMTP id D175994A for ; Mon, 25 May 2015 15:09:32 +0000 (UTC) (envelope-from ian@freebsd.org) Received: from ilsoft.org (unknown [73.34.117.227]) by outbound3.ore.mailhop.org (Halon Mail Gateway) with ESMTPSA; Mon, 25 May 2015 15:09:00 +0000 (UTC) Received: from revolution.hippie.lan (revolution.hippie.lan [172.22.42.240]) by ilsoft.org (8.14.9/8.14.9) with ESMTP id t4PF9QPl010656; Mon, 25 May 2015 09:09:26 -0600 (MDT) (envelope-from ian@freebsd.org) Message-ID: <1432566566.1200.37.camel@freebsd.org> Subject: Re: svn commit: r283331 - head/sys/arm/arm From: Ian Lepore To: John Baldwin Cc: Andrew Turner , Andrew Turner , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Date: Mon, 25 May 2015 09:09:26 -0600 In-Reply-To: <3083392.nvUXfWWOav@ralph.baldwin.cx> References: <201505232228.t4NMSxs2032365@svn.freebsd.org> <1484557.5zjnsBffEc@ralph.baldwin.cx> <20150525132148.3d5adb40@bender.Home> <3083392.nvUXfWWOav@ralph.baldwin.cx> Content-Type: text/plain; charset="us-ascii" X-Mailer: Evolution 3.12.10 FreeBSD GNOME Team Port Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 25 May 2015 15:09:33 -0000 On Mon, 2015-05-25 at 10:31 -0400, John Baldwin wrote: > On Monday, May 25, 2015 01:21:48 PM Andrew Turner wrote: > > On Mon, 25 May 2015 07:23:28 -0400 > > John Baldwin wrote: > > > > > On Saturday, May 23, 2015 10:28:59 PM Andrew Turner wrote: > > > > Author: andrew > > > > Date: Sat May 23 22:28:59 2015 > > > > New Revision: 283331 > > > > URL: https://svnweb.freebsd.org/changeset/base/283331 > > > > > > > > Log: > > > > Use the wait-for-event instruction to put the core we have just > > > > enabled to sleep while it waits to start scheduling. The boot core > > > > can then use the send-event instruction to wake the cores when they > > > > should enter the scheduler. > > > > > > > > MFC after: 1 week > > > > > > > > Modified: > > > > head/sys/arm/arm/mp_machdep.c > > > > > > > > Modified: head/sys/arm/arm/mp_machdep.c > > > > ============================================================================== > > > > --- head/sys/arm/arm/mp_machdep.c Sat May 23 21:58:41 > > > > 2015 (r283330) +++ head/sys/arm/arm/mp_machdep.c Sat > > > > May 23 22:28:59 2015 (r283331) @@ -185,8 +185,11 @@ > > > > init_secondary(int cpu) atomic_add_rel_32(&mp_naps, 1); > > > > > > > > /* Spin until the BSP releases the APs */ > > > > - while (!aps_ready) > > > > - ; > > > > + while (!atomic_load_acq_int(&aps_ready)) { > > > > +#if __ARM_ARCH >= 7 > > > > + __asm __volatile("wfe"); > > > > +#endif > > > > + } > > > > > > I don't know that this atomic load acquire is really changing > > > anything here? Since aps_ready is volatile reading it should > > > already be "atomic" on each check around the loop. > > > > It's also adding acquire semantics to ensure we don't > > incorrectly reorder memory operations across the call. > > I think the _rel barrier on the update to mp_naps above probably > already does that, but ok. > > > > > /* Initialize curthread */ > > > > KASSERT(PCPU_GET(idlethread) != NULL, ("no idle thread")); > > > > @@ -353,6 +356,10 @@ release_aps(void *dummy __unused) > > > > arm_unmask_irq(i); > > > > } > > > > atomic_store_rel_int(&aps_ready, 1); > > > > + /* Wake the other threads up */ > > > > +#if __ARM_ARCH >= 7 > > > > + armv7_sev(); > > > > +#endif > > > > > > So I'm not at all familiar with these instructions or what they do, > > > but are the events level triggered? In particular, is there any > > > sort of race where the sev might arrive in between the check of > > > aps_ready and the wfe on an AP? (For example, if wfe/sev were > > > similar to using mwait on x86 for wfe and a memory write for sev, > > > x86 would require a call to monitor before doing a check of > > > aps_ready to handle the race like so: > > > > > > while (!aps_ready) { > > > monitor(&aps_ready); > > > if (!aps_ready) > > > mwait(); > > > } > > > > > > > The armv7_sev function includes a barrier to ensure any previous memory > > operations have been flushed to cache before we send the event. The sev > > instruction then sets the event register in every processor. > > > > The wfe instruction will check this event register and, if it is unset, > > it the processor can then enter a low power mode. From my reading of > > the documentation, if an event has been signalled before executing the > > wfe then the instruction is a nop so will exit the loop as the new > > value of aps_ready will be visible on all processors. > > Mmmm, does that mean then that you can (conceivably) lose the race the other > way where it "sees" ap_ready's update before it calls wfe and never calls > wfe to "harvest" the event from sev? (In practice I think this is not > possible during boot as AP's can't get preempted and there is typically > a "long" time between AP's being signalled to start and start_aps being > set. However, this would be a concern for use of wfe/sev for other use > cases such as for the cpu_idle hook perhaps?) > That's the "you must be prepared to handle spurious wakeups" part of the sev/wfe contract. The point of WFE is only power-saving, so if your loop spins one time due to an unharvested prior event flag still set, that's deemed harmless. (Userland is allowed to issue SEV instructions, which always target all cores, so there's no expectation of 1:1 relation between sending and waiting.) -- Ian