git: a6728314ff6d - main - ufs: do not busy or reference the mount in ufs_sync_nlink()

The branch main has been updated by sobomax:

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

commit a6728314ff6d6c73cab55c5bbd0025ed4ab56d53
Author:     Maxim Sobolev <sobomax@FreeBSD.org>
AuthorDate: 2026-10-13 03:57:26 +0000
Commit:     Maxim Sobolev <sobomax@FreeBSD.org>
CommitDate: 2026-10-13 04:00:26 +0000

    ufs: do not busy or reference the mount in ufs_sync_nlink()
    
    With soft updates, mkdir(2), link(2) and rename(2) into a directory
    whose i_nlink is at UFS_LINK_MAX while i_effnlink is below it call
    ufs_sync_nlink1() to sync the file system and restart.  It busied the
    mount with vfs_busy(mp, 0), which sleeps for as long as an unmount is in
    progress.  The caller is still between vn_start_write() and
    vn_finished_write() there, and the unmount, once it has set
    MNTK_UNMOUNT, goes on to suspend writes in VFS_UNMOUNT() and waits in
    vfs_write_suspend() for that write to end.  Neither could make progress,
    and every later vfs_busy() of the mount, such as from getfsstat(2) or a
    lookup crossing the mount point, hung behind them.
    
    The window is wide: after setting MNTK_UNMOUNT, the unmount waits for
    the busy references to drain, which includes another thread already
    syncing in ufs_sync_nlink1(), and for vfs_periodic().  A forced unmount
    of a file system with a directory at the link limit under mkdir/rmdir
    load hits it within seconds.
    
    This is the deadlock that 29d03af1e4da fixed for VOP_RENAME(), and the
    same reasoning applies: the caller's vn_start_write() already keeps the
    unmount from getting past the write suspension, before which
    ffs_unmount() tears nothing down, so the mount point does not need to
    be busied for VFS_SYNC().  The extra vfs_ref() taken by the callers is
    not needed either: a successful vn_start_write() keeps the reference
    that VOP_GETWRITEMOUNT() took until vn_finished_write() drops it.  With
    both gone, ufs_sync_nlink1() was only VFS_SYNC() and ERELOOKUP, so
    remove it and call VFS_SYNC() directly from ufs_sync_nlink() and the
    two places in ufs_rename().
    
    Fixes:  8db679af66b0 ("UFS: make mkdir() and link() reliable when using SU and reaching nlink limit")
    Fixes:  bc6d0d72f4f4 ("UFS rename: make it reliable when using SU and reaching nlink limit")
    
    Sponsored by:   Sippy Software, Inc.
    Reviewed by:    kib
    Tested by:      pho
    Differential Revision:  https://reviews.freebsd.org/D60204
    MFC after:      1 month
---
 sys/ufs/ufs/ufs_vnops.c | 42 +++++++++++++++++++-----------------------
 1 file changed, 19 insertions(+), 23 deletions(-)

diff --git a/sys/ufs/ufs/ufs_vnops.c b/sys/ufs/ufs/ufs_vnops.c
index aecb784ec46a..e32257529403 100644
--- a/sys/ufs/ufs/ufs_vnops.c
+++ b/sys/ufs/ufs/ufs_vnops.c
@@ -202,27 +202,22 @@ ufs_itimes(struct vnode *vp, struct timespec *tsa)
 	VI_UNLOCK(vp);
 }
 
-static int
-ufs_sync_nlink1(struct mount *mp)
-{
-	int error;
-
-	error = vfs_busy(mp, 0);
-	if (error == 0) {
-		VFS_SYNC(mp, MNT_WAIT);
-		vfs_unbusy(mp);
-		error = ERELOOKUP;
-	}
-	vfs_rel(mp);
-	return (error);
-}
-
+/*
+ * With soft updates, i_nlink can stay at UFS_LINK_MAX after links were
+ * removed, until the removals are on disk.  Sync the file system and have
+ * the caller restart the operation with ERELOOKUP.
+ *
+ * The caller is between vn_start_write() and vn_finished_write().  That
+ * holds a reference on the mount, and keeps an unmount from getting past
+ * the write suspension in VFS_UNMOUNT(), so the mount does not need to be
+ * referenced or busied here while the vnodes are unlocked.  vfs_busy()
+ * would even deadlock, as it waits for an unmount in progress to finish.
+ */
 static int
 ufs_sync_nlink(struct vnode *vp, struct vnode *vp1)
 {
 	struct inode *ip;
 	struct mount *mp;
-	int error;
 
 	ip = VTOI(vp);
 	if (ip->i_nlink < UFS_LINK_MAX)
@@ -231,13 +226,12 @@ ufs_sync_nlink(struct vnode *vp, struct vnode *vp1)
 		return (EMLINK);
 
 	mp = vp->v_mount;
-	vfs_ref(mp);
 	VOP_UNLOCK(vp);
 	if (vp1 != NULL)
 		VOP_UNLOCK(vp1);
-	error = ufs_sync_nlink1(mp);
+	VFS_SYNC(mp, MNT_WAIT);
 	vn_lock_pair(vp, false, LK_EXCLUSIVE, vp1, false, LK_EXCLUSIVE);
-	return (error);
+	return (ERELOOKUP);
 }
 
 /*
@@ -1452,7 +1446,6 @@ relock:
 			error = EMLINK;
 			goto unlockout;
 		}
-		vfs_ref(mp);
 		MPASS(!want_seqc_end);
 		VOP_UNLOCK(fdvp);
 		VOP_UNLOCK(fvp);
@@ -1460,7 +1453,9 @@ relock:
 		if (tvp != NULL)
 			vref(tvp);
 		VOP_VPUT_PAIR(tdvp, &tvp, true);
-		error = ufs_sync_nlink1(mp);
+		/* See ufs_sync_nlink(). */
+		VFS_SYNC(mp, MNT_WAIT);
+		error = ERELOOKUP;
 		vrele(fdvp);
 		vrele(fvp);
 		vrele(tdvp);
@@ -1592,14 +1587,15 @@ relock:
 				vn_seqc_write_end(fvp);
 				vn_seqc_write_end(fdvp);
 				want_seqc_end = false;
-				vfs_ref(mp);
 				VOP_UNLOCK(fdvp);
 				VOP_UNLOCK(fvp);
 				vref(tdvp);
 				if (tvp != NULL)
 					vref(tvp);
 				VOP_VPUT_PAIR(tdvp, &tvp, true);
-				error = ufs_sync_nlink1(mp);
+				/* See ufs_sync_nlink(). */
+				VFS_SYNC(mp, MNT_WAIT);
+				error = ERELOOKUP;
 				vrele(fdvp);
 				vrele(fvp);
 				vrele(tdvp);