Date: Mon, 16 May 2011 16:09:21 -0400 From: John Baldwin <jhb@freebsd.org> To: Andriy Gapon <avg@freebsd.org> Cc: Max Laier <max@love2party.net>, FreeBSD current <freebsd-current@freebsd.org>, neel@freebsd.org, Peter Grehan <grehan@freebsd.org> Subject: Re: proposed smp_rendezvous change Message-ID: <201105161609.21898.jhb@freebsd.org> In-Reply-To: <4DD17AB3.1070606@FreeBSD.org> References: <4DCD357D.6000109@FreeBSD.org> <201105161421.27665.jhb@freebsd.org> <4DD17AB3.1070606@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Monday, May 16, 2011 3:27:47 pm Andriy Gapon wrote:
> on 16/05/2011 21:21 John Baldwin said the following:
> > How about this:
> ...
> > /*
> > * Shared mutex to restrict busywaits between smp_rendezvous() and
> > @@ -311,39 +312,62 @@ restart_cpus(cpumask_t map)
> > void
> > smp_rendezvous_action(void)
> > {
> > - void* local_func_arg = smp_rv_func_arg;
> > - void (*local_setup_func)(void*) = smp_rv_setup_func;
> > - void (*local_action_func)(void*) = smp_rv_action_func;
> > - void (*local_teardown_func)(void*) = smp_rv_teardown_func;
> > + void *local_func_arg;
> > + void (*local_setup_func)(void*);
> > + void (*local_action_func)(void*);
> > + void (*local_teardown_func)(void*);
> > + int generation;
> >
> > /* Ensure we have up-to-date values. */
> > atomic_add_acq_int(&smp_rv_waiters[0], 1);
> > while (smp_rv_waiters[0] < smp_rv_ncpus)
> > cpu_spinwait();
> >
> > - /* setup function */
> > + /* Fetch rendezvous parameters after acquire barrier. */
> > + local_func_arg = smp_rv_func_arg;
> > + local_setup_func = smp_rv_setup_func;
> > + local_action_func = smp_rv_action_func;
> > + local_teardown_func = smp_rv_teardown_func;
>
> I want to ask once again - please pretty please convince me that the above
> cpu_spinwait() loop is really needed and, by extension, that the assignments
> should be moved behind it.
> Please :)
Well, moving the assignments down is a style fix for one, and we can always
remove the first rendezvous as a follow up if desired.
However, suppose you have an arch where sending an IPI is not a barrier
(this seems unlikely). In that case, the atomic_add_acq_int() will not
succeed (and return) until it has seen the earlier write by the CPU
initiating the rendezvous to clear smp_rv_waiters[0] to zero. The actual
spin on the smp_rv_waiters[] value is not strictly necessary however and
is probably just cut and pasted to match the other uses of values in
the smp_rv_waiters[] array.
(atomic_add_acq_int() could spin on architectures where it is implemented
using compare-and-swap (e.g. sparc64) or locked-load conditional-store (e.g.
Alpha).)
--
John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201105161609.21898.jhb>