Date: Mon, 25 May 2015 10:31:08 -0400 From: John Baldwin <jhb@freebsd.org> To: Andrew Turner <andrew@fubar.geek.nz> Cc: Andrew Turner <andrew@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r283331 - head/sys/arm/arm Message-ID: <3083392.nvUXfWWOav@ralph.baldwin.cx> In-Reply-To: <20150525132148.3d5adb40@bender.Home> References: <201505232228.t4NMSxs2032365@svn.freebsd.org> <1484557.5zjnsBffEc@ralph.baldwin.cx> <20150525132148.3d5adb40@bender.Home>
next in thread | previous in thread | raw e-mail | index | archive | help
On Monday, May 25, 2015 01:21:48 PM Andrew Turner wrote: > On Mon, 25 May 2015 07:23:28 -0400 > John Baldwin <jhb@freebsd.org> 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?) -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3083392.nvUXfWWOav>