Make __thrsleep(2) and __thrwakeup(2) MP-safe

Threads in __thrsleep(2) are tracked using queues, one queue per each
process for synchronization between threads of a process, and one
system-wide queue for the special ident -1 handling. Each of these
queues has an associated rwlock that serializes access.

The queue lock is released when calling copyin() and copyout() in
thrsleep(). This preserves the existing behaviour where a blocked copy
operation does not prevent other threads from making progress.

Tested by anton@, claudio@
OK anton@, claudio@, tedu@, mpi@

4 files changed, 86 insertions, 39 deletions

diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c
index a9eebaaa35c..1f74865ed6a 100644
--- a/sys/kern/kern_fork.c
+++ b/sys/kern/kern_fork.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_fork.c,v 1.220 2020/01/06 10:25:10 visa Exp $	*/
+/*	$OpenBSD: kern_fork.c,v 1.221 2020/01/21 15:20:47 visa Exp $	*/
 /*	$NetBSD: kern_fork.c,v 1.29 1996/02/09 18:59:34 christos Exp $	*/
 
 /*
@@ -191,7 +191,9 @@ process_initialize(struct process *pr, struct proc *p)
 	LIST_INIT(&pr->ps_children);
 	LIST_INIT(&pr->ps_ftlist);
 	LIST_INIT(&pr->ps_sigiolst);
+	TAILQ_INIT(&pr->ps_tslpqueue);
 
+	rw_init(&pr->ps_lock, "pslock");
 	mtx_init(&pr->ps_mtx, IPL_MPFLOOR);
 
 	timeout_set(&pr->ps_realit_to, realitexpire, pr);
diff --git a/sys/kern/kern_synch.c b/sys/kern/kern_synch.c
index da5b4e9b34e..02c733ad8f1 100644
--- a/sys/kern/kern_synch.c
+++ b/sys/kern/kern_synch.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_synch.c,v 1.158 2020/01/16 16:35:04 mpi Exp $	*/
+/*	$OpenBSD: kern_synch.c,v 1.159 2020/01/21 15:20:47 visa Exp $	*/
 /*	$NetBSD: kern_synch.c,v 1.37 1996/04/22 01:38:37 christos Exp $	*/
 
 /*
@@ -634,7 +634,14 @@ thrsleep_unlock(void *lock)
 	return copyout(&unlocked, atomiclock, sizeof(unlocked));
 }
 
-static int globalsleepaddr;
+struct tslpentry {
+	TAILQ_ENTRY(tslpentry)	tslp_link;
+	long			tslp_ident;
+};
+
+/* thrsleep queue shared between processes */
+static struct tslpqueue thrsleep_queue = TAILQ_HEAD_INITIALIZER(thrsleep_queue);
+static struct rwlock thrsleep_lock = RWLOCK_INITIALIZER("thrsleeplk");
 
 int
 thrsleep(struct proc *p, struct sys___thrsleep_args *v)
@@ -647,10 +654,13 @@ thrsleep(struct proc *p, struct sys___thrsleep_args *v)
 		syscallarg(const int *) abort;
 	} */ *uap = v;
 	long ident = (long)SCARG(uap, ident);
+	struct tslpentry entry;
+	struct tslpqueue *queue;
+	struct rwlock *qlock;
 	struct timespec *tsp = (struct timespec *)SCARG(uap, tp);
 	void *lock = SCARG(uap, lock);
 	uint64_t nsecs = INFSLP;
-	int abort, error;
+	int abort = 0, error;
 	clockid_t clock_id = SCARG(uap, clock_id);
 
 	if (ident == 0)
@@ -676,32 +686,41 @@ thrsleep(struct proc *p, struct sys___thrsleep_args *v)
 		nsecs = TIMESPEC_TO_NSEC(tsp);
 	}
 
