Skip site navigation (1)Skip section navigation (2) 
Date:      Fri, 03 Jul 2026 18:58:58 +0000
From:      Robert Wing <rew@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 2de20c5c77cf - main - autofs: enable witness for autofs node lock
Message-ID:  <6a480672.33785.206f5e6c@gitrepo.freebsd.org>
index | next in thread | raw e-mail 

The branch main has been updated by rew:

URL: https://cgit.FreeBSD.org/src/commit/?id=2de20c5c77cf8d5b2059054cff0e0c1fc124739d

commit 2de20c5c77cf8d5b2059054cff0e0c1fc124739d
Author:     Robert Wing <rew@FreeBSD.org>
AuthorDate: 2026-07-03 18:55:37 +0000
Commit:     Robert Wing <rew@FreeBSD.org>
CommitDate: 2026-07-03 18:55:37 +0000

    autofs: enable witness for autofs node lock
    
    Previously, an_vnode_lock was initialized with SX_NOWITNESS to silence
    lock order reversals. The reversals would occur when autofs_node_vn()
    was called with the directory vnode lock held, then lock an_vnode_lock,
    then lock the vnode attached to the autofs node. It looked like:
    
        directory vnode -> an_vnode_lock -> vnode attached to autofs node
    
    The established lock order is now vnode -> an_vnode_lock
    
    Currently, we don't have to worry about losing an autofs node during the
    unlock/lock as autofs nodes are only removed during an unmount() after
    vflush(). When autofs_node_vn() is called, the mountpoint has either
    been busied (preventing unmount) or a directory vnode is locked which
    prevents vflush() from finishing until the directory vnode is unlocked.
    
    Reviewed by:    kib
    Differential Revision:  https://reviews.freebsd.org/D57857
---
 sys/fs/autofs/autofs_vnops.c | 44 ++++++++++++++++++--------------------------
 1 file changed, 18 insertions(+), 26 deletions(-)

diff --git a/sys/fs/autofs/autofs_vnops.c b/sys/fs/autofs/autofs_vnops.c
index e8e47f20166a..b6424fc2ff4c 100644
--- a/sys/fs/autofs/autofs_vnops.c
+++ b/sys/fs/autofs/autofs_vnops.c
@@ -572,16 +572,7 @@ autofs_node_new(struct autofs_node *parent, struct autofs_mount *amp,
 		anp->an_name = strdup(name, M_AUTOFS);
 	anp->an_fileno = atomic_fetchadd_int(&amp->am_last_fileno, 1);
 	callout_init(&anp->an_callout, 1);
-	/*
-	 * The reason for SX_NOWITNESS here is that witness(4)
-	 * cannot tell vnodes apart, so the following perfectly
-	 * valid lock order...
-	 *
-	 * vnode lock A -> autofsvlk B -> vnode lock B
-	 *
-	 * ... gets reported as a LOR.
-	 */
-	sx_init_flags(&anp->an_vnode_lock, "autofsvlk", SX_NOWITNESS);
+	sx_init(&anp->an_vnode_lock, "autofsvlk");
 	getnanotime(&anp->an_ctime);
 	anp->an_parent = parent;
 	anp->an_mount = amp;
@@ -645,6 +636,7 @@ autofs_node_vn(struct autofs_node *anp, struct mount *mp, int flags,
 {
 	struct vnode *vp;
 	int error;
+	enum vgetstate vs;
 
 	AUTOFS_ASSERT_UNLOCKED(anp->an_mount);
 
@@ -652,53 +644,53 @@ autofs_node_vn(struct autofs_node *anp, struct mount *mp, int flags,
 
 	vp = anp->an_vnode;
 	if (vp != NULL) {
-		error = vget(vp, flags | LK_RETRY);
-		if (error != 0) {
-			AUTOFS_WARN("vget failed with error %d", error);
-			sx_xunlock(&anp->an_vnode_lock);
-			return (error);
-		}
+lockvp:
+		vs = vget_prep(vp);
+		sx_xunlock(&anp->an_vnode_lock);
+		vget_finish(vp, flags | LK_RETRY, vs);
 		if (VN_IS_DOOMED(vp)) {
 			/*
 			 * We got forcibly unmounted.
 			 */
 			AUTOFS_DEBUG("doomed vnode");
-			sx_xunlock(&anp->an_vnode_lock);
 			vput(vp);
 
 			return (ENOENT);
 		}
 
 		*vpp = vp;
-		sx_xunlock(&anp->an_vnode_lock);
 		return (0);
 	}
+	sx_xunlock(&anp->an_vnode_lock);
 
 	error = getnewvnode("autofs", mp, &autofs_vnodeops, &vp);
-	if (error != 0) {
-		sx_xunlock(&anp->an_vnode_lock);
+	if (error != 0)
 		return (error);
-	}
 
 	vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
 
 	vp->v_type = VDIR;
 	if (anp->an_parent == NULL)
 		vp->v_vflag |= VV_ROOT;
-	vp->v_data = anp;
-
 	VN_LOCK_ASHARE(vp);
 
 	error = insmntque(vp, mp);
 	if (error != 0) {
 		AUTOFS_DEBUG("insmntque() failed with error %d", error);
-		sx_xunlock(&anp->an_vnode_lock);
 		return (error);
 	}
 
-	KASSERT(anp->an_vnode == NULL, ("lost race"));
+	sx_xlock(&anp->an_vnode_lock);
+	if (anp->an_vnode != NULL) {
+		vp->v_op = &dead_vnodeops;
+		vp->v_data = NULL;
+		vgone(vp);
+		vput(vp);
+		vp = anp->an_vnode;
+		goto lockvp;
+	}
+	vp->v_data = anp;
 	anp->an_vnode = vp;
-
 	sx_xunlock(&anp->an_vnode_lock);
 
 	vn_set_state(vp, VSTATE_CONSTRUCTED);
home | help

Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6a480672.33785.206f5e6c>