kernel - Fix condvar races
authorMatthew Dillon <dillon@apollo.backplane.com>
Fri, 17 Aug 2012 19:40:09 +0000 (12:40 -0700)
committerMatthew Dillon <dillon@apollo.backplane.com>
Fri, 17 Aug 2012 19:40:09 +0000 (12:40 -0700)
* Interlocked sleep was not using the PINTERLOCKED flag.

* Misc other adjustments.

sys/kern/kern_condvar.c

index a23be8b..8ec0c3e 100644 (file)
@@ -18,20 +18,27 @@ cv_destroy(struct cv *c)
 }
 
 int
 }
 
 int
-_cv_timedwait(struct cv *c, struct lock *l, int timo, int wakesig)
+_cv_timedwait(struct cv *c, struct lock *lk, int timo, int wakesig)
 {
        int flags = wakesig ? PCATCH : 0;
        int error;
 
 {
        int flags = wakesig ? PCATCH : 0;
        int error;
 
-       spin_lock(&c->cv_lock);
+       /*
+        * Can interlock without critical section/spinlock as long
+        * as we don't block before calling *sleep().  PINTERLOCKED
+        * must be passed to the *sleep() to use the manual interlock
+        * (else a new one is created which opens a timing race).
+        */
        tsleep_interlock(c, flags);
        tsleep_interlock(c, flags);
+
+       spin_lock(&c->cv_lock);
        c->cv_waiters++;
        spin_unlock(&c->cv_lock);
        c->cv_waiters++;
        spin_unlock(&c->cv_lock);
-       if (l != NULL)
-               lockmgr(l, LK_RELEASE);
-       error = tsleep(c, flags, c->cv_desc, timo);
-       if (l != NULL)
-               lockmgr(l, LK_EXCLUSIVE);
+
+       if (lk)
+               error = lksleep(c, lk, flags | PINTERLOCKED, c->cv_desc, timo);
+       else
+               error = tsleep(c, flags | PINTERLOCKED, c->cv_desc, timo);
 
        return (error);
 }
 
        return (error);
 }
@@ -40,19 +47,17 @@ void
 _cv_signal(struct cv *c, int broadcast)
 {
        spin_lock(&c->cv_lock);
 _cv_signal(struct cv *c, int broadcast)
 {
        spin_lock(&c->cv_lock);
-       if (c->cv_waiters == 0)
-               goto out;
-
-       if (broadcast) {
+       if (c->cv_waiters == 0) {
+               spin_unlock(&c->cv_lock);
+       } else if (broadcast) {
                c->cv_waiters = 0;
                c->cv_waiters = 0;
+               spin_unlock(&c->cv_lock);       /* must unlock first */
                wakeup(c);
        } else {
                c->cv_waiters--;
                wakeup(c);
        } else {
                c->cv_waiters--;
+               spin_unlock(&c->cv_lock);       /* must unlock first */
                wakeup_one(c);
        }
                wakeup_one(c);
        }
-
-out:
-       spin_unlock(&c->cv_lock);
 }
 
 int
 }
 
 int