Question about cv_signal(9) (never mind)

[John Baldwin]

On Monday 14 June 2004 02:53 pm, John Polstra wrote:
> On 14-Jun-2004 John Baldwin wrote:
> > On Saturday 12 June 2004 08:12 pm, John Polstra wrote:
> >> On 12-Jun-2004 John Polstra wrote:
> >> > [Why does a caller to cv_signal(9) have to hold the associated mutex?]
> >>
> >> Never mind.  I understand now.  It allows the implementation to
> >> avoid doing any locking internally.  That seems perfectly
> >> reasonable, and I withdraw my question.
> >
> > To be honest, it's also largely there to try to keep people from writing
> > code that can lose wakeups.  The count optimization came later.  If the
> > optimization of dropping the lock is more important and we think that
> > people really won't make the mistake of not using locks when they should
> > to avoid the lost wakeups then we could drop the count optimization and
> > allow cv_signal() to not require the lock perhaps.
>
> It seems to me that some sort of mutual exclusion is needed when
> manipulating the cv structure and the associated sleep queue.  If
> the user doesn't provide it then the implementation will have to
> provide it internally.  So unless there is a cheaper kind of mutual
> exclusion that can be used inside the implementation, your current
> solution seems the best to me.
>
> I had never thought through the need for some kind of mutual
> exclusion, and the papers that describe the optimization I mentioned
> simply ignore it and its performance implications.

Well, the sleep queue is not protected by the data lock, it is protected by a 
spin lock associated with the hash table bucket that the condition variable's 
address hashes to.  Specifically, note that sleepq_lookup() locks the 
associated spin lock and returns with it locked where as sleepq_release(), 
etc. unlocks the spin lock.  The sleepq is locked inside sleepq_signal() and 
sleepq_wakeup().  If you don't implement the count optimization, then 
cv_signal() just calls sleepq_signal() and cv_wakeup() just calls 
sleepq_wakeup() with no need for any locking of the cv structure itself.  See 
rev 1.47 of kern_condvar.c and the current implementations of wakeup() and 
wakeup_one() for example.  The other member of struct condvar is static in 
that it is only set during initialization and doesn't need any locking.

-- 
John Baldwin <jhb at FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve"  =  http://www.FreeBSD.org