-	p->p_thrslpid = ident;
+	if (ident == -1) {
+		queue = &thrsleep_queue;
+		qlock = &thrsleep_lock;
+	} else {
+		queue = &p->p_p->ps_tslpqueue;
+		qlock = &p->p_p->ps_lock;
+	}
 
-	if ((error = thrsleep_unlock(lock)))
-		goto out;
+	/* Interlock with wakeup. */
+	entry.tslp_ident = ident;
+	rw_enter_write(qlock);
+	TAILQ_INSERT_TAIL(queue, &entry, tslp_link);
+	rw_exit_write(qlock);
 
-	if (SCARG(uap, abort) != NULL) {
-		if ((error = copyin(SCARG(uap, abort), &abort,
-		    sizeof(abort))) != 0)
-			goto out;
-		if (abort) {
-			error = EINTR;
-			goto out;
-		}
-	}
+	error = thrsleep_unlock(lock);
 
-	if (p->p_thrslpid == 0)
-		error = 0;
-	else {
-		void *sleepaddr = &p->p_thrslpid;
-		if (ident == -1)
-			sleepaddr = &globalsleepaddr;
-		error = tsleep_nsec(sleepaddr, PWAIT|PCATCH, "thrsleep", nsecs);
+	if (error == 0 && SCARG(uap, abort) != NULL)
+		error = copyin(SCARG(uap, abort), &abort, sizeof(abort));
+
+	rw_enter_write(qlock);
+	if (error != 0)
+		goto out;
+	if (abort != 0) {
+		error = EINTR;
+		goto out;
+	}
+	if (entry.tslp_ident != 0) {
+		error = rwsleep_nsec(&entry, qlock, PWAIT|PCATCH, "thrsleep",
+		    nsecs);
 	}
 
 out:
-	p->p_thrslpid = 0;
+	if (entry.tslp_ident != 0)
+		TAILQ_REMOVE(queue, &entry, tslp_link);
+	rw_exit_write(qlock);
 
 	if (error == ERESTART)
 		error = ECANCELED;
@@ -746,25 +765,48 @@ sys___thrwakeup(struct proc *p, void *v, register_t *retval)
 		syscallarg(const volatile void *) ident;
 		syscallarg(int) n;
 	} */ *uap = v;
+	struct tslpentry *entry, *tmp;
+	struct tslpqueue *queue;
+	struct rwlock *qlock;
 	long ident = (long)SCARG(uap, ident);
 	int n = SCARG(uap, n);
-	struct proc *q;
 	int found = 0;
 
 	if (ident == 0)
 		*retval = EINVAL;
