cvs commit: src/sys/vm vm_zeroidle.c

[Daniel Eischen]

On Wed, 10 Nov 2004, John Baldwin wrote:

> On Monday 08 November 2004 05:33 pm, David Schultz wrote:
> > On Mon, Nov 08, 2004, John Baldwin wrote:
> > > It is no longer required to hold the mutex over cv_wait() and
> > > cv_signal().  I intentionally changed that so that you can do:
> > >
> > > 	lock()
> > > 	blah()
> > > 	unlock()
> > > 	cv_signal()
> > >
> > > and reduce the number of context switches if you preempt in cv_signal().
> >
> > Hmm...I agree with Alfred that allowing this is a bad idea.  By
> > permitting this, you're adding two additional mutex operations to
> > the critical path in order to avoid an inefficiency that will
> > almost never occur.
>
> Actually, it would always occur on a UP system if you preempt in the
> signal/broadcast.  FWIW, I've specifically had other people ask for this
> "feature".  Note that this also now allows you to do things like
> 'cv_signal()' from a fast interrupt handler if needbe.
>
[ ... ]
> > The original formulation of this kind of condition variable was in
> > Mesa[1], which requires the lock to be held during cv_signal()
> > specifically for efficiency.[2]  Solaris also requires the mutex to
> > be held across cv_signal().  PThreads is the only API I know of to
> > have it the other way around.
> >
> >
> > [1] http://research.microsoft.com/Lampson/23-ProcessesInMesa/WebPage.html
> >
> > [2] It supported a second, less efficient type of CV that would
> >     allow device microcode to signal device drivers without
> >     holding the mutex, but all other CVs required the mutex to be
> >     held when the CV was signalled.  But this second kind of CV
> >     is irrelevant today.
>
> Well, it is easy enough to back out if the differering opinions on the subject
> can reach a consensus.  There was a discussion on smp@ a while back in favor
> of allowing cv_signal/broadcast to not require the mutex to be held there
> earlier.

I think the reason Solaris can enforce that you hold the mutex around
cv_signal() and cv_broadcast() is that it knows if the mutex is used
by an interrupt handler.  You must initialize mutexes with the registered
interrupt iblock cookie if they are going to be used by an interrupt
handler, so the system can handle these mutexes specially when non-ISR
code holds them.  I think this allows the ISR to take the mutex without
suffering the penalty of blocking when the mutex is locked.

Since there is no similar knowledge in our mutexes/CVs, I think it
makes sense to allow cv_signal() and cv_broadcast() without holding
the lock (similar to sem_post() being signal safe).

-- 
DE