something's not quite right yet. ticket locks result in more CPU usage

and spinning in kernel. partially back out, but in a way that makes going
forward again easy.
seen by ajacoutot

6 files changed, 40 insertions, 52 deletions

diff --git a/lib/librthread/rthread.c b/lib/librthread/rthread.c
index 3a2e6bf55b2..bf9eff0dd4d 100644
--- a/lib/librthread/rthread.c
+++ b/lib/librthread/rthread.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rthread.c,v 1.70 2013/06/01 20:47:40 tedu Exp $ */
+/*	$OpenBSD: rthread.c,v 1.71 2013/06/01 23:06:26 tedu Exp $ */
 /*
  * Copyright (c) 2004,2005 Ted Unangst <tedu@openbsd.org>
  * All Rights Reserved.
@@ -82,43 +82,20 @@ struct pthread_attr _rthread_attr_default = {
 void
 _spinlock(volatile struct _spinlock *lock)
 {
-	uint32_t me;
-
-	while (_atomic_lock(&lock->atomiclock))
+	while (_atomic_lock(&lock->ticket))
 		sched_yield();
-	me = lock->waiter++;
-	lock->atomiclock = _ATOMIC_LOCK_UNLOCKED;
-	while (me != lock->ready) {
-		if (me < lock->ready) {
-			_rthread_debug(0, "SPINLOCK FAIL: %d %d\n",
-			    me, lock->ready);
-			_exit(1);
-		}
-		if (me > lock->ready + 1)
-			sched_yield();
-	}
 }
 
 int
 _spinlocktry(volatile struct _spinlock *lock)
 {
-	int gotit = 0;
-
-	while (_atomic_lock(&lock->atomiclock))
-		sched_yield();
-	if (lock->waiter == lock->ready) {
-		lock->waiter++;
-		gotit = 1;
-	}
-	lock->atomiclock = _ATOMIC_LOCK_UNLOCKED;
-
-	return gotit;
+	return 0 == _atomic_lock(&lock->ticket);
 }
 
 void
 _spinunlock(volatile struct _spinlock *lock)
 {
-	lock->ready++;
+	lock->ticket = _ATOMIC_LOCK_UNLOCKED;
 }
 
 /*
@@ -648,7 +625,8 @@ _rthread_dl_lock(int what)
 		} else if (owner != self) {
 			TAILQ_INSERT_TAIL(&lockers, self, waiting);
 			while (owner != self) {
-				__thrsleep(self, 0 | 0x8, NULL, &lock.ready, NULL);
+				__thrsleep(self, 0 | _USING_TICKETS, NULL,
+				    &lock.ticket, NULL);
 				_spinlock(&lock);
 			}
 		}
diff --git a/lib/librthread/rthread.h b/lib/librthread/rthread.h
index df83b7dcc43..e10d2576054 100644
--- a/lib/librthread/rthread.h
+++ b/lib/librthread/rthread.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rthread.h,v 1.42 2013/06/01 20:47:40 tedu Exp $ */
+/*	$OpenBSD: rthread.h,v 1.43 2013/06/01 23:06:26 tedu Exp $ */
 /*
  * Copyright (c) 2004,2005 Ted Unangst <tedu@openbsd.org>
  * All Rights Reserved.
@@ -36,11 +36,17 @@
 #define RTHREAD_STACK_SIZE_DEF (256 * 1024)
 #endif
 
+#define _USING_TICKETS 0
+/*
+ * tickets don't work yet? (or seem much slower, with lots of system time)
+ * until then, keep the struct around to avoid excessive changes going
+ * back and forth.
+ */
 struct _spinlock {
-	_atomic_lock_t atomiclock;
-	uint32_t waiter;
-	uint32_t ready;
-	int pad;
+	_atomic_lock_t ticket;
+	uint32_t __waiter;
+	uint32_t __ready;
+	int __pad;
 };
 
 #define	_SPINLOCK_UNLOCKED { _ATOMIC_LOCK_UNLOCKED, 0, 0 }
