kernel - remove spin_lock_rd() and spin_unlock_rd()
authorMatthew Dillon <dillon@apollo.backplane.com>
Mon, 30 Aug 2010 06:25:42 +0000 (23:25 -0700)
committerMatthew Dillon <dillon@apollo.backplane.com>
Mon, 30 Aug 2010 07:01:18 +0000 (00:01 -0700)
* Remove shared spinlocks entirely.  We only have exclusive spinlocks
  now.

* Strip out the exclusive/shared contention handling code.

* Slightly rework the initial exponential backoff and test.  Do the
  atomic swap after the exponential backoff instead of before so
  we do not add a superfluous backoff after actually acquiring
  the spinlock.

sys/kern/kern_ktr.c
sys/kern/kern_spinlock.c
sys/kern/lwkt_thread.c
sys/kern/usched_bsd4.c
sys/platform/pc32/isa/clock.c
sys/platform/pc64/isa/clock.c
sys/sys/globaldata.h
sys/sys/spinlock2.h

index 7b65515..d0748f3 100644 (file)
@@ -288,12 +288,6 @@ ktr_resync_callback(void *dummy __unused)
                        spin_unlock_wr(&spin);
                }
                logtest_noargs(spin_end);
-               logtest_noargs(spin_beg);
-               for (count = ktr_testspincnt; count; --count) {
-                       spin_lock_rd(&spin);
-                       spin_unlock_rd(&spin);
-               }
-               logtest_noargs(spin_end);
                ktr_testspincnt = 0;
        }
 #endif
index c44a850..ece6e04 100644 (file)
@@ -108,41 +108,16 @@ exponential_init(struct exponential_backoff *bo, struct spinlock *mtx)
        bo->backoff = BACKOFF_INITIAL;
        bo->nsec = 0;
        bo->mtx = mtx;
+       bo->base = 0;   /* silence gcc */
 }
 
 /*
- * We were either contested due to another exclusive lock holder,
- * or due to the presence of shared locks.  We have to undo the mess
- * we created by returning the shared locks.
- *
- * If there was another exclusive lock holder only the exclusive bit
- * in value will be the only bit set.  We don't have to do anything since
- * restoration does not involve any work.  
- *
- * Otherwise we successfully obtained the exclusive bit.  Attempt to
- * clear the shared bits.  If we are able to clear the shared bits 
- * we win.  Otherwise we lose and we have to restore the shared bits
- * we couldn't clear (and also clear our exclusive bit).
+ * We contested due to another exclusive lock holder.  We lose.
  */
 int
