Index | Thread | Search

From:
David Gwynne <david@gwynne.id.au>
Subject:
avoid holding timeout_mutex and SCHED_LOCK at the same time (again)
To:
tech@openbsd.org
Date:
Wed, 30 Apr 2025 10:14:12 +1000

Download raw body.

Thread
  • David Gwynne:

    avoid holding timeout_mutex and SCHED_LOCK at the same time (again)

avoiding holding both locks was necessary back in kern_timeout.c r1.50,
but was lost when TIMEOUT_MPSAFE was introduced in r1.96. we've
obviously gotten away with it for the last year or so, but it feels
like a trap for the future.

ok?

Index: kern_timeout.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_timeout.c,v
diff -u -p -r1.101 kern_timeout.c
--- kern_timeout.c	13 Jan 2025 03:21:10 -0000	1.101
+++ kern_timeout.c	30 Apr 2025 00:09:02 -0000
@@ -782,12 +782,36 @@ softclock_create_thread(void *arg)
 #endif
 }
 
+static void
+softclock_thread_run(struct circq *todo)
+{
+	struct timeout *to;
+
+	for (;;) {
+		/*
+		 * Avoid holding both timeout_mutex and SCHED_LOCK
+		 * at the same time.
+		 */
+		sleep_setup(todo, PSWP, "tmoslp");
+		sleep_finish(0, CIRCQ_EMPTY(todo));
+
+		mtx_enter(&timeout_mutex);
+		tostat.tos_thread_wakeups++;
+		while (!CIRCQ_EMPTY(todo)) {
+			to = timeout_from_circq(CIRCQ_FIRST(todo));
+			CIRCQ_REMOVE(&to->to_list);
+			timeout_run(to);
+			tostat.tos_run_thread++;
+		}
+		mtx_leave(&timeout_mutex);
+	}
+}
+
 void
 softclock_thread(void *arg)
 {
 	CPU_INFO_ITERATOR cii;
 	struct cpu_info *ci;
-	struct timeout *to;
 	int s;
 
 	KERNEL_ASSERT_LOCKED();
@@ -801,18 +825,7 @@ softclock_thread(void *arg)
 	sched_peg_curproc(ci);
 
 	s = splsoftclock();
-	mtx_enter(&timeout_mutex);
-	for (;;) {
-		while (!CIRCQ_EMPTY(&timeout_proc)) {
-			to = timeout_from_circq(CIRCQ_FIRST(&timeout_proc));
-			CIRCQ_REMOVE(&to->to_list);
-			timeout_run(to);
-			tostat.tos_run_thread++;
-		}
-		tostat.tos_thread_wakeups++;
-		msleep_nsec(&timeout_proc, &timeout_mutex, PSWP, "tmoslp",
-		    INFSLP);
-	}
+	softclock_thread_run(&timeout_proc);
 	splx(s);
 }
 
@@ -820,23 +833,10 @@ softclock_thread(void *arg)
 void
 softclock_thread_mp(void *arg)
 {
-	struct timeout *to;
-
 	KERNEL_ASSERT_LOCKED();
 	KERNEL_UNLOCK();
 
-	mtx_enter(&timeout_mutex);
-	for (;;) {
-		while (!CIRCQ_EMPTY(&timeout_proc_mp)) {
-			to = timeout_from_circq(CIRCQ_FIRST(&timeout_proc_mp));
-			CIRCQ_REMOVE(&to->to_list);
-			timeout_run(to);
-			tostat.tos_run_thread++;
-		}
-		tostat.tos_thread_wakeups++;
-		msleep_nsec(&timeout_proc_mp, &timeout_mutex, PSWP, "tmoslp",
-		    INFSLP);
-	}
+	softclock_thread_run(&timeout_proc_mp);
 }
 #endif /* MULTIPROCESSOR */