NetBSD Problem Report #60739

From www@netbsd.org  Thu Sep 17 20:33:37 2026
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
	 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256
	 client-signature RSA-PSS (2048 bits) client-digest SHA256)
	(Client CN "mail.netbsd.org", Issuer "YR1" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id DB7951A923B
	for <gnats-bugs@gnats.NetBSD.org>; Thu, 17 Sep 2026 20:33:37 +0000 (UTC)
Message-Id: <20260917203336.66AE21A923C@mollari.NetBSD.org>
Date: Thu, 17 Sep 2026 20:33:36 +0000 (UTC)
From: loicgre@gmail.com
Reply-To: loicgre@gmail.com
To: gnats-bugs@NetBSD.org
Subject: Turnstile priority inheritance short-circuits walking of waiter chain when trylock fails
X-Send-Pr-Version: www-1.0
X-From4GNATS: "loicgre@gmail.com via gnats" <gnats-admin@NetBSD.org>

>Number:         60739
>Notify-List:    riastradh, yamt
>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
>Closed-Date:    
>Last-Modified:  Thu Oct 01 13: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.
 		 *

>Release-Note:

>Audit-Trail:

State-Changed-From-To: open->feedback
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Fri, 18 Sep 2026 12:40:17 +0000
State-Changed-Why:
Candidate fix commmitted -- I kept the optimization as long as we have
never failed a trylock and restarted, and added your comment.  How
does it look now?

(Also, out of curiosity: how did you find this?)


From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/60739 CVS commit: src/sys/kern
Date: Fri, 18 Sep 2026 12:38:43 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Fri Sep 18 12:38:43 UTC 2026

 Modified Files:
 	src/sys/kern: kern_turnstile.c

 Log Message:
 turnstile(9): Fix priority inheritance if trylock requires restart.

 While here, sprinkle some comments explaining what's going on.

 Not 100% sure this optimization, which was added in 2007 by yamt@ in
 the original implementation of priority inheritance, is worthwhile,
 but it's easy enough to fix for now.

 PR kern/60739: Turnstile priority inheritance short-circuits walking
 of waiter chain when trylock fails


 To generate a diff of this commit:
 cvs rdiff -u -r1.56 -r1.57 src/sys/kern/kern_turnstile.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: loicgre@gmail.com
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/60739 (Turnstile priority inheritance short-circuits
 walking of waiter chain when trylock fails)
Date: Fri, 18 Sep 2026 14:38:53 -0400

 On Fri, Sep 18, 2026 at 12:40:18PM +0000, riastradh@NetBSD.org wrote:
 >Synopsis: Turnstile priority inheritance short-circuits walking of waiter chain when trylock fails
 >
 >State-Changed-From-To: open->feedback
 >State-Changed-By: riastradh@NetBSD.org
 >State-Changed-When: Fri, 18 Sep 2026 12:40:17 +0000
 >State-Changed-Why:
 >Candidate fix commmitted -- I kept the optimization as long as we have
 >never failed a trylock and restarted, and added your comment.  How
 >does it look now?
 Just checked out the fix, looks good to me! Thank you for
 addressing this so promptly.
 >
 >(Also, out of curiosity: how did you find this?)
 I would love to be able to tell you I was running NetBSD on some huge
 machine and noticing latency spikes, but alas the answer is more boring.
 I noticed that I had made the same (incorrect) assumption in my own code
 and remembered that I had "inspired" myself from the NetBSD turnstile
 code, so I checked upstream whether or not that condition was there, and
 it was!
 >
 >
 >

From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/60739 CVS commit: src/sys/kern
Date: Wed, 23 Sep 2026 19:07:40 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Wed Sep 23 19:07:40 UTC 2026

 Modified Files:
 	src/sys/kern: kern_turnstile.c

 Log Message:
 Revert "turnstile(9): Fix priority inheritance if trylock requires restart."

 This was originally done in an attempt to resolve the issue described
 in:

 PR kern/60739: Turnstile priority inheritance short-circuits walking
 of waiter chain when trylock fails

 But it may have broken other things:

 PR kern/60776: reproducable assertion failure with today's current

 > Today's amd64 kernel reproducably crashes for me when running build.sh.
 >
 > [ 4043.4787057] panic: kernel diagnostic assertion "lwp_eprio(l) >= ts->ts_eprio" failed: file "../../../../kern/kern_turnstile.c", line 356
 > [ 4043.6286910] cpu1: Begin traceback...
 > [ 4043.6686907] vpanic() at netbsd:vpanic+0x189
 > [ 4043.7186903] kern_assert() at netbsd:kern_assert+0x4b
 > [ 4043.7786898] turnstile_wakeup() at netbsd:turnstile_wakeup+0x2c5
 > [ 4043.8586891] ubc_alloc.constprop.0() at netbsd:ubc_alloc.constprop.0+0x680
 > [ 4043.9386885] ubc_uiomove() at netbsd:ubc_uiomove+0x108
 > [ 4043.9986879] ffs_write() at netbsd:ffs_write+0x297
 > [ 4044.0586874] VOP_WRITE() at netbsd:VOP_WRITE+0x78
 > [ 4044.1086870] vn_write() at netbsd:vn_write+0xce
 > [ 4044.1686865] dofilewrite() at netbsd:dofilewrite+0x80
 > [ 4044.2286860] sys_write() at netbsd:sys_write+0x49
 > [ 4044.2786855] syscall() at netbsd:syscall+0x9d

 So let's back it out for now pending further analysis.


 To generate a diff of this commit:
 cvs rdiff -u -r1.57 -r1.58 src/sys/kern/kern_turnstile.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: feedback->open
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Wed, 23 Sep 2026 21:19:38 +0000
State-Changed-Why:
feedback provided, fix committed, fix reverted because it likely broke
things (PR kern/60776, reproducable assertion failure with today's
current, https://gnats.netbsd.org/60776), maybe we should find a way to
write atf tests for this before proceeding


From: =?UTF-8?Q?Lo=C3=AFc_Gregoire?= <loicgre@gmail.com>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/60739 (Turnstile priority inheritance short-circuits walking
 of waiter chain when trylock fails)
Date: Thu, 1 Oct 2026 09:34:18 -0400

 Hi, I wasn't sure if the message got through to you so I thought I
 would reply here.
 I have submitted a possible fix to the problem in the kern/60776 thread.
 Thank you.

 Le mer. 23 sept. 2026, =C3=A0 17 h 19, <riastradh@netbsd.org> a =C3=A9crit =
 :
 >
 > Synopsis: Turnstile priority inheritance short-circuits walking of waiter=
  chain when trylock fails
 >
 > State-Changed-From-To: feedback->open
 > State-Changed-By: riastradh@NetBSD.org
 > State-Changed-When: Wed, 23 Sep 2026 21:19:38 +0000
 > State-Changed-Why:
 > feedback provided, fix committed, fix reverted because it likely broke
 > things (PR kern/60776, reproducable assertion failure with today's
 > current, https://gnats.netbsd.org/60776), maybe we should find a way to
 > write atf tests for this before proceeding
 >
 >
 >

>Unformatted:

Home	PR Database Search