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
_______________________________________________
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
