- Rename rw_test_and_set to rw_cas, since most litterature uses the

  test_and_set name for some other operation, while cas is generally
  used for compare and set (cmpxchg in intel land, cas in sparc land).

- Make rw locks properly MP safe (provided that rw_cas is implemented
  in MD code). Most operations were MP safe except the sleep where we
  could have set the "I'm sleeping" flag before actually going to sleep
  so that the wakeup could miss us. Now, using the split tsleep,
  we first setup the sleep (put us on the sleep queues), then set
  the flag aborting the sleep if the lock has changed and then finally
  go to sleep.

miod@ ok (and he's been prodding me for days to get this in)

2 files changed, 31 insertions, 14 deletions

diff --git a/sys/kern/kern_rwlock.c b/sys/kern/kern_rwlock.c
index 85f1e6ace92..3641403f369 100644
--- a/sys/kern/kern_rwlock.c
+++ b/sys/kern/kern_rwlock.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_rwlock.c,v 1.10 2007/04/04 18:01:57 art Exp $	*/
+/*	$OpenBSD: kern_rwlock.c,v 1.11 2007/05/04 12:56:15 art Exp $	*/
 
 /*
  * Copyright (c) 2002, 2003 Artur Grabowski <art@openbsd.org>
@@ -31,6 +31,8 @@
 #include <sys/rwlock.h>
 #include <sys/limits.h>
 
+#include <machine/lock.h>
+
 /* XXX - temporary measure until proc0 is properly aligned */
 #define RW_PROC(p) (((long)p) & ~RWLOCK_MASK)
 
@@ -86,7 +88,7 @@ rw_enter_read(struct rwlock *rwl)
 	unsigned long owner = rwl->rwl_owner;
 
 	if (__predict_false((owner & RWLOCK_WRLOCK) ||
-	    rw_test_and_set(&rwl->rwl_owner, owner, owner + RWLOCK_READ_INCR)))
+	    rw_cas(&rwl->rwl_owner, owner, owner + RWLOCK_READ_INCR)))
 		rw_enter(rwl, RW_READ);
 }
 
@@ -95,7 +97,7 @@ rw_enter_write(struct rwlock *rwl)
 {
 	struct proc *p = curproc;
 
-	if (__predict_false(rw_test_and_set(&rwl->rwl_owner, 0,
+	if (__predict_false(rw_cas(&rwl->rwl_owner, 0,
 	    RW_PROC(p) | RWLOCK_WRLOCK)))
 		rw_enter(rwl, RW_WRITE);
 }
@@ -106,7 +108,7 @@ rw_exit_read(struct rwlock *rwl)
 	unsigned long owner = rwl->rwl_owner;
 
 	if (__predict_false((owner & RWLOCK_WAIT) ||
-	    rw_test_and_set(&rwl->rwl_owner, owner, owner - RWLOCK_READ_INCR)))
+	    rw_cas(&rwl->rwl_owner, owner, owner - RWLOCK_READ_INCR)))
 		rw_exit(rwl);
 }
 
@@ -116,12 +118,13 @@ rw_exit_write(struct rwlock *rwl)
 	unsigned long owner = rwl->rwl_owner;
 
 	if (__predict_false((owner & RWLOCK_WAIT) ||
-	    rw_test_and_set(&rwl->rwl_owner, owner, 0)))
+	    rw_cas(&rwl->rwl_owner, owner, 0)))
 		rw_exit(rwl);
 }
 
+#ifndef rw_cas
 int
-rw_test_and_set(volatile unsigned long *p, unsigned long o, unsigned long n)
+rw_cas(volatile unsigned long *p, unsigned long o, unsigned long n)
 {
 	if (*p != o)
 		return (1);
@@ -131,6 +134,8 @@ rw_test_and_set(volatile unsigned long *p, unsigned long o, unsigned long n)
 }
 #endif
 
+#endif
+
 #ifdef DIAGNOSTIC
 /*
  * Put the diagnostic functions here to keep the main code free
@@ -175,16 +180,17 @@ int
 rw_enter(struct rwlock *rwl, int flags)
 {
 	const struct rwlock_op *op;
+	struct sleep_state sls;
 	unsigned long inc, o;
-	int error, prio;
+	int error;
 
 	op = &rw_ops[flags & RW_OPMASK];
 
 	inc = op->inc + RW_PROC(curproc) * op->proc_mult;
 retry:
 	while (__predict_false(((o = rwl->rwl_owner) & op->check) != 0)) {
-		if (rw_test_and_set(&rwl->rwl_owner, o, o | op->wait_set))
-			continue;
+		unsigned long set = o | op->wait_set;
+		int do_sleep, prio;
 
 		prio = op->wait_prio;
 		if (flags & RW_INTR)
@@ -194,13 +200,22 @@ retry:
 
 		if (flags & RW_NOSLEEP)
 			return (EBUSY);
-		if ((error = tsleep(rwl, prio, rwl->rwl_name, 0)) != 0)
+
+		sleep_setup(&sls, rwl, op->wait_prio, rwl->rwl_name);
+		if (flags & RW_INTR)
+			sleep_setup_signal(&sls, op->wait_prio | PCATCH);
+
+		do_sleep = !rw_cas(&rwl->rwl_owner, o, set);
+
+		sleep_finish(&sls, do_sleep);
+		if ((flags & RW_INTR) &&
+		    (error = sleep_finish_signal(&sls)) != 0)
 			return (error);
 		if (flags & RW_SLEEPFAIL)
 			return (EAGAIN);
 	}
 
-	if (__predict_false(rw_test_and_set(&rwl->rwl_owner, o, o + inc)))
+	if (__predict_false(rw_cas(&rwl->rwl_owner, o, o + inc)))
 		goto retry;
 
 	/*
@@ -229,7 +244,7 @@ rw_exit(struct rwlock *rwl)
 		else
 			set = (owner - RWLOCK_READ_INCR) &
 				~(RWLOCK_WAIT|RWLOCK_WRWANT);
-	} while (rw_test_and_set(&rwl->rwl_owner, owner, set));
+	} while (rw_cas(&rwl->rwl_owner, owner, set));
 
 	if (owner & RWLOCK_WAIT)
 		wakeup(rwl);
diff --git a/sys/sys/rwlock.h b/sys/sys/rwlock.h
index 405b9d9351d..61c48eea1ce 100644
--- a/sys/sys/rwlock.h
+++ b/sys/sys/rwlock.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rwlock.h,v 1.9 2007/04/04 18:01:57 art Exp $	*/
+/*	$OpenBSD: rwlock.h,v 1.10 2007/05/04 12:56:15 art Exp $	*/
 /*
  * Copyright (c) 2002 Artur Grabowski <art@openbsd.org>
  * All rights reserved. 
@@ -106,6 +106,8 @@ void rw_exit(struct rwlock *);
 #define RW_SLEEPFAIL	0x20UL		/* fail if we slept for the lock */
 #define RW_NOSLEEP	0x40UL		/* don't wait for the lock */
 
-int rw_test_and_set(volatile unsigned long *, unsigned long, unsigned long);
+#ifndef rw_cas
+int rw_cas(volatile unsigned long *, unsigned long, unsigned long);
+#endif
 
 #endif