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

[Taylor R Campbell <riastradh%NetBSD.org@localhost>]

> 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.