Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys Tidy rwlocks a bit, no functional change intended. Mainly:



details:   https://anonhg.NetBSD.org/src/rev/1d5b0917331a
branches:  trunk
changeset: 467297:1d5b0917331a
user:      ad <ad%NetBSD.org@localhost>
date:      Sun Jan 19 18:34:24 2020 +0000

description:
Tidy rwlocks a bit, no functional change intended.  Mainly:

- rw_downgrade(): do it in a for () loop like all the others.
- Explicitly carry around RW_NODEBUG - don't be lazy.
- Remove pointless macros.
- Don't make assertions conditional on LOCKDEBUG, there's no reason.
- Make space for a new flag bit (not added yet).

diffstat:

 sys/kern/kern_rwlock.c |  262 ++++++++++++++++++++++++------------------------
 sys/sys/rwlock.h       |   17 +--
 2 files changed, 138 insertions(+), 141 deletions(-)

diffs (truncated from 532 to 300 lines):

diff -r 692fedeaec35 -r 1d5b0917331a sys/kern/kern_rwlock.c
--- a/sys/kern/kern_rwlock.c    Sun Jan 19 17:54:19 2020 +0000
+++ b/sys/kern/kern_rwlock.c    Sun Jan 19 18:34:24 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: kern_rwlock.c,v 1.60 2020/01/12 18:37:10 ad Exp $      */
+/*     $NetBSD: kern_rwlock.c,v 1.61 2020/01/19 18:34:24 ad Exp $      */
 
 /*-
  * Copyright (c) 2002, 2006, 2007, 2008, 2009, 2019, 2020
@@ -39,7 +39,9 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_rwlock.c,v 1.60 2020/01/12 18:37:10 ad Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_rwlock.c,v 1.61 2020/01/19 18:34:24 ad Exp $");
+
+#include "opt_lockdebug.h"
 
 #define        __RWLOCK_PRIVATE
 
@@ -63,58 +65,32 @@
  * LOCKDEBUG
  */
 
-#if defined(LOCKDEBUG)
+#define        RW_DEBUG_P(rw)          (((rw)->rw_owner & RW_NODEBUG) == 0)
 
