From owner-cvs-src@FreeBSD.ORG Tue Sep 2 12:35:36 2003 Return-Path: Delivered-To: cvs-src@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id F1D7916A4BF; Tue, 2 Sep 2003 12:35:35 -0700 (PDT) Received: from milla.ask33.net (milla.ask33.net [217.197.166.60]) by mx1.FreeBSD.org (Postfix) with ESMTP id 82F9F43FEC; Tue, 2 Sep 2003 12:35:34 -0700 (PDT) (envelope-from nick@milla.ask33.net) Received: by milla.ask33.net (Postfix, from userid 1001) id DA3F53ABB2D; Tue, 2 Sep 2003 21:37:31 +0200 (CEST) Date: Tue, 2 Sep 2003 21:37:31 +0200 From: Pawel Jakub Dawidek To: Jeff Roberson Message-ID: <20030902193731.GV47959@garage.freebsd.pl> References: <200308280655.h7S6tJTS064892@repoman.freebsd.org> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-md5; protocol="application/pgp-signature"; boundary="nNlRuuqCdLba03wQ" Content-Disposition: inline In-Reply-To: <200308280655.h7S6tJTS064892@repoman.freebsd.org> X-PGP-Key-URL: http://garage.freebsd.pl/jules.asc X-OS: FreeBSD 4.8-RELEASE-p3 i386 X-URL: http://garage.freebsd.pl User-Agent: Mutt/1.5.1i cc: cvs-src@FreeBSD.org cc: src-committers@FreeBSD.org cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/sys buf.h src/sys/kern vfs_bio.c vfs_cluster.c src/sys/ufs/ffs ffs_softdep.c X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 02 Sep 2003 19:35:36 -0000 --nNlRuuqCdLba03wQ Content-Type: text/plain; charset=iso-8859-2 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Aug 27, 2003 at 11:55:19PM -0700, Jeff Roberson wrote: +> Commiter: Jeff Roberson +> Branch: HEAD +>=20 +> Files: +> 1.398 src/sys/kern/vfs_bio.c =20 +> 1.143 src/sys/kern/vfs_cluster.c =20 +> 1.155 src/sys/sys/buf.h =20 +> 1.140 src/sys/ufs/ffs/ffs_softdep.c =20 +>=20 +> 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 throwi= ng +> 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 backgro= und +> 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-grou= nd +> write will not be started without our knowledge, one may only be +> completed while we're not looking. Rather than remove the code, Do= cument +> two of the places where this extra locking is done. A pass should = be +> done to verify and minimize the locking later. [...] +> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D +> 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; +> =20 +> + /* +> + * XXX This code and the code that calls it need to be reviewed to +> + * verify its use of the vnode interlock. +> + */ +> + +> for (;;) { +> if ((bp =3D *bpp) =3D=3D NULL) +> return (0); +> - /* XXX Probably needs interlock */ +> + VI_LOCK(bp->b_vp); +> if (BUF_LOCK(bp, LK_EXCLUSIVE | LK_NOWAIT, NULL) =3D=3D 0) { +> - if ((bp->b_xflags & BX_BKGRDINPROG) =3D=3D 0) +> + if ((bp->b_vflags & BV_BKGRDINPROG) =3D=3D 0) { +> + VI_UNLOCK(bp->b_vp); +> break; +> + } +> BUF_UNLOCK(bp); +> - if (waitfor !=3D MNT_WAIT) +> + if (waitfor !=3D MNT_WAIT) { +> + VI_UNLOCK(bp->b_vp); +> return (0); +> - bp->b_xflags |=3D BX_BKGRDWAIT; +> - interlocked_sleep(&lk, SLEEP, &bp->b_xflags, NULL, +> - PRIBIO, "getbuf", 0); +> + } +> + bp->b_vflags |=3D 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 !=3D MNT_WAIT) +> return (0); +> error =3D interlocked_sleep(&lk, LOCKBUF, bp, NULL,=20 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. --=20 Pawel Jakub Dawidek pawel@dawidek.net UNIX Systems Programmer/Administrator http://garage.freebsd.pl Am I Evil? Yes, I Am! http://cerber.sourceforge.net --nNlRuuqCdLba03wQ Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.0.7 (FreeBSD) iQCVAwUBP1Txez/PhmMH/Mf1AQGMrAP9GBEjAMhSK5uM/ZztxHpF4tCe7LVmyzHA mudrt6WJxCr/th8+3N1mOmKHlciT/SZn19rHP4ksdcJPMzRIU9Bl5iDSFNdtDpWR PsW2QaQ3RE3MZvsfwwCqRvytZA2TJcFShUy8d8yHFlI5wopHO8HaR3mA1k3twUvv rUjqfBcYCCI= =Ar1p -----END PGP SIGNATURE----- --nNlRuuqCdLba03wQ--