Module Name: src
Committed By: ad
Date: Mon Jan 13 18:22:56 UTC 2020
Modified Files:
src/lib/libpthread: pthread.c pthread_cond.c pthread_int.h
pthread_misc.c pthread_mutex.c pthread_rwlock.c
Log Message:
Rip out some very ambitious optimisations around pthread_mutex that are
don't buy much. This stuff is hard enough to get right in the kernel let
alone userspace, and I don't trust that it's right.
To generate a diff of this commit:
cvs rdiff -u -r1.153 -r1.154 src/lib/libpthread/pthread.c
cvs rdiff -u -r1.65 -r1.66 src/lib/libpthread/pthread_cond.c \
src/lib/libpthread/pthread_mutex.c
cvs rdiff -u -r1.97 -r1.98 src/lib/libpthread/pthread_int.h
cvs rdiff -u -r1.15 -r1.16 src/lib/libpthread/pthread_misc.c
cvs rdiff -u -r1.36 -r1.37 src/lib/libpthread/pthread_rwlock.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/lib/libpthread/pthread.c
diff -u src/lib/libpthread/pthread.c:1.153 src/lib/libpthread/pthread.c:1.154
--- src/lib/libpthread/pthread.c:1.153 Tue Mar 5 01:35:52 2019
+++ src/lib/libpthread/pthread.c Mon Jan 13 18:22:56 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: pthread.c,v 1.153 2019/03/05 01:35:52 christos Exp $ */
+/* $NetBSD: pthread.c,v 1.154 2020/01/13 18:22:56 ad Exp $ */
/*-
* Copyright (c) 2001, 2002, 2003, 2006, 2007, 2008 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
*/
#include <sys/cdefs.h>
-__RCSID("$NetBSD: pthread.c,v 1.153 2019/03/05 01:35:52 christos Exp $");
+__RCSID("$NetBSD: pthread.c,v 1.154 2020/01/13 18:22:56 ad Exp $");
#define __EXPOSE_STACK 1
@@ -319,7 +319,6 @@ pthread__initthread(pthread_t t)
t->pt_havespecific = 0;
t->pt_early = NULL;
t->pt_lwpctl = &pthread__dummy_lwpctl;
- t->pt_blocking = 0;
t->pt_droplock = NULL;
memcpy(&t->pt_lockops, pthread__lock_ops, sizeof(t->pt_lockops));
@@ -1157,15 +1156,9 @@ pthread__park(pthread_t self, pthread_mu
int rv, error;
void *obj;
- /*
- * For non-interlocked release of mutexes we need a store
- * barrier before incrementing pt_blocking away from zero.
- * This is provided by pthread_mutex_unlock().
- */
self->pt_willpark = 1;
pthread_mutex_unlock(lock);
self->pt_willpark = 0;
- self->pt_blocking++;
/*
* Wait until we are awoken by a pending unpark operation,
@@ -1239,8 +1232,6 @@ pthread__park(pthread_t self, pthread_mu
pthread_mutex_unlock(lock);
}
self->pt_early = NULL;
- self->pt_blocking--;
- membar_sync();
return rv;
}
Index: src/lib/libpthread/pthread_cond.c
diff -u src/lib/libpthread/pthread_cond.c:1.65 src/lib/libpthread/pthread_cond.c:1.66
--- src/lib/libpthread/pthread_cond.c:1.65 Fri Dec 8 03:08:19 2017
+++ src/lib/libpthread/pthread_cond.c Mon Jan 13 18:22:56 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: pthread_cond.c,v 1.65 2017/12/08 03:08:19 christos Exp $ */
+/* $NetBSD: pthread_cond.c,v 1.66 2020/01/13 18:22:56 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.65 2017/12/08 03:08:19 christos Exp $");
+__RCSID("$NetBSD: pthread_cond.c,v 1.66 2020/01/13 18:22:56 ad Exp $");
#include <stdlib.h>
#include <errno.h>
@@ -164,7 +164,6 @@ pthread_cond_timedwait(pthread_cond_t *c
self->pt_willpark = 1;
pthread_mutex_unlock(mutex);
self->pt_willpark = 0;
- self->pt_blocking++;
do {
retval = _lwp_park(clkid, TIMER_ABSTIME,
__UNCONST(abstime), self->pt_unpark,
@@ -172,8 +171,6 @@ pthread_cond_timedwait(pthread_cond_t *c
__UNVOLATILE(&mutex->ptm_waiters));
self->pt_unpark = 0;
} while (retval == -1 && errno == ESRCH);
- self->pt_blocking--;
- membar_sync();
pthread_mutex_lock(mutex);
/*
Index: src/lib/libpthread/pthread_mutex.c
diff -u src/lib/libpthread/pthread_mutex.c:1.65 src/lib/libpthread/pthread_mutex.c:1.66
--- src/lib/libpthread/pthread_mutex.c:1.65 Tue Mar 5 22:49:38 2019
+++ src/lib/libpthread/pthread_mutex.c Mon Jan 13 18:22:56 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: pthread_mutex.c,v 1.65 2019/03/05 22:49:38 christos Exp $ */
+/* $NetBSD: pthread_mutex.c,v 1.66 2020/01/13 18:22:56 ad Exp $ */
/*-
* Copyright (c) 2001, 2003, 2006, 2007, 2008 The NetBSD Foundation, Inc.
@@ -47,7 +47,7 @@
*/
#include <sys/cdefs.h>
-__RCSID("$NetBSD: pthread_mutex.c,v 1.65 2019/03/05 22:49:38 christos Exp $");
+__RCSID("$NetBSD: pthread_mutex.c,v 1.66 2020/01/13 18:22:56 ad Exp $");
#include <sys/types.h>
#include <sys/lwpctl.h>
@@ -235,10 +235,7 @@ pthread__mutex_pause(void)
/*
* Spin while the holder is running. 'lwpctl' gives us the true
- * status of the thread. pt_blocking is set by libpthread in order
- * to cut out system call and kernel spinlock overhead on remote CPUs
- * (could represent many thousands of clock cycles). pt_blocking also
- * makes this thread yield if the target is calling sched_yield().
+ * status of the thread.
*/
NOINLINE static void *
pthread__mutex_spin(pthread_mutex_t *ptm, pthread_t owner)
@@ -250,8 +247,7 @@ pthread__mutex_spin(pthread_mutex_t *ptm
thread = (pthread_t)MUTEX_OWNER(owner);
if (thread == NULL)
break;
- if (thread->pt_lwpctl->lc_curcpu == LWPCTL_CPU_NONE ||
- thread->pt_blocking)
+ if (thread->pt_lwpctl->lc_curcpu == LWPCTL_CPU_NONE)
break;
if (count < 128)
count += count;
@@ -262,10 +258,10 @@ pthread__mutex_spin(pthread_mutex_t *ptm
return owner;
}
-NOINLINE static void
+NOINLINE static bool
pthread__mutex_setwaiters(pthread_t self, pthread_mutex_t *ptm)
{
- void *new, *owner;
+ void *owner, *next;
/*
* Note that the mutex can become unlocked before we set
@@ -281,34 +277,16 @@ pthread__mutex_setwaiters(pthread_t self
* the value of ptm_owner/pt_mutexwait after we have entered
* the waiters list (the CAS itself must be atomic).
*/
-again:
- membar_consumer();
- owner = ptm->ptm_owner;
-
- if (MUTEX_OWNER(owner) == 0) {
- pthread__mutex_wakeup(self, ptm);
- return;
- }
- if (!MUTEX_HAS_WAITERS(owner)) {
- new = (void *)((uintptr_t)owner | MUTEX_WAITERS_BIT);
- if (atomic_cas_ptr(&ptm->ptm_owner, owner, new) != owner) {
- goto again;
+ for (owner = ptm->ptm_owner;; owner = next) {
+ if (MUTEX_OWNER(owner) == 0) {
+ pthread__mutex_wakeup(self, ptm);
+ return true;
}
- }
-
- /*
- * Note that pthread_mutex_unlock() can do a non-interlocked CAS.
- * We cannot know if the presence of the waiters bit is stable
- * while the holding thread is running. There are many assumptions;
- * see sys/kern/kern_mutex.c for details. In short, we must spin if
- * we see that the holder is running again.
- */
- membar_sync();
- if (MUTEX_OWNER(owner) != (uintptr_t)self)
- pthread__mutex_spin(ptm, owner);
-
- if (membar_consumer(), !MUTEX_HAS_WAITERS(ptm->ptm_owner)) {
- goto again;
+ if (MUTEX_HAS_WAITERS(owner)) {
+ return false;
+ }
+ next = atomic_cas_ptr(&ptm->ptm_owner, owner,
+ (void *)((uintptr_t)owner | MUTEX_WAITERS_BIT));
}
}
@@ -386,9 +364,12 @@ pthread__mutex_lock_slow(pthread_mutex_t
if (next == waiters)
break;
}
-
+
/* Set the waiters bit and block. */
- pthread__mutex_setwaiters(self, ptm);
+ membar_sync();
+ if (pthread__mutex_setwaiters(self, ptm)) {
+ continue;
+ }
/*
* We may have been awoken by the current thread above,
@@ -398,15 +379,13 @@ pthread__mutex_lock_slow(pthread_mutex_t
* being set to zero). Otherwise it is unsafe to re-enter
* the thread onto the waiters list.
*/
+ membar_sync();
while (self->pt_mutexwait) {
- self->pt_blocking++;
error = _lwp_park(CLOCK_REALTIME, TIMER_ABSTIME,
__UNCONST(ts), self->pt_unpark,
__UNVOLATILE(&ptm->ptm_waiters),
__UNVOLATILE(&ptm->ptm_waiters));
self->pt_unpark = 0;
- self->pt_blocking--;
- membar_sync();
if (__predict_true(error != -1)) {
continue;
}
@@ -471,15 +450,11 @@ pthread_mutex_unlock(pthread_mutex_t *pt
if (__predict_false(__uselibcstub))
return __libc_mutex_unlock_stub(ptm);
- /*
- * Note this may be a non-interlocked CAS. See lock_slow()
- * above and sys/kern/kern_mutex.c for details.
- */
#ifndef PTHREAD__ATOMIC_IS_MEMBAR
membar_exit();
#endif
self = pthread__self();
- value = atomic_cas_ptr_ni(&ptm->ptm_owner, self, NULL);
+ value = atomic_cas_ptr(&ptm->ptm_owner, self, NULL);
if (__predict_true(value == self)) {
pthread__smt_wake();
return 0;
@@ -582,12 +557,9 @@ pthread__mutex_wakeup(pthread_t self, pt
pthread_t thread, next;
ssize_t n, rv;
- /*
- * Take ownership of the current set of waiters. No
- * need for a memory barrier following this, all loads
- * are dependent upon 'thread'.
- */
+ /* Take ownership of the current set of waiters. */
thread = atomic_swap_ptr(&ptm->ptm_waiters, NULL);
+ membar_datadep_consumer(); /* for alpha */
pthread__smt_wake();
for (;;) {
Index: src/lib/libpthread/pthread_int.h
diff -u src/lib/libpthread/pthread_int.h:1.97 src/lib/libpthread/pthread_int.h:1.98
--- src/lib/libpthread/pthread_int.h:1.97 Wed Dec 18 15:11:57 2019
+++ src/lib/libpthread/pthread_int.h Mon Jan 13 18:22:56 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: pthread_int.h,v 1.97 2019/12/18 15:11:57 joerg Exp $ */
+/* $NetBSD: pthread_int.h,v 1.98 2020/01/13 18:22:56 ad Exp $ */
/*-
* Copyright (c) 2001, 2002, 2003, 2006, 2007, 2008 The NetBSD Foundation, Inc.
@@ -131,7 +131,6 @@ struct __pthread_st {
*/
int pt_dummy1 __aligned(128);
struct lwpctl *pt_lwpctl; /* Kernel/user comms area */
- volatile int pt_blocking; /* Blocking in userspace */
volatile int pt_rwlocked; /* Handed rwlock successfully */
volatile int pt_signalled; /* Received pthread_cond_signal() */
volatile int pt_mutexwait; /* Waiting to acquire mutex */
Index: src/lib/libpthread/pthread_misc.c
diff -u src/lib/libpthread/pthread_misc.c:1.15 src/lib/libpthread/pthread_misc.c:1.16
--- src/lib/libpthread/pthread_misc.c:1.15 Thu Mar 21 16:49:12 2013
+++ src/lib/libpthread/pthread_misc.c Mon Jan 13 18:22:56 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: pthread_misc.c,v 1.15 2013/03/21 16:49:12 christos Exp $ */
+/* $NetBSD: pthread_misc.c,v 1.16 2020/01/13 18:22:56 ad Exp $ */
/*-
* Copyright (c) 2001, 2006, 2007, 2008 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
*/
#include <sys/cdefs.h>
-__RCSID("$NetBSD: pthread_misc.c,v 1.15 2013/03/21 16:49:12 christos Exp $");
+__RCSID("$NetBSD: pthread_misc.c,v 1.16 2020/01/13 18:22:56 ad Exp $");
#include <errno.h>
#include <string.h>
@@ -151,20 +151,9 @@ pthread_sigmask(int how, const sigset_t
int
pthread__sched_yield(void)
{
- pthread_t self;
- int error;
if (__predict_false(__uselibcstub))
return __libc_thr_yield();
- self = pthread__self();
-
- /* Memory barrier for unlocked mutex release. */
- membar_producer();
- self->pt_blocking++;
- error = _sys_sched_yield();
- self->pt_blocking--;
- membar_sync();
-
- return error;
+ return _sys_sched_yield();
}
Index: src/lib/libpthread/pthread_rwlock.c
diff -u src/lib/libpthread/pthread_rwlock.c:1.36 src/lib/libpthread/pthread_rwlock.c:1.37
--- src/lib/libpthread/pthread_rwlock.c:1.36 Mon Dec 16 22:22:11 2019
+++ src/lib/libpthread/pthread_rwlock.c Mon Jan 13 18:22:56 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: pthread_rwlock.c,v 1.36 2019/12/16 22:22:11 uwe Exp $ */
+/* $NetBSD: pthread_rwlock.c,v 1.37 2020/01/13 18:22:56 ad Exp $ */
/*-
* Copyright (c) 2002, 2006, 2007, 2008 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
*/
#include <sys/cdefs.h>
-__RCSID("$NetBSD: pthread_rwlock.c,v 1.36 2019/12/16 22:22:11 uwe Exp $");
+__RCSID("$NetBSD: pthread_rwlock.c,v 1.37 2020/01/13 18:22:56 ad Exp $");
#include <sys/types.h>
#include <sys/lwpctl.h>
@@ -139,8 +139,7 @@ pthread__rwlock_spin(uintptr_t owner)
thread = (pthread_t)(owner & RW_THREAD);
if (__predict_false(thread == NULL) ||
- thread->pt_lwpctl->lc_curcpu == LWPCTL_CPU_NONE ||
- thread->pt_blocking)
+ thread->pt_lwpctl->lc_curcpu == LWPCTL_CPU_NONE)
return 0;
for (i = 128; i != 0; i--)
