Recent lwkt_token work broke UP builds. Fix the token code to operate
authorMatthew Dillon <dillon@dragonflybsd.org>
Fri, 19 May 2006 18:26:29 +0000 (18:26 +0000)
committerMatthew Dillon <dillon@dragonflybsd.org>
Fri, 19 May 2006 18:26:29 +0000 (18:26 +0000)
properly for both UP and SMP builds.  The SMP build uses spinlocks to
control access and also to do the preemption check.  The tokens are
explicitly obtained when a thread is switched in and released when a
thread is (non-preemptively) switched out.  Spinlocks cannot be
used for this purpose on UP because they are coded to a degenerate
case on a UP build.

On a UP build an explicit preemption check is needed, but no spinlock or
per-thread counter is required because the definition of a token is that
it is only 'held' while a thread is actually running or preempted.  So,
by definition, a token can always be obtained and held by a thread on UP
EXCEPT in the case where a preempting thread is trying to obtain a token
held by the preempted thread.

Conditionalize elements in the lwkt_token structure definition to guarentee
that SMP fields cannot be used in UP builds or vise-versa.  The lwkt_token
structure is made the same size for both builds.  Also remove some of
the degenerate spinlock functions (spin_trylock() and spin_tryunlock())
for UP builds to force a compile-time error if an attempt is made to use
them.  spin_lock*() and spin_unlock*() are retained as degenerate cases
on UP.

Reported-by: Sascha Wildner <saw@online.de>, walt <wa1ter@myrealbox.com>
sys/ddb/db_ps.c
sys/kern/lwkt_thread.c
sys/kern/lwkt_token.c
sys/sys/spinlock2.h
sys/sys/thread.h

index 1cf5bb1..d8efba5 100644 (file)
@@ -31,7 +31,7 @@
  * SUCH DAMAGE.
  *
  * $FreeBSD: src/sys/ddb/db_ps.c,v 1.20 1999/08/28 00:41:09 peter Exp $
- * $DragonFly: src/sys/ddb/db_ps.c,v 1.16 2006/05/18 16:25:17 dillon Exp $
+ * $DragonFly: src/sys/ddb/db_ps.c,v 1.17 2006/05/19 18:26:27 dillon Exp $
  */
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -205,8 +205,10 @@ db_dump_td_tokens(thread_t td)
                tok = ref->tr_tok;
 
                db_printf(" %p[tok=%p", ref, ref->tr_tok);
+#ifdef SMP
                if (td == tok->t_owner)
                    db_printf(",held");
+#endif
                db_printf("]");
        }
        db_printf("\n");
index f7b6168..b9a76bc 100644 (file)
@@ -31,7 +31,7 @@
  * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  * 
- * $DragonFly: src/sys/kern/lwkt_thread.c,v 1.94 2006/05/18 17:53:45 dillon Exp $
+ * $DragonFly: src/sys/kern/lwkt_thread.c,v 1.95 2006/05/19 18:26:28 dillon Exp $
  */
 
 /*
@@ -511,8 +511,10 @@ lwkt_switch(void)
            td->td_release(td);
 
     crit_enter_gd(gd);
+#ifdef SMP
     if (td->td_toks)
            lwkt_relalltokens(td);
+#endif
 
     /*
      * We had better not be holding any spin locks, but don't get into an
index 31ab8e7..fadc15f 100644 (file)
@@ -31,7 +31,7 @@
  * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  * 
- * $DragonFly: src/sys/kern/lwkt_token.c,v 1.24 2006/05/18 16:25:19 dillon Exp $
+ * $DragonFly: src/sys/kern/lwkt_token.c,v 1.25 2006/05/19 18:26:28 dillon Exp $
  */
 
 #ifdef _KERNEL
@@ -135,6 +135,11 @@ SYSCTL_INT(_lwkt, OID_AUTO, token_debug, CTLFLAG_RW, &token_debug, 0, "");
 /*
  * Obtain all the tokens required by the specified thread on the current
  * cpu, return 0 on failure and non-zero on success.
+ *
+ * NOTE: This code does not work with UP 'degenerate' spinlocks.  SMP only.
+ *
+ * The preemption code will not allow a target thread holding spinlocks to
+ * preempt the current thread so we do not have to implement this for UP.
  */
 int
 lwkt_getalltokens(thread_t td)
