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