Source-Changes-HG archive

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

[src/trunk]: src/lib/libpthread - Try to eliminate a hang in "parked" I've be...



details:   https://anonhg.NetBSD.org/src/rev/9fc14429d3c7
branches:  trunk
changeset: 932815:9fc14429d3c7
user:      ad <ad%NetBSD.org@localhost>
date:      Sat May 16 22:53:37 2020 +0000

description:
- Try to eliminate a hang in "parked" I've been seeing while stress testing.
  Centralise wakeup of deferred waiters in pthread__clear_waiters() and use
  throughout libpthread.  Make fewer assumptions.  Be more conservative in
  pthread_mutex when dealing with pending waiters.

- Remove the "hint" argument everywhere since the kernel doesn't use it any
  more.

diffstat:

 lib/libpthread/pthread.c         |  107 ++++++++----
 lib/libpthread/pthread_barrier.c |    8 +-
 lib/libpthread/pthread_cond.c    |   24 +--
 lib/libpthread/pthread_int.h     |    8 +-
 lib/libpthread/pthread_mutex.c   |  323 ++++++++++++++------------------------
 lib/libpthread/pthread_rwlock.c  |    8 +-
 6 files changed, 207 insertions(+), 271 deletions(-)

diffs (truncated from 854 to 300 lines):

diff -r fb7ec21dd705 -r 9fc14429d3c7 lib/libpthread/pthread.c
--- a/lib/libpthread/pthread.c  Sat May 16 22:07:06 2020 +0000
+++ b/lib/libpthread/pthread.c  Sat May 16 22:53:37 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: pthread.c,v 1.169 2020/05/15 14:30:23 joerg Exp $      */
+/*     $NetBSD: pthread.c,v 1.170 2020/05/16 22:53:37 ad Exp $ */
 
 /*-
  * Copyright (c) 2001, 2002, 2003, 2006, 2007, 2008, 2020
@@ -31,7 +31,7 @@
  */
 
 #include <sys/cdefs.h>
-__RCSID("$NetBSD: pthread.c,v 1.169 2020/05/15 14:30:23 joerg Exp $");
+__RCSID("$NetBSD: pthread.c,v 1.170 2020/05/16 22:53:37 ad Exp $");
 
 #define        __EXPOSE_STACK  1
 
@@ -105,7 +105,7 @@
 
 int pthread__concurrency;
 int pthread__nspins;
-int pthread__unpark_max = PTHREAD__UNPARK_MAX;
+size_t pthread__unpark_max = PTHREAD__UNPARK_MAX;
 int pthread__dbg;      /* set by libpthread_dbg if active */
 
 /*
@@ -191,9 +191,9 @@
 {
        pthread_t first;
        char *p;
-       int i;
        int mib[2];
        unsigned int value;
+       ssize_t slen;
        size_t len;
        extern int __isthreaded;
 
@@ -223,16 +223,16 @@
 
        /* Initialize locks first; they're needed elsewhere. */
        pthread__lockprim_init();