-spin_trylock_wr_contested(globaldata_t gd, struct spinlock *mtx, int value)
+spin_trylock_wr_contested2(globaldata_t gd)
 {
-       int bit;
-
        ++spinlocks_contested1;
-       if ((value & SPINLOCK_EXCLUSIVE) == 0) {
-               while (value) {
-                       bit = bsfl(value);
-                       if (globaldata_find(bit)->gd_spinlock_rd == mtx) {
-                               atomic_swap_int(&mtx->lock, value);
-                               --gd->gd_spinlocks_wr;
-                               --gd->gd_curthread->td_critcount;
-                               return (FALSE);
-                       }
-                       value &= ~(1 << bit);
-               }
-               return (TRUE);
-       }
        --gd->gd_spinlocks_wr;
        --gd->gd_curthread->td_critcount;
        return (FALSE);
@@ -156,102 +131,27 @@ spin_trylock_wr_contested(globaldata_t gd, struct spinlock *mtx, int value)
  * would have been set and we can throw away value. 
  */
 void
-spin_lock_wr_contested(struct spinlock *mtx, int value)
+spin_lock_wr_contested2(struct spinlock *mtx)
 {
        struct exponential_backoff backoff;
-       globaldata_t gd = mycpu;
-       int bit;
-       int mask;
+       int value;
 
        /*
         * Wait until we can gain exclusive access vs another exclusive
         * holder.
         */
-       exponential_init(&backoff, mtx);
        ++spinlocks_contested1;
-       logspin(beg, mtx, 'w');
+       exponential_init(&backoff, mtx);
 
-       while (value & SPINLOCK_EXCLUSIVE) {
-               value = atomic_swap_int(&mtx->lock, SPINLOCK_EXCLUSIVE);
-               if (exponential_backoff(&backoff)) {
-                       value &= ~SPINLOCK_EXCLUSIVE;
+       logspin(beg, mtx, 'w');
+       do {
+               if (exponential_backoff(&backoff))
                        break;
-               }
-       }
-
-       /*
-        * Kill the cached shared bit for our own cpu.  This is the most
-        * common case and there's no sense wasting cpu on it.  Since
-        * spinlocks aren't recursive, we can't own a shared ref on the
-        * spinlock while trying to get an exclusive one.
-        *
-        * If multiple bits are set do not stall on any single cpu.  Check
-        * all cpus that have the cache bit set, then loop and check again,
-        * until we've cleaned all the bits.
-        */
-       value &= ~gd->gd_cpumask;
-
-       while ((mask = value) != 0) {
-               while (mask) {
-                       bit = bsfl(value);
-                       if (globaldata_find(bit)->gd_spinlock_rd != mtx) {
-                               value &= ~(1 << bit);
-                       } else if (exponential_backoff(&backoff)) {
-                               value = 0;
-                               break;
-                       }
-                       mask &= ~(1 << bit);
-               }
-       }
+               value = atomic_swap_int(&mtx->lock, SPINLOCK_EXCLUSIVE);
+       } while (value & SPINLOCK_EXCLUSIVE);
        logspin(end, mtx, 'w');
 }
 
-/*
- * The cache bit wasn't set for our cpu.  Loop until we can set the bit.
- * As with the spin_lock_rd() inline we need a memory fence after setting
- * gd_spinlock_rd to interlock against exclusive spinlocks waiting for
- * that field to clear.
- */
-void
-spin_lock_rd_contested(struct spinlock *mtx)
-{
-       struct exponential_backoff backoff;
-       globaldata_t gd = mycpu;
-       int value = mtx->lock;
-
-       /*
-        * Shortcut the op if we can just set the cache bit.  This case
-        * occurs when the last lock was an exclusive lock.
-        */
-       while ((value & SPINLOCK_EXCLUSIVE) == 0) {
-               if (atomic_cmpset_int(&mtx->lock, value, value|gd->gd_cpumask))
-                       return;
-               value = mtx->lock;
-       }
-
-       exponential_init(&backoff, mtx);
-       ++spinlocks_contested1;
-
-       logspin(beg, mtx, 'r');
-
-       while ((value & gd->gd_cpumask) == 0) {
-               if (value & SPINLOCK_EXCLUSIVE) {
-                       gd->gd_spinlock_rd = NULL;
-                       if (exponential_backoff(&backoff)) {
-                               gd->gd_spinlock_rd = mtx;
-                               break;
-                       }
-                       gd->gd_spinlock_rd = mtx;
-                       cpu_mfence();
-               } else {
-                       if (atomic_cmpset_int(&mtx->lock, value, value|gd->gd_cpumask))
-                               break;
-               }
-               value = mtx->lock;
-       }
-       logspin(end, mtx, 'r');
-}
-
 /*
  * Handle exponential backoff and indefinite waits.
  *
@@ -377,18 +277,6 @@ sysctl_spin_lock_test(SYSCTL_HANDLER_ARGS)
                }
        }
 
-       /*
-        * Time best-case shared spinlocks
-        */
-       if (value == 3) {
-               globaldata_t gd = mycpu;
-
-               spin_init(&mtx);
-               for (i = spin_test_count; i > 0; --i) {
-                   spin_lock_rd_quick(gd, &mtx);
-                   spin_unlock_rd_quick(gd, &mtx);
-               }
-       }
         return (0);
 }
 
index c8004a0..41dd7a5 100644 (file)
@@ -563,9 +563,6 @@ lwkt_switch(void)
      * We had better not be holding any spin locks, but don't get into an
      * endless panic loop.
      */
