Module Name: src
Committed By: riastradh
Date: Wed Feb 28 04:13:00 UTC 2024
Modified Files:
src/sys/kern: kern_heartbeat.c
Log Message:
heartbeat(9): No kpreempt_disable/enable in heartbeat_suspend/resume.
This causes a leak of l_nopreempt in xc_thread when a CPU is offlined
and onlined again, because the offlining heartbeat_suspend and the
onlining heartbeat_resume happen in separate xcalls.
No change to callers because they are already bound to the CPU:
1. cnpollc does kpreempt_disable/enable itself around the calls to
heartbeat_suspend/resume anyway
2. cpu_xc_offline/online run in the xcall thread, which is always
bound to the CPU that is being offlined or onlined
To generate a diff of this commit:
cvs rdiff -u -r1.10 -r1.11 src/sys/kern/kern_heartbeat.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/kern/kern_heartbeat.c
diff -u src/sys/kern/kern_heartbeat.c:1.10 src/sys/kern/kern_heartbeat.c:1.11
--- src/sys/kern/kern_heartbeat.c:1.10 Wed Sep 6 12:29:14 2023
+++ src/sys/kern/kern_heartbeat.c Wed Feb 28 04:12:59 2024
@@ -1,4 +1,4 @@
-/* $NetBSD: kern_heartbeat.c,v 1.10 2023/09/06 12:29:14 riastradh Exp $ */
+/* $NetBSD: kern_heartbeat.c,v 1.11 2024/02/28 04:12:59 riastradh Exp $ */
/*-
* Copyright (c) 2023 The NetBSD Foundation, Inc.
@@ -82,7 +82,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_heartbeat.c,v 1.10 2023/09/06 12:29:14 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_heartbeat.c,v 1.11 2024/02/28 04:12:59 riastradh Exp $");
#ifdef _KERNEL_OPT
#include "opt_ddb.h"
@@ -132,24 +132,20 @@ void *heartbeat_sih __read_mostly;
*
* Called after the current CPU has been marked offline but before
* it has stopped running, or after IPL has been raised for
- * polling-mode console input. Binds to the current CPU as a side
- * effect. Nestable (but only up to 2^32 times, so don't do this
- * in a loop). Reversed by heartbeat_resume.
+ * polling-mode console input. Nestable. Reversed by
+ * heartbeat_resume.
+ *
+ * Caller must be bound to the CPU, i.e., curcpu_stable() must be
+ * true. This function does not assert curcpu_stable() since it
+ * is used in the ddb entry path, where any assertions risk
+ * infinite regress into undebuggable chaos, so callers must be
+ * careful.
*/
void
heartbeat_suspend(void)
{
unsigned *p;
- /*
- * We could use curlwp_bind, but we'd have to record whether we
- * were already bound or not to pass to curlwp_bindx in
- * heartbeat_resume. Using kpreempt_disable is simpler and
- * unlikely to have any adverse consequences, since this only
- * happens when we're about to go into a tight polling loop at
- * raised IPL anyway.
- */
- kpreempt_disable();
p = &curcpu()->ci_heartbeat_suspend;
atomic_store_relaxed(p, *p + 1);
}
@@ -186,6 +182,9 @@ heartbeat_resume_cpu(struct cpu_info *ci
* Called after the current CPU has started running but before it
* has been marked online, or when ending polling-mode input
* before IPL is restored. Reverses heartbeat_suspend.
+ *
+ * Caller must be bound to the CPU, i.e., curcpu_stable() must be
+ * true.
*/
void
heartbeat_resume(void)
@@ -194,6 +193,8 @@ heartbeat_resume(void)
unsigned *p;
int s;
+ KASSERT(curcpu_stable());
+
/*
* Reset the state so nobody spuriously thinks we had a heart
* attack as soon as the heartbeat checks resume.
@@ -204,7 +205,6 @@ heartbeat_resume(void)
p = &ci->ci_heartbeat_suspend;
atomic_store_relaxed(p, *p - 1);
- kpreempt_enable();
}
/*
