From d8b4e12631ad799d9f95efcdcc04086ad2e4bcc4 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Wed, 28 Feb 2024 03:55:49 +0000 Subject: [PATCH] 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 --- sys/kern/kern_heartbeat.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/sys/kern/kern_heartbeat.c b/sys/kern/kern_heartbeat.c index 8d4ea01880d1..4d13745a203b 100644 --- a/sys/kern/kern_heartbeat.c +++ b/sys/kern/kern_heartbeat.c @@ -132,24 +132,18 @@ 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. */ 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(); + KASSERT(curcpu_stable()); + p = &curcpu()->ci_heartbeat_suspend; atomic_store_relaxed(p, *p + 1); } @@ -186,6 +180,8 @@ 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. */ void heartbeat_resume(void) @@ -194,6 +190,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.