diff --git a/lib/librthread/rthread_file.c b/lib/librthread/rthread_file.c
index 06afc366015..b0e0327bbe6 100644
--- a/lib/librthread/rthread_file.c
+++ b/lib/librthread/rthread_file.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rthread_file.c,v 1.5 2013/06/01 20:47:40 tedu Exp $	*/
+/*	$OpenBSD: rthread_file.c,v 1.6 2013/06/01 23:06:26 tedu Exp $	*/
 /*
  * Copyright (c) 1995 John Birrell <jb@cimlogic.com.au>.
  * All rights reserved.
@@ -204,7 +204,8 @@ void
 		 */
 		TAILQ_INSERT_TAIL(&p->lockers,self,waiting);
 		while (p->owner != self) {
-			__thrsleep(self, 0 | 0x8, NULL, &hash_lock.ready, NULL);
+			__thrsleep(self, 0 | _USING_TICKETS, NULL,
+			    &hash_lock.ticket, NULL);
 			_spinlock(&hash_lock);
 		}
 	}
diff --git a/lib/librthread/rthread_rwlock.c b/lib/librthread/rthread_rwlock.c
index f3b68367ead..1bf354d84da 100644
--- a/lib/librthread/rthread_rwlock.c
+++ b/lib/librthread/rthread_rwlock.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rthread_rwlock.c,v 1.3 2013/06/01 20:47:40 tedu Exp $ */
+/*	$OpenBSD: rthread_rwlock.c,v 1.4 2013/06/01 23:06:26 tedu Exp $ */
 /*
  * Copyright (c) 2004,2005 Ted Unangst <tedu@openbsd.org>
  * Copyright (c) 2012 Philip Guenther <guenther@openbsd.org>
@@ -117,8 +117,8 @@ _rthread_rwlock_rdlock(pthread_rwlock_t *lockp, const struct timespec *abstime,
 		error = EDEADLK;
 	else {
 		do {
-			if (__thrsleep(lock, CLOCK_REALTIME | 0x8, abstime,
-			    &lock->lock.ready, NULL) == EWOULDBLOCK)
+			if (__thrsleep(lock, CLOCK_REALTIME | _USING_TICKETS,
+			    abstime, &lock->lock.ticket, NULL) == EWOULDBLOCK)
 				return (ETIMEDOUT);
 			_spinlock(&lock->lock);
 		} while (lock->owner != NULL || !TAILQ_EMPTY(&lock->writers));
@@ -180,8 +180,9 @@ _rthread_rwlock_wrlock(pthread_rwlock_t *lockp, const struct timespec *abstime,
 		/* gotta block */
 		TAILQ_INSERT_TAIL(&lock->writers, thread, waiting);
 		do {
-			do_wait = __thrsleep(thread, CLOCK_REALTIME | 0x8, abstime,
-			    &lock->lock.ready, NULL) != EWOULDBLOCK;
+			do_wait = __thrsleep(thread, CLOCK_REALTIME |
+			    _USING_TICKETS, abstime,
+			    &lock->lock.ticket, NULL) != EWOULDBLOCK;
 			_spinlock(&lock->lock);
 		} while (lock->owner != thread && do_wait);
 
