Re: proposed smp_rendezvous change

I'd like to propose that we move forward with fixing the race, before 
redesigning the API - please.

We agree that there is a problem with the exit rendezvous.  Let's fix that.  
We agree that the generation approach proposed by NetAPP is the right way to 
go?  Can we check it in, then?  Again, I'd like some comments in the code if 
at all possible.

A second commit should take care of the entry sync - which may or may not be 
required on some architectures.  If we decide that we don't need it, we should 
remove it.  Otherwise, we must move all the assignments from global smp_rv_* 
to the stack in smp_rendezvous_action to after the sync.  Otherwise we should 
just kill it.  In any case, it would be nice to clean this up.

After that, I have one more bugfix for rmlock(9) - one of the three consumers 
of this API (that I am aware of).  As it uses a spinlock inside its IPI 
action, we have to mark smp_rendezvous_action a critical section otherwise the 
spin unlock might yield ... with the expected consequences.

It is arguable if you should be allowed to use spinlocks in the rendevous at 
all, but that's beside the point.

On Monday 16 May 2011 11:52:10 John Baldwin wrote:
> On Sunday, May 15, 2011 12:36:45 pm Andriy Gapon wrote:
...
> As we chatted about on IRC, I think we should err on the side of making
> this API less complex (so that all of the actions for a given rendezvous
> finish before the next rendezvous begins) as this stuff is already fairly
> complex. We should only add more complexity if we really need it.

+1

> > BTW, I don't think that we ever use smp_rendezvous() in its full glory,
> > i.e. with non-trivial setup, action and teardown (except perhaps for
> > jkim's stress test).  OpenSolaris seems to be doing just fine with the
> > following three types of simpler CPU cross-calls (as they call them):
> > 1. async - a master CPU requests an action to be executed on target CPUs
> > but doesn't wait for anything
> > - we have no explicit equivalent for this

I'd like to see something like this.  Otherwise we end up with hacks like 
callouts that sched_bind and stuff like that.  I can think of plenty use cases 
for this API.

> > 2. call - a master CPU requests an action to be executed on target CPUs
> > and waits until all the target CPUs have finished executing it
> > - this would be equivalent smp_rendezvous_cpus(no_barrier, action,
> > no_barrier)
> > 
> > 3. synch - a master CPU requests an action to be executed on target CPUs
> > and waits until all the target CPUs have finished executing it and the
> > slave CPUs are also waiting for their siblings to be done
> > - this would be equivalent smp_rendezvous_cpus(no_barrier, action, NULL)
> > 
> > Seems that our current code doesn't use/need more than that as well.
> > But I really like the capability that smp_rendezvous_cpus() potentially
> > provides.
> 
> I would be fine with stripping down our rendezvous API a bit.  They could
> all still use a single IPI and handler but perhaps prevent a few wrapper
> APIs that provide simpler semantics like this.  Back to my statement
> above, we've had this uber-generic rendezvous API for a while, but if we
> don't really need it to be quite so complex I'd be happy to dumb down the
> API to what we really need so the implementation and use can be simpler.

Again, agreed.