-    KASSERT(gd->gd_spinlock_rd == NULL || panicstr != NULL, 
-           ("lwkt_switch: still holding a shared spinlock %p!", 
-            gd->gd_spinlock_rd));
     KASSERT(gd->gd_spinlocks_wr == 0 || panicstr != NULL, 
            ("lwkt_switch: still holding %d exclusive spinlocks!",
             gd->gd_spinlocks_wr));
@@ -989,7 +986,6 @@ lwkt_preempt(thread_t ntd, int critcount)
      * We could try to acquire the tokens but this case is so rare there
      * is no need to support it.
      */
-    KKASSERT(gd->gd_spinlock_rd == NULL);
     KKASSERT(gd->gd_spinlocks_wr == 0);
 
     if (TD_TOKS_HELD(ntd)) {
index 18096f4..61847d5 100644 (file)
@@ -555,7 +555,6 @@ bsd4_schedulerclock(struct lwp *lp, sysclock_t period, sysclock_t cpstamp)
         * Spinlocks also hold a critical section so there should not be
         * any active.
         */
-       KKASSERT(gd->gd_spinlock_rd == NULL);
        KKASSERT(gd->gd_spinlocks_wr == 0);
 
        bsd4_resetpriority(lp);
index f404d9f..c5acda6 100644 (file)
@@ -469,8 +469,7 @@ DRIVERSLEEP(int usec)
 {
        globaldata_t gd = mycpu;
 
-       if (gd->gd_intr_nesting_level || 
-           gd->gd_spinlock_rd != NULL || gd->gd_spinlocks_wr) {
+       if (gd->gd_intr_nesting_level || gd->gd_spinlocks_wr) {
                DODELAY(usec, 0);
        } else {
                DODELAY(usec, 1);
index 94ba26e..ae7bbca 100644 (file)
@@ -472,8 +472,7 @@ DRIVERSLEEP(int usec)
 {
        globaldata_t gd = mycpu;
 
-       if (gd->gd_intr_nesting_level || 
-           gd->gd_spinlock_rd || gd->gd_spinlocks_wr) {
+       if (gd->gd_intr_nesting_level || gd->gd_spinlocks_wr) {
                DODELAY(usec, 0);
        } else {
                DODELAY(usec, 1);
index 2c5878d..d78d5d0 100644 (file)
@@ -159,7 +159,7 @@ struct globaldata {
        sysid_t         gd_sysid_alloc;         /* allocate unique sysid */
 
        struct tslpque  *gd_tsleep_hash;        /* tsleep/wakeup support */
-       struct spinlock *gd_spinlock_rd;        /* Shared spinlock held */
+       void            *gd_unused08;
        int             gd_spinlocks_wr;        /* Exclusive spinlocks held */
        struct systimer *gd_systimer_inprog;    /* in-progress systimer */
        int             gd_timer_running;
index 7fb1e8b..4d0af1a 100644 (file)
 #include <machine/cpufunc.h>
 #endif
 
-/*
- * SPECIAL NOTE!  Obtaining a spinlock does not enter a critical section
- * or protect against FAST interrupts but it will prevent thread preemption.
- * Because the spinlock code path is ultra critical, we do not check for
- * LWKT reschedule requests (due to an interrupt thread not being able to
- * preempt).
- */
-
 #ifdef SMP
 
-extern int spin_trylock_wr_contested(globaldata_t gd, struct spinlock *mtx,
-    int value);
-extern void spin_lock_wr_contested(struct spinlock *mtx, int value);
-extern void spin_lock_rd_contested(struct spinlock *mtx);
+extern int spin_trylock_wr_contested2(globaldata_t gd);
+extern void spin_lock_wr_contested2(struct spinlock *mtx);
 
 #endif
 
@@ -90,7 +80,7 @@ spin_trylock_wr(struct spinlock *mtx)
        cpu_ccfence();
        ++gd->gd_spinlocks_wr;
        if ((value = atomic_swap_int(&mtx->lock, SPINLOCK_EXCLUSIVE)) != 0)
-               return (spin_trylock_wr_contested(gd, mtx, value));
+               return (spin_trylock_wr_contested2(gd));
        return (TRUE);
 }
 
@@ -110,8 +100,7 @@ spin_trylock_wr(struct spinlock *mtx)
 #endif
 
 /*
- * Obtain an exclusive spinlock and return.  Shortcut the case where the only
- * cached read lock was from our own cpu (it can just be cleared).
+ * Obtain an exclusive spinlock and return.
  */
 static __inline void
 spin_lock_wr_quick(globaldata_t gd, struct spinlock *mtx)
@@ -124,11 +113,8 @@ spin_lock_wr_quick(globaldata_t gd, struct spinlock *mtx)
        cpu_ccfence();
        ++gd->gd_spinlocks_wr;
 #ifdef SMP
-       if ((value = atomic_swap_int(&mtx->lock, SPINLOCK_EXCLUSIVE)) != 0) {
-               value &= ~gd->gd_cpumask;
-               if (value)
-                       spin_lock_wr_contested(mtx, value);
-       }
+       if ((value = atomic_swap_int(&mtx->lock, SPINLOCK_EXCLUSIVE)) != 0)
+               spin_lock_wr_contested2(mtx);
 #endif
 }
 
@@ -138,43 +124,6 @@ spin_lock_wr(struct spinlock *mtx)
        spin_lock_wr_quick(mycpu, mtx);
 }
 
-/*
- * Obtain a shared spinlock and return.  This is a critical code path.
- *
- * The vast majority of the overhead is in the cpu_mfence() (5ns vs 1ns for
- * the entire rest of the procedure).  Unfortunately we have to ensure that
- * spinlock pointer is written out before we check the cpumask to interlock
- * against an exclusive spinlock that clears the cpumask and then checks
- * the spinlock pointer.
- *
- * But what is EXTREMELY important here is that we do not have to perform
- * a locked bus cycle on the spinlock itself if the shared bit for our cpu
- * is already found to be set.  We only need the mfence, and the mfence is
- * local to the cpu and never conflicts with other cpu's.
- *
- * This means that multiple parallel shared acessors (e.g. filedescriptor
- * table lookups, namecache lookups) run at full speed and incur NO cache
- * contention at all.  It is the difference between 10ns and 40-100ns.
- */
-static __inline void
-spin_lock_rd_quick(globaldata_t gd, struct spinlock *mtx)
-{
-       ++gd->gd_curthread->td_critcount;
-       cpu_ccfence();
-       gd->gd_spinlock_rd = mtx;
-#ifdef SMP
-       cpu_mfence();
-       if ((mtx->lock & gd->gd_cpumask) == 0)
-               spin_lock_rd_contested(mtx);
-#endif
-}
-
-static __inline void
-spin_lock_rd(struct spinlock *mtx)
-{
-       spin_lock_rd_quick(mycpu,mtx);
-}
-
 /*
  * Release an exclusive spinlock.  We can just do this passively, only
  * ensuring that our spinlock count is left intact until the mutex is
@@ -198,28 +147,6 @@ spin_unlock_wr(struct spinlock *mtx)
        spin_unlock_wr_quick(mycpu, mtx);
 }
 
-/*
- * Release a shared spinlock.  We leave the shared bit set in the spinlock
- * as a cache and simply clear the spinlock pointer for the cpu.  This
- * fast-paths another shared lock later at the cost of an exclusive lock
- * having to check per-cpu spinlock pointers to determine when there are no
- * shared holders remaining.
- */
-static __inline void
-spin_unlock_rd_quick(globaldata_t gd, struct spinlock *mtx)
-{
-       KKASSERT(gd->gd_spinlock_rd == mtx);
-       gd->gd_spinlock_rd = NULL;
-       cpu_ccfence();
-       --gd->gd_curthread->td_critcount;
-}
-
-static __inline void
-spin_unlock_rd(struct spinlock *mtx)
-{
-       spin_unlock_rd_quick(mycpu, mtx);
-}
-
 static __inline void
 spin_init(struct spinlock *mtx)
 {