diff --git a/lib/librthread/rthread_sem.c b/lib/librthread/rthread_sem.c
index 81d9b951aa4..b4a08140817 100644
--- a/lib/librthread/rthread_sem.c
+++ b/lib/librthread/rthread_sem.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rthread_sem.c,v 1.8 2013/06/01 20:47:40 tedu Exp $ */
+/*	$OpenBSD: rthread_sem.c,v 1.9 2013/06/01 23:06:26 tedu Exp $ */
 /*
  * Copyright (c) 2004,2005 Ted Unangst <tedu@openbsd.org>
  * All Rights Reserved.
@@ -42,8 +42,9 @@ _sem_wait(sem_t sem, int tryonly, const struct timespec *abstime,
 	} else {
 		sem->waitcount++;
 		do {
-			r = __thrsleep(&sem->waitcount, CLOCK_REALTIME | 0x8,
-			    abstime, &sem->lock.ready, delayed_cancel);
+			r = __thrsleep(&sem->waitcount, CLOCK_REALTIME |
+			    _USING_TICKETS, abstime, &sem->lock.ticket,
+			    delayed_cancel);
 			_spinlock(&sem->lock);
 			/* ignore interruptions other than cancelation */
 			if (r == EINTR && (delayed_cancel == NULL ||
diff --git a/lib/librthread/rthread_sync.c b/lib/librthread/rthread_sync.c
index d8ee607194d..72e664b431e 100644
--- a/lib/librthread/rthread_sync.c
+++ b/lib/librthread/rthread_sync.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rthread_sync.c,v 1.38 2013/06/01 20:47:40 tedu Exp $ */
+/*	$OpenBSD: rthread_sync.c,v 1.39 2013/06/01 23:06:26 tedu Exp $ */
 /*
  * Copyright (c) 2004,2005 Ted Unangst <tedu@openbsd.org>
  * Copyright (c) 2012 Philip Guenther <guenther@openbsd.org>
@@ -127,8 +127,9 @@ _rthread_mutex_lock(pthread_mutex_t *mutexp, int trywait,
 				abort();
 
 			/* self-deadlock, possibly until timeout */
-			while (__thrsleep(self, CLOCK_REALTIME | 0x8, abstime,
-			    &mutex->lock.ready, NULL) != EWOULDBLOCK)
+			while (__thrsleep(self, CLOCK_REALTIME |
+			    _USING_TICKETS, abstime,
+			    &mutex->lock.ticket, NULL) != EWOULDBLOCK)
 				_spinlock(&mutex->lock);
 			return (ETIMEDOUT);
 		}
@@ -144,8 +145,8 @@ _rthread_mutex_lock(pthread_mutex_t *mutexp, int trywait,
 		/* add to the wait queue and block until at the head */
 		TAILQ_INSERT_TAIL(&mutex->lockers, self, waiting);
 		while (mutex->owner != self) {
-			ret = __thrsleep(self, CLOCK_REALTIME | 0x8, abstime,
-			    &mutex->lock.ready, NULL);
+			ret = __thrsleep(self, CLOCK_REALTIME | _USING_TICKETS,
+			    abstime, &mutex->lock.ticket, NULL);
 			_spinlock(&mutex->lock);
 			assert(mutex->owner != NULL);
 			if (ret == EWOULDBLOCK) {
@@ -350,8 +351,8 @@ pthread_cond_timedwait(pthread_cond_t *condp, pthread_mutex_t *mutexp,
 
 	/* wait until we're the owner of the mutex again */
 	while (mutex->owner != self) {
-		error = __thrsleep(self, cond->clock | 0x8, abstime,
-		    &mutex->lock.ready, &self->delayed_cancel);
+		error = __thrsleep(self, cond->clock | _USING_TICKETS, abstime,
+		    &mutex->lock.ticket, &self->delayed_cancel);
 
 		/*
 		 * If abstime == NULL, then we're definitely waiting
@@ -497,8 +498,8 @@ pthread_cond_wait(pthread_cond_t *condp, pthread_mutex_t *mutexp)
 
 	/* wait until we're the owner of the mutex again */
 	while (mutex->owner != self) {
-		error = __thrsleep(self, 0 | 0x8, NULL, &mutex->lock.ready,
-		    &self->delayed_cancel);
+		error = __thrsleep(self, 0 | _USING_TICKETS, NULL,
+		    &mutex->lock.ticket, &self->delayed_cancel);
 
 		/*
 		 * If we took a normal signal (not from