NetBSD-Bugs archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
kern/60739: Turnstile priority inheritance short-circuits walking of waiter chain when trylock fails
>Number: 60739
>Category: kern
>Synopsis: Turnstile priority inheritance short-circuits walking of waiter chain when trylock fails
>Confidential: no
>Severity: non-critical
>Priority: low
>Responsible: kern-bug-people
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Thu Sep 17 20:35:01 +0000 2026
>Originator: Loïc Grégoire
>Release: trunk
>Organization:
>Environment:
>Description:
This particular block of code in the turnstile implementation (kern_turnstile.c) is problematic:
/*
* If the owner's priority is already higher than ours,
* there's nothing to do anymore.
*/
if (prio <= lwp_eprio(owner)) {
if (dolock)
lwp_unlock(owner);
break;
}
This seems fine (and even clever) at first glance, because it avoids walking the whole waiter chain when we know the other threads are already boosted. The problem is that this is broken when the trylock fails and the whole walk is restarted:
/*
* The owner was changed behind us or trylock failed.
* Restart from curlwp.
*
* Note that there may be a livelock here:
* the owner may try grabbing cur's lock (which is the
* tc lock) while we're trying to grab the owner's lock.
*/
lwp_unlock(l);
l = cur;
lwp_lock(l);
prio = lwp_eprio(l);
continue;
Notice that in the chain:
A (curlwp) -> B -> C
If we fail to acquire the lock between B and C, the whole walk is restarted from A. The above problematic condition is triggered immediately and the loop is broken out of; C never got its priority boosted!
Attached is a patch that fixes this issue, I also appended a comment to clarify that re-boosting threads is perfectly fine.
If this is not the suitable medium to send such a patch, please tell me so and I will do it differently.
>How-To-Repeat:
>Fix:
diff --git a/sys/kern/kern_turnstile.c b/sys/kern/kern_turnstile.c
index bc1be507a..b8e002173 100644
--- a/sys/kern/kern_turnstile.c
+++ b/sys/kern/kern_turnstile.c
@@ -254,7 +254,9 @@ turnstile_lendpri(lwp_t *cur)
if (l == owner || (dolock && !lwp_trylock(owner))) {
/*
* The owner was changed behind us or trylock failed.
- * Restart from curlwp.
+ * Restart from curlwp. Priority inheritance is
+ * idempotent, so there is no issue with restarting from curlwp; the
+ * priorities we already updated will simply get re-applied again.
*
* Note that there may be a livelock here:
* the owner may try grabbing cur's lock (which is the
@@ -266,15 +268,7 @@ turnstile_lendpri(lwp_t *cur)
prio = lwp_eprio(l);
continue;
}
- /*
- * If the owner's priority is already higher than ours,
- * there's nothing to do anymore.
- */
- if (prio <= lwp_eprio(owner)) {
- if (dolock)
- lwp_unlock(owner);
- break;
- }
+
/*
* Lend our priority to the 'owner' LWP.
*
Home |
Main Index |
Thread Index |
Old Index