cvs commit: src/sys/sys buf.h src/sys/kern vfs_bio.c vfs_cluster.c src/sys/ufs/ffs ffs_softdep.c

[Pawel Jakub Dawidek]

On Wed, Aug 27, 2003 at 11:55:19PM -0700, Jeff Roberson wrote:
+>   Commiter:	Jeff Roberson <jeff at FreeBSD.org>
+>   Branch:	HEAD
+> 
+>   Files:
+> 	1.398   src/sys/kern/vfs_bio.c         
+> 	1.143   src/sys/kern/vfs_cluster.c     
+> 	1.155   src/sys/sys/buf.h              
+> 	1.140   src/sys/ufs/ffs/ffs_softdep.c  
+> 
+>   Log:
+>    - Move BX_BKGRDWAIT and BX_BKGRDINPROG to BV_ and the b_vflags field.
+>    - Surround all accesses of the BKGRD{WAIT,INPROG} flags with the vnode
+>      interlock.
+>    - Don't use the B_LOCKED flag and QUEUE_LOCKED for background write
+>      buffers.  Check for the BKGRDINPROG flag before recycling or throwing
+>      away a buffer.  We do this instead because it is not safe for us to move
+>      the original buffer to a new queue from the callback on the background
+>      write buffer.
+>    - Remove the B_LOCKED flag and the locked buffer queue.  They are no longer
+>      used.
+>    - The vnode interlock is used around checks for BKGRDINPROG where it may
+>      not be strictly necessary.  If we hold the buf lock the a back-ground
+>      write will not be started without our knowledge, one may only be
+>      completed while we're not looking.  Rather than remove the code, Document
+>      two of the places where this extra locking is done.  A pass should be
+>      done to verify and minimize the locking later.
[...]
+> ===================================================================
+> RCS file: /usr/local/www/cvsroot/FreeBSD/src/sys/ufs/ffs/ffs_softdep.c,v
+> retrieving revision 1.139
+> retrieving revision 1.140
+> diff -u -p -r1.139 -r1.140
+> --- src/sys/ufs/ffs/ffs_softdep.c	2003/03/18 08:45:24	1.139
+> +++ src/sys/ufs/ffs/ffs_softdep.c	2003/08/28 06:55:18	1.140
[...]
+> @@ -5803,21 +5801,31 @@ getdirtybuf(bpp, waitfor)
+>  	struct buf *bp;
+>  	int error;
+>  
+> +	/*
+> +	 * XXX This code and the code that calls it need to be reviewed to
+> +	 * verify its use of the vnode interlock.
+> +	 */
+> +
+>  	for (;;) {
+>  		if ((bp = *bpp) == NULL)
+>  			return (0);
+> -		/* XXX Probably needs interlock */
+> +		VI_LOCK(bp->b_vp);
+>  		if (BUF_LOCK(bp, LK_EXCLUSIVE | LK_NOWAIT, NULL) == 0) {
+> -			if ((bp->b_xflags & BX_BKGRDINPROG) == 0)
+> +			if ((bp->b_vflags & BV_BKGRDINPROG) == 0) {
+> +				VI_UNLOCK(bp->b_vp);
+>  				break;
+> +			}
+>  			BUF_UNLOCK(bp);
+> -			if (waitfor != MNT_WAIT)
+> +			if (waitfor != MNT_WAIT) {
+> +				VI_UNLOCK(bp->b_vp);
+>  				return (0);
+> -			bp->b_xflags |= BX_BKGRDWAIT;
+> -			interlocked_sleep(&lk, SLEEP, &bp->b_xflags, NULL,
+> -			    PRIBIO, "getbuf", 0);
+> +			}
+> +			bp->b_vflags |= BV_BKGRDWAIT;
+> +			interlocked_sleep(&lk, SLEEP, &bp->b_xflags,
+> +			    VI_MTX(bp->b_vp), PRIBIO|PDROP, "getbuf", 0);
+>  			continue;
+>  		}
+> +		VI_UNLOCK(bp->b_vp);
+>  		if (waitfor != MNT_WAIT)
+>  			return (0);
+>  		error = interlocked_sleep(&lk, LOCKBUF, bp, NULL, 

I'm afraid that something is wrong with this change.
I get panics:
_mtx_lock_flags()
getdirtybuf()
[...]

I'll provide more info when I'll reproduce it.

-- 
Pawel Jakub Dawidek                       pawel at dawidek.net
UNIX Systems Programmer/Administrator     http://garage.freebsd.pl
Am I Evil? Yes, I Am!                     http://cerber.sourceforge.net
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 305 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/cvs-src/attachments/20030902/31eccf15/attachment.bin