From: Martin Pieuchot <mpi@openbsd.org>
Subject: reaper: free threads w/o KERNEL_LOCK()
To: tech@openbsd.org
Date: Wed, 24 Jul 2024 15:39:00 +0200

Use atomic operations to modify `nthreads' which allows us to remove the
lock dance around proc_free().

Note that I changed the semantic of fork_check_maxthread() to access the
variable only once.

ok?

Index: kern/kern_fork.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_fork.c,v
diff -u -p -r1.260 kern_fork.c
--- kern/kern_fork.c	3 Jun 2024 12:48:25 -0000	1.260
+++ kern/kern_fork.c	24 Jul 2024 12:40:43 -0000
@@ -65,7 +65,7 @@
 #include <machine/tcb.h>
 
 int	nprocesses = 1;		/* process 0 */
-int	nthreads = 1;		/* proc 0 */
+int	nthreads = 1;		/* [a] proc 0 */
 struct	forkstat forkstat;
 
 void fork_return(void *);
@@ -304,6 +304,8 @@ struct timeval fork_tfmrate = { 10, 0 };
 int
 fork_check_maxthread(uid_t uid)
 {
+	int val;
+
 	/*
 	 * Although process entries are dynamically created, we still keep
 	 * a global limit on the maximum number we will create. We reserve
@@ -313,14 +315,15 @@ fork_check_maxthread(uid_t uid)
 	 * the variable nthreads is the current number of procs, maxthread is
 	 * the limit.
 	 */
-	if ((nthreads >= maxthread - 5 && uid != 0) || nthreads >= maxthread) {
+	val = atomic_inc_int_nv(&nthreads);
+	if ((val >= maxthread - 5 && uid != 0) || val >= maxthread) {
 		static struct timeval lasttfm;
 
 		if (ratecheck(&lasttfm, &fork_tfmrate))
 			tablefull("thread");
+		atomic_dec_int(&nthreads);
 		return EAGAIN;
 	}
-	nthreads++;
 
 	return 0;
 }
@@ -366,7 +369,7 @@ fork1(struct proc *curp, int flags, void
 
 		if (ratecheck(&lasttfm, &fork_tfmrate))
 			tablefull("process");
-		nthreads--;
+		atomic_dec_int(&nthreads);
 		return EAGAIN;
 	}
 	nprocesses++;
@@ -379,7 +382,7 @@ fork1(struct proc *curp, int flags, void
 	if (uid != 0 && count > lim_cur(RLIMIT_NPROC)) {
 		(void)chgproccnt(uid, -1);
 		nprocesses--;
-		nthreads--;
+		atomic_dec_int(&nthreads);
 		return EAGAIN;
 	}
 
@@ -387,7 +390,7 @@ fork1(struct proc *curp, int flags, void
 	if (uaddr == 0) {
 		(void)chgproccnt(uid, -1);
 		nprocesses--;
-		nthreads--;
+		atomic_dec_int(&nthreads);
 		return (ENOMEM);
 	}
 
@@ -544,7 +547,7 @@ thread_fork(struct proc *curp, void *sta
 
 	uaddr = uvm_uarea_alloc();
 	if (uaddr == 0) {
-		nthreads--;
+		atomic_dec_int(&nthreads);
 		return ENOMEM;
 	}
 
Index: kern/kern_exit.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_exit.c,v
diff -u -p -r1.226 kern_exit.c
--- kern/kern_exit.c	24 Jul 2024 12:17:31 -0000	1.226
+++ kern/kern_exit.c	24 Jul 2024 12:40:43 -0000
@@ -429,7 +429,7 @@ proc_free(struct proc *p)
 {
 	crfree(p->p_ucred);
 	pool_put(&proc_pool, p);
-	nthreads--;
+	atomic_dec_int(&nthreads);
 }
 
 /*
@@ -468,9 +468,7 @@ reaper(void *arg)
 
 		if (p->p_flag & P_THREAD) {
 			/* Just a thread */
-			KERNEL_LOCK();
 			proc_free(p);
-			KERNEL_UNLOCK();
 		} else {
 			struct process *pr = p->p_p;