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

["J. Hannken-Illjes via gnats" <gnats-admin%NetBSD.org@localhost>]

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

From: "J. Hannken-Illjes" <hannken%netbsd.org@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: 
Subject: Re: kern/60531: looping "vflushbuf: dirty" message
Date: Fri, 7 Aug 2026 16:39:33 +0000

 --ItY42kjenHJt4ZkN
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: inline
 
 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.
 
 The first part should be sufficient to prevent looping here, an updfated
 patch is attached.
 
 -- 
 J. Hannken-Illjes - hannken%netbsd.org@localhost
 
 --ItY42kjenHJt4ZkN
 Content-Type: text/x-diff; charset=us-ascii
 Content-Disposition: attachment; filename=pr60531.patch
 
 diff -r 983d69604667 sys/kern/vfs_subr.c
 --- sys/kern/vfs_subr.c	Fri Aug 07 13:18:45 2026 +0200
 +++ sys/kern/vfs_subr.c	Fri Aug 07 18:28:00 2026 +0200
 @@ -171,6 +171,8 @@ const int	vttoif_tab[9] = {
  	(bp)->b_vnbufs.le_next = NOLIST;				\
  }
  
 +static uint64_t bufcache_flushgen = 1;
 +
  int doforce = 1;		/* 1 => permit forcible unmounting */
  
  /*
 @@ -329,19 +331,19 @@ restart:
  
  /*
   * Flush all dirty buffers from a vnode.
 - * Called with the underlying vnode locked, which should prevent new dirty
 - * buffers from being queued.
 + * Called with the underlying vnode locked.
   */
  int
  vflushbuf(struct vnode *vp, int flags)
  {
  	struct buf *bp, *nbp;
  	int error, pflags;
 -	bool dirty, sync;
 -#ifdef DEBUG
 -	bool warned = false;
 -#endif
 +	bool sync;
 +	uint64_t flushgen;
  
 +	KASSERT(VOP_ISLOCKED(vp));
 +
 +	flushgen = 0;
  	sync = (flags & FSYNC_WAIT) != 0;
  	pflags = PGO_CLEANIT | PGO_ALLPAGES |
  	    (sync ? PGO_SYNCIO : 0) |
 @@ -349,12 +351,20 @@ vflushbuf(struct vnode *vp, int flags)
  	rw_enter(vp->v_uobj.vmobjlock, RW_WRITER);
  	(void) VOP_PUTPAGES(vp, 0, 0, pflags);
  
 +	/*
 +	 * Ensure every dirty block associated with this vnode has
 +	 * begun to be written out (BC_BUSY, I/O in progress) -- any
 +	 * delayed writes must delay no longer.
 +	 */
  loop:
  	mutex_enter(&bufcache_lock);
 +	if (flushgen == 0)
 +		flushgen = bufcache_flushgen++;
  	for (bp = LIST_FIRST(&vp->v_dirtyblkhd); bp; bp = nbp) {
  		KASSERT(bp->b_vp == vp);
 +		KASSERT((bp->b_flags & B_PHYS) == 0);
  		nbp = LIST_NEXT(bp, b_vnbufs);
 -		if ((bp->b_cflags & BC_BUSY))
 +		if ((bp->b_cflags & BC_BUSY) != 0 || bp->b_flushgen > flushgen)
  			continue;
  		if ((bp->b_oflags & BO_DELWRI) == 0)
  			panic("vflushbuf: not dirty, bp %p", bp);
 @@ -378,28 +388,17 @@ loop:
  	if (!sync)
  		return 0;
  
 +	/*
 +	 * 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);
 -	dirty = !LIST_EMPTY(&vp->v_dirtyblkhd);
 +	KASSERT((vp->v_type == VBLK && spec_node_getmountedfs(vp)) ||
 +	    LIST_EMPTY(&vp->v_dirtyblkhd));
  	mutex_exit(vp->v_interlock);
  
 -	if (dirty) {
 -#ifdef DEBUG
 -		if (!warned) {
 -			static struct timeval vflushbuf_warntime;
 -			const struct timeval interval = {60,0};
 -
 -			mutex_enter(&bufcache_lock);
 -			if (ratecheck(&vflushbuf_warntime, &interval))
 -				vprint("vflushbuf: dirty", vp);
 -			mutex_exit(&bufcache_lock);
 -			warned = true;
 -		}
 -#endif
 -		goto loop;
 -	}
 -
  	return 0;
  }
  
 @@ -511,6 +510,7 @@ reassignbuf(struct buf *bp, struct vnode
  	KASSERT(bp->b_objlock == vp->v_interlock);
  	KASSERT(mutex_owned(vp->v_interlock));
  	KASSERT((bp->b_cflags & BC_BUSY) != 0);
 +	KASSERT((bp->b_flags & B_PHYS) == 0);
  
  	/*
  	 * Delete from old vnode list, if on one.
 @@ -549,6 +549,7 @@ reassignbuf(struct buf *bp, struct vnode
  			    (vp->v_mount->mnt_flag & MNT_ASYNC) == 0)
  				vn_syncer_add_to_worklist(vp, delayx);
  		}
 +		bp->b_flushgen = bufcache_flushgen;
  	}
  	bufinsvn(bp, listheadp);
  }
 diff -r 983d69604667 sys/sys/buf.h
 --- sys/sys/buf.h	Fri Aug 07 13:18:45 2026 +0200
 +++ sys/sys/buf.h	Fri Aug 07 18:28:00 2026 +0200
 @@ -135,8 +135,15 @@ struct buf {
  						      (partition relative) */
  	daddr_t			b_rawblkno;	/* b: raw physical block number
  						      (volume relative) */
 -	struct proc		*b_proc;	/* b: proc if BB_PHYS */
 -	void			*b_saveaddr;	/* b: saved b_data for physio */
 +	union {
 +		/* B_PHYS */
 +		struct {
 +			struct proc *b_proc;	/* b: proc if B_PHYS */
 +			void	*b_saveaddr;	/* b: saved b_data for physio */
 +		};
 +		/* !B_PHYS */
 +		uint64_t	b_flushgen;	/* b: flush generation */
 +	};
  	struct cpu_info		*b_ci;		/* b: originating CPU */
  
  	/*
 
 --ItY42kjenHJt4ZkN--