Re: kern/60795: LFS userland cleaner and umount can enter deadlock

["Taylor R Campbell via gnats" <gnats-admin%NetBSD.org@localhost>]

The following reply was made to PR kern/60795; it has been noted by GNATS.

From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
To: Shinichi Doyashiki <clare%csel.org@localhost>
Cc: gnats-bugs%NetBSD.org@localhost, netbsd-bugs%NetBSD.org@localhost,
	"J. Hannken-Illjes" <hannken%mailbox.org@localhost>,
	Konrad Schroder <perseant%hhhh.org@localhost>
Subject: Re: kern/60795: LFS userland cleaner and umount can enter deadlock
Date: Sat, 26 Sep 2026 12:46:03 +0000

 > Date: Sat, 26 Sep 2026 06:18:19 +0000 (UTC)
 > From: clare%csel.org@localhost
 > 
 > the umount process
 > 
 > db{0}> bt /a ffff9bff0365e000
 > trace: pid 21479 lid 21479 at 0xffffcc094754cc00
 > sleepq_block() at netbsd:sleepq_block+0xea
 > turnstile_block() at netbsd:turnstile_block+0x24b
 > rw_vector_enter() at netbsd:rw_vector_enter+0x161
 > lfs_prelock() at netbsd:lfs_prelock+0x86
 > lfs_seglock() at netbsd:lfs_seglock+0x16
 > [...]
 > 
 > the lfs_cleanerd (userland) process
 > 
 > db{0}> bt /a ffff9bff0365e400
 > trace: pid 22182 lid 22182 at 0xffffcc09486d4950
 > sleepq_block() at netbsd:sleepq_block+0xea
 > cv_wait() at netbsd:cv_wait+0xca
 > fstrans_start() at netbsd:fstrans_start+0x14c
 > [...]
 > bread() at netbsd:bread+0x18
 > [...]
 > lfs_rewrite_segments() at netbsd:lfs_rewrite_segments+0x3f6
 > lfs_fcntl() at netbsd:lfs_fcntl+0x15b3
 
 - unmount suspends fs, waits for seglock.
 - lfs_rewrite_segments takes seglock, tries block I/O which waits for
   concurrent suspension to resume.
 *deadlock*
 
 Perhaps lfs_fcntl, or lfs_rewrite_segments, should start a lazy
 fstrans on the file system before trying to do any work on it, so it
 isn't blocked by a concurrent suspension for unmount?
 
 	fstrans_start_lazy(ap->a_vp->v_mount);
 	...
 	fstrans_done(ap->a_vp->v_mount);
 
 Maybe the fstrans should be started before testing IMNT_SHUTDOWN?
 
    1897 	/* Avoid locking a draining lock */
    1898 	if (ap->a_vp->v_mount->mnt_iflag & IMNT_UNMOUNT) {
    1899 		return ESHUTDOWN;
    1900 	}
 
 https://nxr.NetBSD.org/xref/src/sys/ufs/lfs/lfs_vnops.c#1897
 
 Side note: How does one ensure v_mount is stable at this point in the
 face of concurrent revoke?  Seems like we need to do something like
 
 	for (;;) {
 		mp = atomic_load_consume(&v->v_mount);
 		fstrans_start_lazy(mp);
 		if (__predict_true(mp == atomic_load_consume(&v->v_mount)))
 			break;
 		fstrans_done(mp);
 	}
 	vn_lock(vp, LK_SHARED|LK_WAIT);
 	/*
 	 * mp may be lfs or deadfs if the vnode was concurrently
 	 * revoked; check for concurrent revoke before proceeding.
 	 */
 	mutex_enter(vp->v_interlock);
 	error = vdead_check(vp, 0);
 	mutex_exit(vp->v_interlock);
 	if (error)
 		goto out;
 
 	... check IMNT_SHUTDOWN, do the rest of lfs_fcntl ...
 
 out:	VOP_UNLOCK(vp);
 	fstrans_done(mp);
 
 ...feeling some deja vu about this question, looks like I was
 wondering something similar a couple years ago too:
 
 https://mail-index.NetBSD.org/tech-kern/2024/10/01/msg029758.html
 
 Actually, maybe it doesn't need to be a lazy fstrans at all; maybe the
 ordinary fstrans in vn_lock is enough.  But then maybe we can't hold
 vn_lock across the whole fcntl operation for other reasons -- that
 leads to a lot of code paths that might try to take vnode locks.