-#define        RW_WANTLOCK(rw, op)                                             \
-       LOCKDEBUG_WANTLOCK(RW_DEBUG_P(rw), (rw),                        \
-           (uintptr_t)__builtin_return_address(0), op == RW_READER);
-#define        RW_LOCKED(rw, op)                                               \
-       LOCKDEBUG_LOCKED(RW_DEBUG_P(rw), (rw), NULL,                    \
-           (uintptr_t)__builtin_return_address(0), op == RW_READER);
-#define        RW_UNLOCKED(rw, op)                                             \
-       LOCKDEBUG_UNLOCKED(RW_DEBUG_P(rw), (rw),                        \
-           (uintptr_t)__builtin_return_address(0), op == RW_READER);
-#define        RW_DASSERT(rw, cond)                                            \
-do {                                                                   \
-       if (__predict_false(!(cond)))                                   \
-               rw_abort(__func__, __LINE__, rw, "assertion failed: " #cond);\
-} while (/* CONSTCOND */ 0);
-
-#else  /* LOCKDEBUG */
-
-#define        RW_WANTLOCK(rw, op)     /* nothing */
-#define        RW_LOCKED(rw, op)       /* nothing */
-#define        RW_UNLOCKED(rw, op)     /* nothing */
-#define        RW_DASSERT(rw, cond)    /* nothing */
-
-#endif /* LOCKDEBUG */
+#define        RW_WANTLOCK(rw, op) \
+    LOCKDEBUG_WANTLOCK(RW_DEBUG_P(rw), (rw), \
+        (uintptr_t)__builtin_return_address(0), op == RW_READER);
+#define        RW_LOCKED(rw, op) \
+    LOCKDEBUG_LOCKED(RW_DEBUG_P(rw), (rw), NULL, \
+        (uintptr_t)__builtin_return_address(0), op == RW_READER);
+#define        RW_UNLOCKED(rw, op) \
+    LOCKDEBUG_UNLOCKED(RW_DEBUG_P(rw), (rw), \
+        (uintptr_t)__builtin_return_address(0), op == RW_READER);
 
 /*
  * DIAGNOSTIC
  */
 
 #if defined(DIAGNOSTIC)
-
-#define        RW_ASSERT(rw, cond)                                             \
-do {                                                                   \
-       if (__predict_false(!(cond)))                                   \
+#define        RW_ASSERT(rw, cond) \
+do { \
+       if (__predict_false(!(cond))) \
                rw_abort(__func__, __LINE__, rw, "assertion failed: " #cond);\
 } while (/* CONSTCOND */ 0)
-
 #else
-
 #define        RW_ASSERT(rw, cond)     /* nothing */
-
 #endif /* DIAGNOSTIC */
 
-#define        RW_SETDEBUG(rw, on)             ((rw)->rw_owner |= (on) ? 0 : RW_NODEBUG)
-#define        RW_DEBUG_P(rw)                  (((rw)->rw_owner & RW_NODEBUG) == 0)
-#if defined(LOCKDEBUG)
-#define        RW_INHERITDEBUG(n, o)           (n) |= (o) & RW_NODEBUG
-#else /* defined(LOCKDEBUG) */
-#define        RW_INHERITDEBUG(n, o)           /* nothing */
-#endif /* defined(LOCKDEBUG) */
-
 /*
  * Memory barriers.
  */
@@ -128,29 +104,6 @@
 #define        RW_MEMBAR_PRODUCER()            membar_producer()
 #endif
 
-static void    rw_abort(const char *, size_t, krwlock_t *, const char *);
-static void    rw_dump(const volatile void *, lockop_printer_t);
-static lwp_t   *rw_owner(wchan_t);
-
-static inline uintptr_t
-rw_cas(krwlock_t *rw, uintptr_t o, uintptr_t n)
-{
-
-       RW_INHERITDEBUG(n, o);
-       return (uintptr_t)atomic_cas_ptr((volatile void *)&rw->rw_owner,
-           (void *)o, (void *)n);
-}
-
-static inline void
-rw_swap(krwlock_t *rw, uintptr_t o, uintptr_t n)
-{
-
-       RW_INHERITDEBUG(n, o);
-       n = (uintptr_t)atomic_swap_ptr((volatile void *)&rw->rw_owner,
-           (void *)n);
-       RW_DASSERT(rw, n == o);
-}
-
 /*
  * For platforms that do not provide stubs, or for the LOCKDEBUG case.
  */
@@ -164,6 +117,10 @@
 __strong_alias(rw_tryenter,rw_vector_tryenter);
 #endif
 
+static void    rw_abort(const char *, size_t, krwlock_t *, const char *);
+static void    rw_dump(const volatile void *, lockop_printer_t);
+static lwp_t   *rw_owner(wchan_t);
+
 lockops_t rwlock_lockops = {
        .lo_name = "Reader / writer lock",
        .lo_type = LOCKOPS_SLEEP,
@@ -179,6 +136,37 @@
 };
 
 /*
+ * rw_cas:
+ *
+ *     Do an atomic compare-and-swap on the lock word.
+ */
+static inline uintptr_t
+rw_cas(krwlock_t *rw, uintptr_t o, uintptr_t n)
+{
+
+       return (uintptr_t)atomic_cas_ptr((volatile void *)&rw->rw_owner,
+           (void *)o, (void *)n);
+}
+
+/*
+ * rw_swap:
+ *
+ *     Do an atomic swap of the lock word.  This is used only when it's
+ *     known that the lock word is set up such that it can't be changed
+ *     behind us (assert this), so there's no point considering the result.
+ */
+static inline void
+rw_swap(krwlock_t *rw, uintptr_t o, uintptr_t n)
+{
+
+       n = (uintptr_t)atomic_swap_ptr((volatile void *)&rw->rw_owner,
+           (void *)n);
+
+       RW_ASSERT(rw, n == o);
+       RW_ASSERT(rw, (o & RW_HAS_WAITERS) != 0);
+}
+
+/*
  * rw_dump:
  *
  *     Dump the contents of a rwlock structure.
@@ -214,16 +202,14 @@
  *
  *     Initialize a rwlock for use.
  */
-void _rw_init(krwlock_t *, uintptr_t);
 void
 _rw_init(krwlock_t *rw, uintptr_t return_address)
 {
-       bool dodebug;
 
-       memset(rw, 0, sizeof(*rw));
-
-       dodebug = LOCKDEBUG_ALLOC(rw, &rwlock_lockops, return_address);
-       RW_SETDEBUG(rw, dodebug);
+       if (LOCKDEBUG_ALLOC(rw, &rwlock_lockops, return_address))
+               rw->rw_owner = 0;
+       else
+               rw->rw_owner = RW_NODEBUG;
 }
 
 void
@@ -243,7 +229,7 @@
 {
 
        RW_ASSERT(rw, (rw->rw_owner & ~RW_NODEBUG) == 0);
-       LOCKDEBUG_FREE(RW_DEBUG_P(rw), rw);
+       LOCKDEBUG_FREE((rw->rw_owner & RW_NODEBUG) == 0, rw);
 }
 
 /*
@@ -327,7 +313,7 @@
                need_wait = RW_WRITE_LOCKED | RW_WRITE_WANTED;
                queue = TS_READER_Q;
        } else {
-               RW_DASSERT(rw, op == RW_WRITER);
+               RW_ASSERT(rw, op == RW_WRITER);
                incr = curthread | RW_WRITE_LOCKED;
                set_wait = RW_HAS_WAITERS | RW_WRITE_WANTED;
                need_wait = RW_WRITE_LOCKED | RW_THREAD;
@@ -430,7 +416,7 @@
              (uintptr_t)__builtin_return_address(0)));
        LOCKSTAT_EXIT(lsflag);
 
-       RW_DASSERT(rw, (op != RW_READER && RW_OWNER(rw) == curthread) ||
+       RW_ASSERT(rw, (op != RW_READER && RW_OWNER(rw) == curthread) ||
            (op == RW_READER && RW_COUNT(rw) != 0));
        RW_LOCKED(rw, op);
 }
@@ -448,7 +434,8 @@
        int rcnt, wcnt;
        lwp_t *l;
 
-       curthread = (uintptr_t)curlwp;
+       l = curlwp;
+       curthread = (uintptr_t)l;
        RW_ASSERT(rw, curthread != 0);
 
        /*
@@ -491,8 +478,8 @@
         */
        ts = turnstile_lookup(rw);
        owner = rw->rw_owner;
-       RW_DASSERT(rw, ts != NULL);
-       RW_DASSERT(rw, (owner & RW_HAS_WAITERS) != 0);
+       RW_ASSERT(rw, ts != NULL);
+       RW_ASSERT(rw, (owner & RW_HAS_WAITERS) != 0);
 
        wcnt = TS_WAITERS(ts, TS_WRITER_Q);
        rcnt = TS_WAITERS(ts, TS_READER_Q);
@@ -509,31 +496,35 @@
         * do the work of acquiring the lock in rw_vector_enter().
         */
        if (rcnt == 0 || decr == RW_READ_INCR) {
-               RW_DASSERT(rw, wcnt != 0);
-               RW_DASSERT(rw, (owner & RW_WRITE_WANTED) != 0);
+               RW_ASSERT(rw, wcnt != 0);
+               RW_ASSERT(rw, (owner & RW_WRITE_WANTED) != 0);
 
                if (rcnt != 0) {
                        /* Give the lock to the longest waiting writer. */
                        l = TS_FIRST(ts, TS_WRITER_Q);
-                       newown = (uintptr_t)l | RW_WRITE_LOCKED | RW_HAS_WAITERS;
+                       newown = (uintptr_t)l | (owner & RW_NODEBUG);
+                       newown |= RW_WRITE_LOCKED | RW_HAS_WAITERS;
                        if (wcnt > 1)
                                newown |= RW_WRITE_WANTED;
                        rw_swap(rw, owner, newown);
                        turnstile_wakeup(ts, TS_WRITER_Q, 1, l);
                } else {
                        /* Wake all writers and let them fight it out. */
-                       rw_swap(rw, owner, RW_WRITE_WANTED);
+                       newown = owner & RW_NODEBUG;
+                       newown |= RW_WRITE_WANTED;
+                       rw_swap(rw, owner, newown);
                        turnstile_wakeup(ts, TS_WRITER_Q, wcnt, NULL);
                }
        } else {
-               RW_DASSERT(rw, rcnt != 0);
+               RW_ASSERT(rw, rcnt != 0);
 
                /*
                 * Give the lock to all blocked readers.  If there
                 * is a writer waiting, new readers that arrive
                 * after the release will be blocked out.
                 */
-               newown = rcnt << RW_READ_COUNT_SHIFT;
+               newown = owner & RW_NODEBUG;
+               newown += rcnt << RW_READ_COUNT_SHIFT;
                if (wcnt != 0)
                        newown |= RW_HAS_WAITERS | RW_WRITE_WANTED;
                        
@@ -552,8 +543,10 @@
 rw_vector_tryenter(krwlock_t *rw, const krw_t op)
 {
        uintptr_t curthread, owner, incr, need_wait, next;
+       lwp_t *l;
 
-       curthread = (uintptr_t)curlwp;
+       l = curlwp;
+       curthread = (uintptr_t)l;
 
        RW_ASSERT(rw, curthread != 0);
 
@@ -561,7 +554,7 @@
                incr = RW_READ_INCR;
                need_wait = RW_WRITE_LOCKED | RW_WRITE_WANTED;
        } else {
-               RW_DASSERT(rw, op == RW_WRITER);
+               RW_ASSERT(rw, op == RW_WRITER);



Home | Main Index | Thread Index | Old Index