Date: Fri, 31 Dec 2004 14:46:49 GMT From: David Xu <davidxu@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 67972 for review Message-ID: <200412311446.iBVEkn9O008840@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=67972 Change 67972 by davidxu@davidxu_tiger on 2004/12/31 14:45:48 Rewrite conditional variable code, full cancellation support, this improves performance about 30%. Affected files ... .. //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_cond.c#5 edit Differences ... ==== //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_cond.c#5 (text+ko) ==== @@ -1,36 +1,31 @@ /* - * Copyright (c) 1995 John Birrell <jb@cimlogic.com.au>. + * Copyright (c) 2005, David Xu <davidxu@freebsd.org> * All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions * are met: * 1. Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. + * notice unmodified, this list of conditions, and the following + * disclaimer. * 2. Redistributions in binary form must reproduce the above copyright * notice, this list of conditions and the following disclaimer in the * documentation and/or other materials provided with the distribution. - * 3. All advertising materials mentioning features or use of this software - * must display the following acknowledgement: - * This product includes software developed by John Birrell. - * 4. Neither the name of the author nor the names of any co-contributors - * may be used to endorse or promote products derived from this software - * without specific prior written permission. * - * THIS SOFTWARE IS PROVIDED BY JOHN BIRRELL AND CONTRIBUTORS ``AS IS'' AND - * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE - * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL - * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS - * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) - * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT - * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY - * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF - * SUCH DAMAGE. + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. * - * $FreeBSD: src/lib/libpthread/thread/thr_cond.c,v 1.51 2003/12/09 02:20:56 davidxu Exp $ + * $FreeBSD$ */ + #include <stdlib.h> #include <errno.h> #include <string.h> @@ -42,12 +37,9 @@ /* * Prototypes */ -static inline void check_continuation(struct pthread *, - struct pthread_cond *, pthread_mutex_t *); -static int init_static(struct pthread *thread, - pthread_cond_t *cond); +static int init_static(struct pthread *thread, pthread_cond_t *cond); static int cond_wait_common(pthread_cond_t *cond, pthread_mutex_t *mutex, - const struct timespec *abstime); + const struct timespec *abstime, int cancel); static int cond_signal_common(pthread_cond_t *cond, int broadcast); /* @@ -63,7 +55,6 @@ __weak_reference(_pthread_cond_signal, pthread_cond_signal); __weak_reference(_pthread_cond_broadcast, pthread_cond_broadcast); - int _pthread_cond_init(pthread_cond_t *cond, const pthread_condattr_t *cond_attr) { @@ -81,7 +72,9 @@ * Initialise the condition variable structure: */ umtx_init(&pcond->c_lock); - pcond->c_count = 0; + pcond->c_seqno = 0; + pcond->c_waiters = 0; + pcond->c_wakeups = 0; pcond->c_flags = 0; *cond = pcond; } @@ -96,7 +89,7 @@ int ret; THR_LOCK_ACQUIRE(thread, &_cond_static_lock); - if (*cond == NULL) + if (*cond == NULL) ret = pthread_cond_init(cond, NULL); else ret = 0; @@ -116,15 +109,12 @@ rval = EINVAL; else { /* Lock the condition variable structure: */ - rval = UMTX_LOCK(&(*cond)->c_lock, curthread->tid); - if (rval) - return (rval); - while ((*cond)->c_count) { - rval = umtx_wake(&(*cond)->c_count, (*cond)->c_count); - if (rval <= 0) - break; - (*cond)->c_count -= rval; + THR_LOCK_ACQUIRE(curthread, &(*cond)->c_lock); + if ((*cond)->c_waiters + (*cond)->c_wakeups != 0) { + THR_LOCK_RELEASE(curthread, &(*cond)->c_lock); + return (EBUSY); } + /* * NULL the caller's pointer now that the condition * variable has been destroyed: @@ -133,7 +123,7 @@ *cond = NULL; /* Unlock the condition variable structure: */ - umtx_unlock(&cv->c_lock, curthread->tid); + THR_LOCK_RELEASE(curthread, &cv->c_lock); /* Free the cond lock structure: */ @@ -148,56 +138,109 @@ return (rval); } +struct cond_cancel_info +{ + pthread_mutex_t *mutex; + pthread_cond_t *cond; + long seqno; +}; + +static void +cond_cancel_handler(void *arg) +{ + struct pthread *curthread = _get_curthread(); + struct cond_cancel_info *cci = (struct cond_cancel_info *)arg; + pthread_cond_t cv; + + cv = *(cci->cond); + THR_LOCK_ACQUIRE(curthread, &cv->c_lock); + if (cv->c_seqno != cci->seqno && cv->c_wakeups != 0) { + if (cv->c_waiters > 0) { + cv->c_seqno++; + umtx_wake((struct umtx *)&cv->c_seqno, 1); + } else + cv->c_wakeups--; + } else { + cv->c_waiters--; + } + THR_LOCK_RELEASE(curthread, &cv->c_lock); + + _mutex_cv_lock(cci->mutex); +} + static int cond_wait_common(pthread_cond_t *cond, pthread_mutex_t *mutex, - const struct timespec *abstime) + const struct timespec *abstime, int cancel) { struct pthread *curthread = _get_curthread(); - int rval = 0; + struct cond_cancel_info cci; + pthread_cond_t cv; + long seq, oldseq; + int oldcancel; + int ret = 0; /* * If the condition variable is statically initialized, * perform the dynamic initialization: */ if (__predict_false(*cond == NULL && - (rval = init_static(curthread, cond)) != 0)) - return (rval); + (ret = init_static(curthread, cond)) != 0)) + return (ret); + + cv = *cond; + THR_LOCK_ACQUIRE(curthread, &cv->c_lock); + ret = _mutex_cv_unlock(mutex); + if (ret) { + THR_LOCK_RELEASE(curthread, &cv->c_lock); + return (ret); + } + oldseq = seq = cv->c_seqno; + cci.mutex = mutex; + cci.cond = cond; + cci.seqno = oldseq; - if ((rval = UMTX_LOCK(&(*cond)->c_lock, curthread->tid)) == 0) { - rval = _mutex_cv_unlock(mutex); - if (__predict_false(rval)) { - umtx_unlock(&(*cond)->c_lock, curthread->tid); - return (rval); + cv->c_waiters++; + do { + if (cancel) { + THR_CLEANUP_PUSH(curthread, cond_cancel_handler, &cci); + THR_LOCK_RELEASE(curthread, &cv->c_lock); + oldcancel = _thr_cancel_enter(curthread); + ret = umtx_timedwait((struct umtx *)&cv->c_seqno, + seq, abstime); + _thr_cancel_leave(curthread, oldcancel); + THR_CLEANUP_POP(curthread, 0); + } else { + ret = umtx_timedwait((struct umtx *)&cv->c_seqno, + seq, abstime); + } + THR_LOCK_ACQUIRE(curthread, &cv->c_lock); + seq = cv->c_seqno; + if (abstime != NULL && ret != 0) { + if (ret == EAGAIN || ret == EINTR) + ret = ETIMEDOUT; + break; } - - /* I don't think you may have INIT_MAX threads. */ - if ((*cond)->c_count != INT_MAX) - (*cond)->c_count++; - - rval = umtx_timedwait(&(*cond)->c_lock, curthread->tid, - &(*cond)->c_count, abstime); - if (rval == EINTR) - rval = 0; - else if (rval == EAGAIN) /* POSIX needs ETIMEDOUT */ - rval = ETIMEDOUT; - /* - * Note! we don't touch condition variable after resuming! - * this makes it possible that waker can destroy the condition - * variable after calling pthread_cond_broadcast(), please - * see Single UNIX Specification Version 3 of - * pthread_cond_destroy(). + * loop if we have never been told to wake up + * or we lost a race. */ - check_continuation(curthread, NULL, mutex); - _mutex_cv_lock(mutex); + } while (seq == oldseq || cv->c_wakeups == 0); + + if (seq != oldseq && cv->c_wakeups != 0) { + cv->c_wakeups--; + ret = 0; + } else { + cv->c_waiters--; } - return (rval); + THR_LOCK_RELEASE(curthread, &cv->c_lock); + _mutex_cv_lock(mutex); + return (ret); } int _pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex) { - return cond_wait_common(cond, mutex, NULL); + return cond_wait_common(cond, mutex, NULL, 0); } __strong_reference(_pthread_cond_wait, _thr_cond_wait); @@ -205,12 +248,9 @@ int __pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex) { - struct pthread *curthread = _get_curthread(); int ret; - _thr_cancel_enter(curthread); - ret = cond_wait_common(cond, mutex, NULL); - _thr_cancel_leave(curthread, 1); + ret = cond_wait_common(cond, mutex, NULL, 1); return (ret); } @@ -218,30 +258,31 @@ _pthread_cond_timedwait(pthread_cond_t * cond, pthread_mutex_t * mutex, const struct timespec * abstime) { - if (abstime == NULL) + if (abstime == NULL || abstime->tv_sec < 0 || abstime->tv_nsec < 0 || + abstime->tv_nsec >= 1000000000) return (EINVAL); - return cond_wait_common(cond, mutex, abstime); + + return cond_wait_common(cond, mutex, abstime, 0); } +__strong_reference(_pthread_cond_timedwait, _thr_cond_timedwait); + int __pthread_cond_timedwait(pthread_cond_t *cond, pthread_mutex_t *mutex, const struct timespec *abstime) { - struct pthread *curthread = _get_curthread(); - int ret; + if (abstime == NULL || abstime->tv_sec < 0 || abstime->tv_nsec < 0 || + abstime->tv_nsec >= 1000000000) + return (EINVAL); - if (abstime == NULL) - return (EINVAL); - _thr_cancel_enter(curthread); - ret = _pthread_cond_timedwait(cond, mutex, abstime); - _thr_cancel_leave(curthread, 1); - return (ret); + return cond_wait_common(cond, mutex, abstime, 1); } static int cond_signal_common(pthread_cond_t *cond, int broadcast) { struct pthread *curthread = _get_curthread(); + pthread_cond_t cv; int rval = 0; /* @@ -252,30 +293,23 @@ (rval = init_static(curthread, cond)) != 0)) return (rval); - /* Lock the condition variable structure */ - rval = UMTX_LOCK(&(*cond)->c_lock, curthread->tid); - if (__predict_false(rval)) - return (rval); - - while ((*cond)->c_count) { - /* umtx_wake returns number of threads resumed */ - rval = umtx_wake(&(*cond)->c_count, - broadcast ? (*cond)->c_count : 1); - if (rval > 0) { - /* some threads were resumed. */ - (*cond)->c_count -= rval; - rval = 0; - } else if (rval == 0) { - (*cond)->c_count = 0; - break; + cv = *cond; + /* Lock the condition variable structure. */ + THR_LOCK_ACQUIRE(curthread, &cv->c_lock); + if (cv->c_waiters) { + if (!broadcast) { + cv->c_wakeups++; + cv->c_waiters--; + cv->c_seqno++; + umtx_wake((struct umtx *)&cv->c_seqno, 1); } else { - rval = errno; - break; + cv->c_wakeups += cv->c_waiters; + cv->c_waiters = 0; + cv->c_seqno++; + umtx_wake((struct umtx *)&cv->c_seqno, INT_MAX); } - if (!broadcast) - break; } - umtx_unlock(&(*cond)->c_lock, curthread->tid); + THR_LOCK_RELEASE(curthread, &cv->c_lock); return (rval); } @@ -294,9 +328,3 @@ } __strong_reference(_pthread_cond_broadcast, _thr_cond_broadcast); - -static inline void -check_continuation(struct pthread *curthread, struct pthread_cond *cond, - pthread_mutex_t *mutex) -{ -}
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200412311446.iBVEkn9O008840>