Re: kern/60531: looping "vflushbuf: dirty" message

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

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

From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
To: "J. Hannken-Illjes" <hannken%NetBSD.org@localhost>
Cc: gnats-bugs%NetBSD.org@localhost, kern-bug-people%NetBSD.org@localhost, gnats-admin%NetBSD.org@localhost,
	netbsd-bugs%NetBSD.org@localhost, dholland%NetBSD.org@localhost
Subject: Re: kern/60531: looping "vflushbuf: dirty" message
Date: Fri, 7 Aug 2026 17:29:13 +0000

 > Date: Fri, 7 Aug 2026 16:39:33 +0000
 > From: "J. Hannken-Illjes" <hannken%netbsd.org@localhost>
 >  
 > The FIX will not work, it is not ok to replace 'vp->v_numoutput == 0' with
 > 'LIST_EMPTY(&vp->v_dirtyblkhd)'.
 > 
 > Buffers are removed from v_dirtyblkhd through reassignbuf() when bwrite()
 > initiates the write while v_numoutput gets decremented through
 > biodone()->vwakekup() when the I/O has completed.
 >  
 > With your patch VOP_FSYNC(vp, FSYNC_WAIT) would return before all buffers
 > are written to disk.
 
 Thanks, that would be bad!
 
 > The first part should be sufficient to prevent looping here, an updfated
 > patch is attached.
 > [...]
 >  +	/*
 >  +	 * Wait until all pending writes issued before we incremented
 >  +	 * bufcache_flushgen have completed.
 >  +	 */
 >   	mutex_enter(vp->v_interlock);
 >   	while (vp->v_numoutput != 0)
 >   		cv_wait(&vp->v_cv, vp->v_interlock);
 
 The trouble with waiting for vp->v_numoutput is that if there's
 concurrent I/O on a block device or a snapshot, it can keep adding
 more writes to v_numoutput so that vflushbuf(9) never completes.
 
 So I don't think this change accomplishes much: we add some more
 bookkeeping around flushgen, but we may continue waiting even if we've
 long since passed that flushgen.
 
 In fact this change may even create a deadlock, because:
 
 (a) vflushbuf holds the vnode lock while it waits, and
 (b) it does not issue bwrite for any buffers past flushgen,
 
 so nothing else can flush those buffers concurrently either with
 vflushbuf.
 
 So perhaps we need to wait until:
 
 (a) v_dirtyblkhd has no entries before flushgen,
 AND
 (b) v_cleanblkhd has no entries before flushgen that are still being
     written out.
 
 Not sure exactly how to detect (b).  Is a buffercache(9) buf bp still
 being written out iff
 
 (bp->b_flags & B_READ) == 0 &&
 (bp->b_oflags & BO_DONE) == 0?
 
 I'm not really happy about the prospect of iterating over v_cleanblkhd
 but if the buffers in question -- those currently being written out --
 aren't on v_dirtyblkhd, I don't see another way to tell when they've
 all passed the flushgen short of either iterating over v_cleanblkhd,
 or adding a new queue like v_writingblkhd and dealing with all the
 consequences of that.
 
 >  +	KASSERT((vp->v_type == VBLK && spec_node_getmountedfs(vp)) ||
 >  +	    LIST_EMPTY(&vp->v_dirtyblkhd));
 
 Won't this assertion also fire if vp is a VREG snapshot, since the
 snapshot write path also uses reassignbuf without holding the vnode
 lock, as I noted earlier?
 
 https://mail-index.NetBSD.org/netbsd-bugs/2026/08/02/msg094240.html