@@ -196,13 +201,21 @@ lwkt_relalltokens(thread_t td)
 #endif
 
 /*
- * Acquire a serializing token
+ * Acquire a serializing token.  This routine can block.
+ *
+ * On SMP systems we track ownership and a per-owner counter.  Tokens are
+ * released when a thread switches out and reacquired when a thread
+ * switches back in.  On UP systems we track a global counter for debugging
+ * but otherwise the only issue we have is if a preempting thread wants a
+ * token that is being held by the preempted thread.
  */
-
 static __inline
 void
 _lwkt_gettokref(lwkt_tokref_t ref)
 {
+#ifndef SMP
+    lwkt_tokref_t scan;
+#endif
     lwkt_token_t tok;
     thread_t td;
 
@@ -217,8 +230,9 @@ _lwkt_gettokref(lwkt_tokref_t ref)
     cpu_ccfence();
     td->td_toks = ref;
 
+#ifdef SMP
     /*
-     * Gain ownership of the token's spinlock.
+     * Gain ownership of the token's spinlock, SMP version.
      */
     if (tok->t_owner != td) {
        if (spin_trylock(td, &tok->t_spinlock) == 0) {
@@ -228,14 +242,35 @@ _lwkt_gettokref(lwkt_tokref_t ref)
        KKASSERT(tok->t_owner == NULL && tok->t_count == 0);
        tok->t_owner = td;
     }
-    ref->tr_state = 1;
     ++tok->t_count;
+#else
+    /*
+     * Gain ownership of the token, UP version.   All we have to do
+     * is check the token if we are preempting someone owning the
+     * same token.  If we are, we yield the cpu back to the originator
+     * and we will get rescheduled as non-preemptive.
+     */
+    while ((td = td->td_preempted) != NULL) {
+       for (scan = td->td_toks; scan; scan = scan->tr_next) {
+           if (scan->tr_tok == tok) {
+               lwkt_yield();
+               return;
+           }
+       }
+    }
+    /* NOTE: 'td' invalid after loop */
+    ++tok->t_globalcount;
+#endif
+    ref->tr_state = 1;
 }
 
 static __inline
 int
 _lwkt_trytokref(lwkt_tokref_t ref)
 {
+#ifndef SMP
+    lwkt_tokref_t scan;
+#endif
     lwkt_token_t tok;
     thread_t td;
 
@@ -250,8 +285,9 @@ _lwkt_trytokref(lwkt_tokref_t ref)
     cpu_ccfence();
     td->td_toks = ref;
 
+#ifdef SMP
     /*
-     * Gain ownership of the token's spinlock.
+     * Gain ownership of the token's spinlock, SMP version.
      */
     if (tok->t_owner != td) {
        if (spin_trylock(td, &tok->t_spinlock) == 0) {
@@ -261,8 +297,26 @@ _lwkt_trytokref(lwkt_tokref_t ref)
        KKASSERT(tok->t_owner == NULL && tok->t_count == 0);
        tok->t_owner = td;
     }
-    ref->tr_state = 1;
     ++tok->t_count;
+#else
+    /*
+     * Gain ownership of the token, UP version.   All we have to do
+     * is check the token if we are preempting someone owning the
+     * same token.  If we are, we yield the cpu back to the originator
+     * and we will get rescheduled as non-preemptive.
+     */
+    while ((td = td->td_preempted) != NULL) {
+       for (scan = td->td_toks; scan; scan = scan->tr_next) {
+           if (scan->tr_tok == tok) {
+               td->td_toks = ref->tr_next;
+               return (FALSE);
+           }
+       }
+    }
+    /* NOTE: 'td' invalid after loop */
+    ++tok->t_globalcount;
+#endif
+    ref->tr_state = 1;
     return (TRUE);
 }
 
@@ -308,16 +362,26 @@ lwkt_reltoken(lwkt_tokref *ref)
 
     td = curthread;
     tok = ref->tr_tok;
+
+#ifdef SMP
     KKASSERT(ref->tr_state == 1 && tok->t_owner == td && tok->t_count > 0);
+#else
+    KKASSERT(ref->tr_state == 1 && tok->t_globalcount > 0);
+#endif
 
     for (scanp = &td->td_toks; *scanp != ref; scanp = &((*scanp)->tr_next))
        ;
     *scanp = ref->tr_next;
     ref->tr_state = 0;
+
+#ifdef SMP
     if (--tok->t_count == 0) {
        tok->t_owner = NULL;
        spin_unlock_quick(&tok->t_spinlock);
     }
+#else
+    --tok->t_globalcount;
+#endif
     logtoken(release, ref);
 }
 
@@ -353,9 +417,13 @@ lwkt_token_pool_get(void *ptraddr)
 void
 lwkt_token_init(lwkt_token_t tok)
 {
+#ifdef SMP
     spin_init(&tok->t_spinlock);
     tok->t_owner = NULL;
     tok->t_count = 0;
+#else
+    tok->t_globalcount = 0;
+#endif
 }
 
 void
index 3c4482a..170865e 100644 (file)
@@ -29,7 +29,7 @@
  * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  *
- * $DragonFly: src/sys/sys/spinlock2.h,v 1.6 2006/05/18 16:25:20 dillon Exp $
+ * $DragonFly: src/sys/sys/spinlock2.h,v 1.7 2006/05/19 18:26:29 dillon Exp $
  */
 
 #ifndef _SYS_SPINLOCK2_H_
@@ -143,22 +143,16 @@ spin_uninit(struct spinlock *mtx)
 
 #else  /* SMP */
 
-static __inline boolean_t
-spin_trylock(thread_t td, struct spinlock *mtx)
-{
-       return (TRUE);
-}
-
-static __inline void
-spin_tryunlock(thread_t td, struct spinlock *mtx)
-{
-}
-
-static __inline boolean_t
-spin_is_locked(struct spinlock *mtx)
-{
-       return (FALSE);
-}
+/*
+ * There is no spin_trylock(), spin_tryunlock(), or spin_is_locked()
+ * for UP builds.  These functions are used by the kernel only in
+ * situations where the spinlock actually has to work.
+ *
+ * We provide the rest of the calls for UP as degenerate inlines (note
+ * that the non-quick versions still obtain/release a critical section!).
+ * This way we don't have to have a billion #ifdef's floating around
+ * the rest of the kernel.
+ */
 
 static __inline void   spin_lock_quick(struct spinlock *mtx) { }
 static __inline void   spin_unlock_quick(struct spinlock *mtx) { }
index e6654bf..bd801e0 100644 (file)
@@ -7,7 +7,7 @@
  * Types which must already be defined when this header is included by
  * userland:   struct md_thread
  * 
- * $DragonFly: src/sys/sys/thread.h,v 1.78 2006/05/18 16:25:20 dillon Exp $
+ * $DragonFly: src/sys/sys/thread.h,v 1.79 2006/05/19 18:26:29 dillon Exp $
  */
 
 #ifndef _SYS_THREAD_H_
@@ -82,9 +82,9 @@ struct intrframe;
 
 /*
  * Tokens are used to serialize access to information.  They are 'soft'
- * serialization entities that only stay in effect while the thread is
+ * serialization entities that only stay in effect while a thread is
  * running.  If the thread blocks, other threads can run holding the same
- * tokens.  The tokens are reacquired when the original thread resumes.
+ * token(s).  The tokens are reacquired when the original thread resumes.
  *
  * A thread can depend on its serialization remaining intact through a
  * preemption.  An interrupt which attempts to use the same token as the
@@ -95,13 +95,30 @@ struct intrframe;
  * Tokens are managed through a helper reference structure, lwkt_tokref,
  * which is typically declared on the caller's stack.  Multiple tokref's
  * may reference the same token.
+ *
+ * We do not actually have to track any information in the token itself
+ * on UP systems.  Simply linking the reference into the thread's td_toks
+ * list is sufficient.  We still track a global t_globalcount on UP for
+ * debugging purposes.
  */
+#ifdef SMP
+
 typedef struct lwkt_token {
     struct spinlock    t_spinlock;     /* Controls access */
     struct thread      *t_owner;       /* The current owner of the token */
-    int                        t_count;        /* Recursive count */
+    int                        t_count;        /* Per-thread count */
+} lwkt_token;
+
+#else
+
+typedef struct lwkt_token {
+    struct spinlock    t_unused01;
+    struct thread      *t_unused02;
+    int                        t_globalcount;  /* Global reference count */
 } lwkt_token;
 
+#endif
+
 typedef struct lwkt_tokref {
     lwkt_token_t       tr_tok;         /* token in question */
     lwkt_tokref_t      tr_next;        /* linked list */