svn commit: r282971 - in head/sys: kern sys

[John Baldwin]

On Friday, May 15, 2015 01:50:38 PM John Baldwin wrote:
> Author: jhb
> Date: Fri May 15 13:50:37 2015
> New Revision: 282971
> URL: https://svnweb.freebsd.org/changeset/base/282971
> 
> Log:
>   Previously, cv_waiters was only updated by cv_signal or cv_wait. If a
>   thread awakened due to a time out, then cv_waiters was not decremented.
>   If INT_MAX threads timed out on a cv without an intervening cv_broadcast,
>   then cv_waiters could overflow. To fix this, have each sleeping thread
>   decrement cv_waiters when it resumes.
>   
>   Note that previously cv_waiters was protected by the sleepq chain lock.
>   However, that lock is not held when threads resume from sleep. In
>   addition, the interlock is also not always reacquired after resuming
>   (cv_wait_unlock), nor is it always held by callers of cv_signal() or
>   cv_broadcast(). Instead, use atomic ops to update cv_waiters. Since
>   the sleepq chain lock is still held on every increment, it should
>   still be safe to compare cv_waiters against zero while holding the
>   lock in the wakeup routines as the only way the race should be lost
>   would result in extra calls to sleepq_signal() or sleepq_broadcast().
>   
>   Differential Revision:	https://reviews.freebsd.org/D2427
>   Reviewed by:	benno
>   Reported by:	benno (wrap of cv_waiters in the field)
>   MFC after:	2 weeks

With the additional overhead of the atomic ops it might be worth running
some benchmarks to compare this with removing cv_waiters entirely.  The
theoretical gain from cv_waiters is avoiding looking in the hash table for
a matching sleepqueue when there are no waiters.  When cv_waiters was
a simple integer the cost of having it around was very small, so even a
tiny gain was worth having.  (It is worth noting that in pre-SMPng code it
was fairly common practice to keep a "WANTED" flag around that was set by
waiters and would result in wakeup() being skipped if it wasn't set.  These
flags were maintained by each caller, not centrally.  cv_waiters makes this
sort of thing centrally maintained rather than something that each caller
has to do.)  Now that cv_waiters is updated with atomics, the cost is not
quite as small.

-- 
John Baldwin