-       for (i = 0; i < NHASHLOCK; i++) {
+       for (int i = 0; i < NHASHLOCK; i++) {
                pthread_mutex_init(&hashlocks[i].mutex, NULL);
        }
 
        /* Fetch parameters. */
-       i = (int)_lwp_unpark_all(NULL, 0, NULL);
-       if (i == -1)
+       slen = _lwp_unpark_all(NULL, 0, NULL);
+       if (slen < 0)
                err(EXIT_FAILURE, "_lwp_unpark_all");
-       if (i < pthread__unpark_max)
-               pthread__unpark_max = i;
+       if ((size_t)slen < pthread__unpark_max)
+               pthread__unpark_max = slen;
 
        /* Basic data structure setup */
        pthread_attr_init(&pthread_default_attr);
@@ -604,16 +604,57 @@
  * awoken (because cancelled, for instance) make sure we have no waiters
  * left.
  */
-static void
+void
 pthread__clear_waiters(pthread_t self)
 {
+       int rv;
 
-       if (self->pt_nwaiters != 0) {
-               (void)_lwp_unpark_all(self->pt_waiters, self->pt_nwaiters,
-                   NULL);
-               self->pt_nwaiters = 0;
+       /* Zero waiters or one waiter in error case (pthread_exit()). */
+       if (self->pt_nwaiters == 0) {
+               if (self->pt_unpark != 0 && self->pt_willpark == 0) {
+                       rv = (ssize_t)_lwp_unpark(self->pt_unpark, NULL);
+                       self->pt_unpark = 0;
+                       if (rv != 0 && errno != EALREADY && errno != EINTR &&
+                           errno != ESRCH) {
+                               pthread__errorfunc(__FILE__, __LINE__, __func__,
+                                   "_lwp_unpark failed");
+                       }
+               }
+               return;
        }
-       self->pt_willpark = 0;
+
+       /* One waiter or two waiters (the second being a deferred wakeup). */
+       if (self->pt_nwaiters == 1) {
+               if (self->pt_unpark != 0) {
+                       /* Fall through to multiple waiters case. */
+                       self->pt_waiters[1] = self->pt_unpark;
+                       self->pt_nwaiters = 2;
+                       self->pt_unpark = 0;
+               } else if (self->pt_willpark) {
+                       /* Defer to _lwp_park(). */
+                       self->pt_unpark = self->pt_waiters[0];
+                       self->pt_nwaiters = 0;
+                       return;
+               } else {
+                       /* Wake one now. */
+                       rv = (ssize_t)_lwp_unpark(self->pt_waiters[0], NULL);
+                       self->pt_nwaiters = 0;
+                       if (rv != 0 && errno != EALREADY && errno != EINTR &&
+                           errno != ESRCH) {
+                               pthread__errorfunc(__FILE__, __LINE__, __func__,
+                                   "_lwp_unpark failed");
+                       }
+                       return;
+               }
+       }
+
+       /* Multiple waiters. */
+       rv = _lwp_unpark_all(self->pt_waiters, self->pt_nwaiters, NULL);
+       self->pt_nwaiters = 0;
+       if (rv != 0 && errno != EINTR) {
+               pthread__errorfunc(__FILE__, __LINE__, __func__,
+                   "_lwp_unpark_all failed");
+       }
 }
 
 void
@@ -630,6 +671,7 @@
        self = pthread__self();
 
        /* Disable cancellability. */
+       self->pt_willpark = 0;
        pthread_mutex_lock(&self->pt_lock);
        self->pt_flags |= PT_FLAG_CS_DISABLED;
        self->pt_cancel = 0;
@@ -660,10 +702,12 @@
        if (self->pt_flags & PT_FLAG_DETACHED) {
                /* pthread__reap() will drop the lock. */
                pthread__reap(self);
+               pthread__assert(!self->pt_willpark);
                pthread__clear_waiters(self);
                _lwp_exit();
        } else {
                self->pt_state = PT_STATE_ZOMBIE;
+               pthread__assert(!self->pt_willpark);
                pthread_mutex_unlock(&self->pt_lock);
                pthread__clear_waiters(self);
                /* Note: name will be freed by the joiner. */
@@ -1125,7 +1169,7 @@
 int
 pthread__park(pthread_t self, pthread_mutex_t *lock,
              pthread_queue_t *queue, const struct timespec *abstime,
-             int cancelpt, const void *hint)
+             int cancelpt)
 {
        int rv, error;
        void *obj;
@@ -1170,7 +1214,7 @@
                 * have _lwp_park() restart it before blocking.
                 */
                error = _lwp_park(CLOCK_REALTIME, TIMER_ABSTIME,
-                   __UNCONST(abstime), self->pt_unpark, hint, hint);
+                   __UNCONST(abstime), self->pt_unpark, NULL, NULL);
                self->pt_unpark = 0;
                if (error != 0) {
                        switch (rv = errno) {
@@ -1215,22 +1259,14 @@
                pthread_mutex_t *interlock)
 {
        pthread_t target;
-       u_int max;
-       size_t nwaiters;
 
-       max = pthread__unpark_max;
-       nwaiters = self->pt_nwaiters;
        target = PTQ_FIRST(queue);
-       if (nwaiters == max) {
-               /* Overflow. */
-               (void)_lwp_unpark_all(self->pt_waiters, nwaiters,
-                   __UNVOLATILE(&interlock->ptm_waiters));
-               nwaiters = 0;
+       if (self->pt_nwaiters == pthread__unpark_max) {
+               pthread__clear_waiters(self);
        }
        target->pt_sleepobj = NULL;
-       self->pt_waiters[nwaiters++] = target->pt_lid;
+       self->pt_waiters[self->pt_nwaiters++] = target->pt_lid;
        PTQ_REMOVE(queue, target, pt_sleep);
-       self->pt_nwaiters = nwaiters;
        pthread__mutex_deferwake(self, interlock);
 }
 
@@ -1238,23 +1274,16 @@
 pthread__unpark_all(pthread_queue_t *queue, pthread_t self,
                    pthread_mutex_t *interlock)
 {
+       const size_t max = pthread__unpark_max;
        pthread_t target;
-       u_int max;
-       size_t nwaiters;
 
-       max = pthread__unpark_max;
-       nwaiters = self->pt_nwaiters;
        PTQ_FOREACH(target, queue, pt_sleep) {
-               if (nwaiters == max) {
-                       /* Overflow. */
-                       (void)_lwp_unpark_all(self->pt_waiters, nwaiters,
-                           __UNVOLATILE(&interlock->ptm_waiters));
-                       nwaiters = 0;
+               if (self->pt_nwaiters == max) {
+                       pthread__clear_waiters(self);
                }
                target->pt_sleepobj = NULL;
-               self->pt_waiters[nwaiters++] = target->pt_lid;
+               self->pt_waiters[self->pt_nwaiters++] = target->pt_lid;
        }
-       self->pt_nwaiters = nwaiters;
        PTQ_INIT(queue);
        pthread__mutex_deferwake(self, interlock);
 }
diff -r fb7ec21dd705 -r 9fc14429d3c7 lib/libpthread/pthread_barrier.c
--- a/lib/libpthread/pthread_barrier.c  Sat May 16 22:07:06 2020 +0000
+++ b/lib/libpthread/pthread_barrier.c  Sat May 16 22:53:37 2020 +0000
@@ -1,7 +1,7 @@
-/*     $NetBSD: pthread_barrier.c,v 1.21 2020/01/29 14:41:57 kamil Exp $       */
+/*     $NetBSD: pthread_barrier.c,v 1.22 2020/05/16 22:53:37 ad Exp $  */
 
 /*-
- * Copyright (c) 2001, 2003, 2006, 2007, 2009 The NetBSD Foundation, Inc.
+ * Copyright (c) 2001, 2003, 2006, 2007, 2009, 2020 The NetBSD Foundation, Inc.
  * All rights reserved.
  *
  * This code is derived from software contributed to The NetBSD Foundation
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__RCSID("$NetBSD: pthread_barrier.c,v 1.21 2020/01/29 14:41:57 kamil Exp $");
+__RCSID("$NetBSD: pthread_barrier.c,v 1.22 2020/05/16 22:53:37 ad Exp $");
 
 #include <errno.h>
 
@@ -106,7 +106,7 @@
                PTQ_INSERT_TAIL(&barrier->ptb_waiters, self, pt_sleep);
                self->pt_sleepobj = &barrier->ptb_waiters;
                (void)pthread__park(self, interlock, &barrier->ptb_waiters,
-                   NULL, 0, __UNVOLATILE(&interlock->ptm_waiters));
+                   NULL, 0);
                if (__predict_true(gen != barrier->ptb_generation)) {
                        break;
                }
diff -r fb7ec21dd705 -r 9fc14429d3c7 lib/libpthread/pthread_cond.c
--- a/lib/libpthread/pthread_cond.c     Sat May 16 22:07:06 2020 +0000
+++ b/lib/libpthread/pthread_cond.c     Sat May 16 22:53:37 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: pthread_cond.c,v 1.68 2020/04/14 23:35:07 joerg Exp $  */
+/*     $NetBSD: pthread_cond.c,v 1.69 2020/05/16 22:53:37 ad Exp $     */
 
 /*-
  * Copyright (c) 2001, 2006, 2007, 2008 The NetBSD Foundation, Inc.
@@ -46,7 +46,7 @@
  */
 
 #include <sys/cdefs.h>
-__RCSID("$NetBSD: pthread_cond.c,v 1.68 2020/04/14 23:35:07 joerg Exp $");
+__RCSID("$NetBSD: pthread_cond.c,v 1.69 2020/05/16 22:53:37 ad Exp $");
 
 #include <stdlib.h>
 #include <errno.h>
@@ -161,9 +161,7 @@
                self->pt_willpark = 0;
                do {
                        retval = _lwp_park(clkid, TIMER_ABSTIME,
-                           __UNCONST(abstime), self->pt_unpark,
-                           __UNVOLATILE(&mutex->ptm_waiters),
-                           __UNVOLATILE(&mutex->ptm_waiters));
+                           __UNCONST(abstime), self->pt_unpark, NULL, NULL);
                        self->pt_unpark = 0;
                } while (retval == -1 && errno == ESRCH);
                pthread_mutex_lock(mutex);
@@ -254,9 +252,7 @@
         * caller (this thread) releases the mutex.
         */
        if (__predict_false(self->pt_nwaiters == (size_t)pthread__unpark_max)) {
-               (void)_lwp_unpark_all(self->pt_waiters, self->pt_nwaiters,
-                   __UNVOLATILE(&mutex->ptm_waiters));
-               self->pt_nwaiters = 0;
+               pthread__clear_waiters(self);
        }
        self->pt_waiters[self->pt_nwaiters++] = lid;
        pthread__mutex_deferwake(self, mutex);
@@ -284,7 +280,6 @@
        pthread_t self, signaled;
        pthread_mutex_t *mutex;
        u_int max;
-       size_t nwaiters;
 
        /*
         * Try to defer waking threads (see pthread_cond_signal()).
@@ -294,19 +289,14 @@
        pthread__spinlock(self, &cond->ptc_lock);
        max = pthread__unpark_max;
        mutex = cond->ptc_mutex;



Home | Main Index | Thread Index | Old Index