summaryrefslogtreecommitdiff
path: root/lib/libpthread
diff options
context:
space:
mode:
authorad <ad@NetBSD.org>2007-03-02 17:47:40 +0000
committerad <ad@NetBSD.org>2007-03-02 17:47:40 +0000
commitdba14ef0a08bc45d3d11517130f37fff26a4ba4c (patch)
treed39f1089d2918361f1591fc84abfbf1028d16dd9 /lib/libpthread
parent948e7684e3b4a3d529151da3a97c3fd6fbc28d12 (diff)
Don't grab the state lock to check for cancellation around condition
variables, as _lwp_wakeup/lwp_park provide the necessary barrier.
Diffstat (limited to 'lib/libpthread')
-rw-r--r--lib/libpthread/TODO6
-rw-r--r--lib/libpthread/pthread_cond.c96
2 files changed, 64 insertions, 38 deletions
diff --git a/lib/libpthread/TODO b/lib/libpthread/TODO
index b5cada788b3..ac9f2cdb129 100644
--- a/lib/libpthread/TODO
+++ b/lib/libpthread/TODO
@@ -1,4 +1,4 @@
-$NetBSD: TODO,v 1.6 2007/02/15 15:39:33 yamt Exp $
+$NetBSD: TODO,v 1.7 2007/03/02 17:47:40 ad Exp $
Bugs to fix, mostly with SA:
@@ -108,10 +108,6 @@ Future work for 1:1 threads:
that does something similar. Again, lots of thought and benchmarking
required. (Original idea from matt@)
-- It's possible that we don't need to take so many spinlocks around
- cancellation points like pthread_cond_wait() given that _lwp_wakeup()
- and _lwp_unpark() need to synchronise anyway.
-
- Need to give consideration to the order in which threads enter and exit
synchronisation objects, both in the pthread library and in the kernel.
Commonly locks are acquired/released in order (a, b, c -> c, b, a). The
diff --git a/lib/libpthread/pthread_cond.c b/lib/libpthread/pthread_cond.c
index c42e8061df9..f140a3b5ebf 100644
--- a/lib/libpthread/pthread_cond.c
+++ b/lib/libpthread/pthread_cond.c
@@ -1,4 +1,4 @@
-/* $NetBSD: pthread_cond.c,v 1.20 2006/12/24 18:39:46 ad Exp $ */
+/* $NetBSD: pthread_cond.c,v 1.21 2007/03/02 17:47:40 ad Exp $ */
/*-
* Copyright (c) 2001 The NetBSD Foundation, Inc.
@@ -37,7 +37,7 @@
*/
#include <sys/cdefs.h>
-__RCSID("$NetBSD: pthread_cond.c,v 1.20 2006/12/24 18:39:46 ad Exp $");
+__RCSID("$NetBSD: pthread_cond.c,v 1.21 2007/03/02 17:47:40 ad Exp $");
#include <errno.h>
#include <sys/time.h>
@@ -119,15 +119,11 @@ pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex)
if (__predict_false(pthread__started == 0))
return pthread_cond_wait_nothread(self, mutex, NULL);
+#ifdef PTHREAD_SA
pthread_spinlock(self, &cond->ptc_lock);
SDPRINTF(("(cond wait %p) Waiting on %p, mutex %p\n",
self, cond, mutex));
- pthread_spinlock(self, &self->pt_statelock);
- if (__predict_false(self->pt_cancel)) {
- pthread_spinunlock(self, &self->pt_statelock);
- pthread_spinunlock(self, &cond->ptc_lock);
- pthread_exit(PTHREAD_CANCELED);
- }
+
#ifdef ERRORCHECK
if (cond->ptc_mutex == NULL)
cond->ptc_mutex = mutex;
@@ -137,7 +133,13 @@ pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex)
cond->ptc_mutex == mutex);
#endif
-#ifdef PTHREAD_SA
+ pthread_spinlock(self, &self->pt_statelock);
+ if (__predict_false(self->pt_cancel)) {
+ pthread_spinunlock(self, &self->pt_statelock);
+ pthread_spinunlock(self, &cond->ptc_lock);
+ pthread_exit(PTHREAD_CANCELED);
+ }
+
self->pt_sleepobj = cond;
self->pt_state = PT_STATE_BLOCKED_QUEUE;
self->pt_sleepq = &cond->ptc_waiters;
@@ -149,8 +151,20 @@ pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex)
pthread__block(self, &cond->ptc_lock);
/* Spinlock is unlocked on return */
#else /* PTHREAD_SA */
+ SDPRINTF(("(cond wait %p) Waiting on %p, mutex %p\n",
+ self, cond, mutex));
+ if (__predict_false(self->pt_cancel))
+ pthread_exit(PTHREAD_CANCELED);
+ pthread_spinlock(self, &cond->ptc_lock);
+#ifdef ERRORCHECK
+ if (cond->ptc_mutex == NULL)
+ cond->ptc_mutex = mutex;
+ else
+ pthread__error(EINVAL,
+ "Multiple mutexes used for condition wait",
+ cond->ptc_mutex == mutex);
+#endif
pthread_mutex_unlock(mutex);
- pthread_spinunlock(self, &self->pt_statelock);
(void)pthread__park(self, &cond->ptc_lock, cond,
&cond->ptc_waiters, NULL, 0, 1);
pthread_spinunlock(self, &cond->ptc_lock);
@@ -206,10 +220,12 @@ pthread_cond_timedwait(pthread_cond_t *cond, pthread_mutex_t *mutex,
if (__predict_false(pthread__started == 0))
return pthread_cond_wait_nothread(self, mutex, abstime);
- pthread_spinlock(self, &cond->ptc_lock);
wait.ptw_thread = self;
wait.ptw_cond = cond;
retval = 0;
+
+#ifdef PTHREAD_SA
+ pthread_spinlock(self, &cond->ptc_lock);
SDPRINTF(("(cond timed wait %p) Waiting on %p until %d.%06ld\n",
self, cond, abstime->tv_sec, abstime->tv_nsec/1000));
@@ -228,7 +244,6 @@ pthread_cond_timedwait(pthread_cond_t *cond, pthread_mutex_t *mutex,
cond->ptc_mutex == mutex);
#endif
-#ifdef PTHREAD_SA
pthread__alarm_add(self, &alarm, abstime, pthread_cond_wait__callback,
&wait);
self->pt_state = PT_STATE_BLOCKED_QUEUE;
@@ -246,8 +261,21 @@ pthread_cond_timedwait(pthread_cond_t *cond, pthread_mutex_t *mutex,
if (pthread__alarm_fired(&alarm))
retval = ETIMEDOUT;
#else /* PTHREAD_SA */
+ SDPRINTF(("(cond timed wait %p) Waiting on %p until %d.%06ld\n",
+ self, cond, abstime->tv_sec, abstime->tv_nsec/1000));
+
+ if (__predict_false(self->pt_cancel))
+ pthread_exit(PTHREAD_CANCELED);
+ pthread_spinlock(self, &cond->ptc_lock);
+#ifdef ERRORCHECK
+ if (cond->ptc_mutex == NULL)
+ cond->ptc_mutex = mutex;
+ else
+ pthread__error(EINVAL,
+ "Multiple mutexes used for condition wait",
+ cond->ptc_mutex == mutex);
+#endif
pthread_mutex_unlock(mutex);
- pthread_spinunlock(self, &self->pt_statelock);
retval = pthread__park(self, &cond->ptc_lock, cond,
&cond->ptc_waiters, abstime, 0, 1);
pthread_spinunlock(self, &cond->ptc_lock);
@@ -328,18 +356,26 @@ pthread_cond_signal(pthread_cond_t *cond)
pthread_spinunlock(self, &cond->ptc_lock);
}
#else /* PTHREAD_SA */
- self = pthread__self();
- pthread_spinlock(self, &cond->ptc_lock);
- if ((signaled = PTQ_FIRST(&cond->ptc_waiters)) != NULL) {
- PTQ_REMOVE(&cond->ptc_waiters, signaled, pt_sleep);
- PTHREADD_ADD(PTHREADD_COND_WOKEUP);
+ if (!PTQ_EMPTY(&cond->ptc_waiters)) {
+ self = pthread__self();
+ pthread_spinlock(self, &cond->ptc_lock);
+ signaled = PTQ_FIRST(&cond->ptc_waiters);
+ if (signaled != NULL) {
+ PTQ_REMOVE(&cond->ptc_waiters, signaled, pt_sleep);
+#ifdef ERRORCHECK
+ if (PTQ_EMPTY(&cond->ptc_waiters))
+ cond->ptc_mutex = NULL;
+#endif
+ pthread__unpark(self, &cond->ptc_lock, cond, signaled);
+ PTHREADD_ADD(PTHREADD_COND_WOKEUP);
+ } else {
#ifdef ERRORCHECK
- if (PTQ_EMPTY(&cond->ptc_waiters))
cond->ptc_mutex = NULL;
#endif
+ pthread_spinunlock(self, &cond->ptc_lock);
+ }
}
- pthread__unpark(self, &cond->ptc_lock, cond, signaled);
-#endif /* PTHREAD_SA */
+#endif
return 0;
}
@@ -358,29 +394,23 @@ pthread_cond_broadcast(pthread_cond_t *cond)
SDPRINTF(("(cond signal %p) Broadcasting %p\n",
pthread__self(), cond));
-#ifdef PTHREAD_SA
if (!PTQ_EMPTY(&cond->ptc_waiters)) {
self = pthread__self();
pthread_spinlock(self, &cond->ptc_lock);
- blockedq = cond->ptc_waiters;
- PTQ_INIT(&cond->ptc_waiters);
#ifdef ERRORCHECK
cond->ptc_mutex = NULL;
#endif
+#ifdef PTHREAD_SA
+ blockedq = cond->ptc_waiters;
+ PTQ_INIT(&cond->ptc_waiters);
pthread__sched_sleepers(self, &blockedq);
PTHREADD_ADD(PTHREADD_COND_WOKEUP);
pthread_spinunlock(self, &cond->ptc_lock);
- }
#else /* PTHREAD_SA */
- self = pthread__self();
- pthread_spinlock(self, &cond->ptc_lock);
-#ifdef ERRORCHECK
- if (!PTQ_EMPTY(&cond->ptc_waiters))
- cond->ptc_mutex = NULL;
-#endif
- pthread__unpark_all(self, &cond->ptc_lock, cond, &cond->ptc_waiters);
- PTHREADD_ADD(PTHREADD_COND_WOKEUP);
+ pthread__unpark_all(self, &cond->ptc_lock, cond, &cond->ptc_waiters);
+ PTHREADD_ADD(PTHREADD_COND_WOKEUP);
#endif /* PTHREAD_SA */
+ }
return 0;