-	else if (ident == -1)
-		wakeup(&globalsleepaddr);
 	else {
-		TAILQ_FOREACH(q, &p->p_p->ps_threads, p_thr_link) {
-			if (q->p_thrslpid == ident) {
-				wakeup_one(&q->p_thrslpid);
-				q->p_thrslpid = 0;
+		if (ident == -1) {
+			queue = &thrsleep_queue;
+			qlock = &thrsleep_lock;
+			/*
+			 * Wake up all waiters with ident -1. This is needed
+			 * because ident -1 can be shared by multiple userspace
+			 * lock state machines concurrently. The implementation
+			 * has no way to direct the wakeup to a particular
+			 * state machine.
+			 */
+			n = 0;
+		} else {
+			queue = &p->p_p->ps_tslpqueue;
+			qlock = &p->p_p->ps_lock;
+		}
+
+		rw_enter_write(qlock);
+		TAILQ_FOREACH_SAFE(entry, queue, tslp_link, tmp) {
+			if (entry->tslp_ident == ident) {
+				TAILQ_REMOVE(queue, entry, tslp_link);
+				entry->tslp_ident = 0;
+				wakeup_one(entry);
 				if (++found == n)
 					break;
 			}
 		}
-		*retval = found ? 0 : ESRCH;
+		rw_exit_write(qlock);
+
+		if (ident == -1)
+			*retval = 0;
+		else
+			*retval = found ? 0 : ESRCH;
 	}
 
 	return (0);
diff --git a/sys/kern/syscalls.master b/sys/kern/syscalls.master
index 3d0ef25105a..3d823d74e0b 100644
--- a/sys/kern/syscalls.master
+++ b/sys/kern/syscalls.master
@@ -1,4 +1,4 @@
-;	$OpenBSD: syscalls.master,v 1.201 2020/01/13 14:01:18 visa Exp $
+;	$OpenBSD: syscalls.master,v 1.202 2020/01/21 15:20:47 visa Exp $
 ;	$NetBSD: syscalls.master,v 1.32 1996/04/23 10:24:21 mycroft Exp $
 
 ;	@(#)syscalls.master	8.2 (Berkeley) 1/13/94
@@ -206,7 +206,7 @@
 92	STD		{ int sys_fcntl(int fd, int cmd, ... void *arg); }
 93	STD		{ int sys_accept4(int s, struct sockaddr *name, \
 			    socklen_t *anamelen, int flags); }
-94	STD		{ int sys___thrsleep(const volatile void *ident, \
+94	STD NOLOCK	{ int sys___thrsleep(const volatile void *ident, \
 			    clockid_t clock_id, const struct timespec *tp, \
 			    void *lock, const int *abort); }
 95	STD		{ int sys_fsync(int fd); }
@@ -521,7 +521,7 @@
 298	STD		{ int sys_sched_yield(void); }
 299	STD NOLOCK	{ pid_t sys_getthrid(void); }
 300	OBSOL		t32___thrsleep
-301	STD		{ int sys___thrwakeup(const volatile void *ident, \
+301	STD NOLOCK	{ int sys___thrwakeup(const volatile void *ident, \
 			    int n); }
 302	STD		{ void sys___threxit(pid_t *notdead); }
 303	STD		{ int sys___thrsigdivert(sigset_t sigmask, \
diff --git a/sys/sys/proc.h b/sys/sys/proc.h
index c463e736f8a..0648c02cfd2 100644
--- a/sys/sys/proc.h
+++ b/sys/sys/proc.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: proc.h,v 1.285 2020/01/16 16:35:04 mpi Exp $	*/
+/*	$OpenBSD: proc.h,v 1.286 2020/01/21 15:20:47 visa Exp $	*/
 /*	$NetBSD: proc.h,v 1.44 1996/04/22 01:23:21 christos Exp $	*/
 
 /*-
@@ -153,11 +153,14 @@ RBT_HEAD(unvname_rbt, unvname);
 #ifdef __need_process
 struct futex;
 LIST_HEAD(futex_list, futex);
+struct tslpentry;
+TAILQ_HEAD(tslpqueue, tslpentry);
 struct unveil;
 
 /*
- *  Locks used to protect struct members in this file:
+ * Locks used to protect struct members in this file:
  *	m	this process' `ps_mtx'
+ *	p	this process' `ps_lock'
  *	r	rlimit_lock
  */
 struct process {
@@ -186,6 +189,8 @@ struct process {
 	pid_t	ps_pid;			/* Process identifier. */
 
 	struct	futex_list ps_ftlist;	/* futexes attached to this process */
+	struct	tslpqueue ps_tslpqueue;	/* [p] queue of threads in thrsleep */
+	struct	rwlock	ps_lock;	/* per-process rwlock */
 	struct  mutex	ps_mtx;		/* per-process mutex */
 
 /* The following fields are all zeroed upon creation in process_new. */
@@ -363,8 +368,6 @@ struct proc {
 	u_int	p_iticks;		/* Statclock hits processing intr. */
 	struct	cpu_info * volatile p_cpu; /* [s] CPU we're running on. */
 
-	long 	p_thrslpid;		/* for thrsleep syscall */
-
 	struct	rusage p_ru;		/* Statistics */
 	struct	tusage p_tu;		/* accumulated times. */
 	struct	timespec p_rtime;	/* Real time. */