From owner-svn-src-all@FreeBSD.ORG Mon Sep 13 20:29:10 2010 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3F578106566C; Mon, 13 Sep 2010 20:29:10 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 2DC5E8FC1C; Mon, 13 Sep 2010 20:29:10 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id o8DKTAr4030351; Mon, 13 Sep 2010 20:29:10 GMT (envelope-from kib@svn.freebsd.org) Received: (from kib@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id o8DKTAUa030348; Mon, 13 Sep 2010 20:29:10 GMT (envelope-from kib@svn.freebsd.org) Message-Id: <201009132029.o8DKTAUa030348@svn.freebsd.org> From: Konstantin Belousov Date: Mon, 13 Sep 2010 20:29:10 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org X-SVN-Group: stable-8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r212583 - stable/8/sys/kern X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 13 Sep 2010 20:29:10 -0000 Author: kib Date: Mon Sep 13 20:29:09 2010 New Revision: 212583 URL: http://svn.freebsd.org/changeset/base/212583 Log: MFC r211213: The buffers b_vflags field is not always properly protected by bufobj lock. If b_bufobj is not NULL, then bufobj lock should be held when manipulating the flags. Not doing this sometimes leaves BV_BKGRDINPROG to be erronously set, causing softdep' getdirtybuf() to stuck indefinitely in "getbuf" sleep, waiting for background write to finish which is not actually performed. Add BO_LOCK() in the cases where it was missed. Modified: stable/8/sys/kern/vfs_bio.c stable/8/sys/kern/vfs_subr.c Directory Properties: stable/8/sys/ (props changed) stable/8/sys/amd64/include/xen/ (props changed) stable/8/sys/cddl/contrib/opensolaris/ (props changed) stable/8/sys/contrib/dev/acpica/ (props changed) stable/8/sys/contrib/pf/ (props changed) stable/8/sys/dev/xen/xenpci/ (props changed) Modified: stable/8/sys/kern/vfs_bio.c ============================================================================== --- stable/8/sys/kern/vfs_bio.c Mon Sep 13 20:11:18 2010 (r212582) +++ stable/8/sys/kern/vfs_bio.c Mon Sep 13 20:29:09 2010 (r212583) @@ -391,6 +391,8 @@ bufcountwakeup(struct buf *bp) KASSERT((bp->b_vflags & BV_INFREECNT) == 0, ("buf %p already counted as free", bp)); + if (bp->b_bufobj != NULL) + mtx_assert(BO_MTX(bp->b_bufobj), MA_OWNED); bp->b_vflags |= BV_INFREECNT; old = atomic_fetchadd_int(&numfreebuffers, 1); KASSERT(old >= 0 && old < nbuf, @@ -690,6 +692,8 @@ bremfree(struct buf *bp) if ((bp->b_flags & B_INVAL) || (bp->b_flags & B_DELWRI) == 0) { KASSERT((bp->b_vflags & BV_INFREECNT) != 0, ("buf %p not counted in numfreebuffers", bp)); + if (bp->b_bufobj != NULL) + mtx_assert(BO_MTX(bp->b_bufobj), MA_OWNED); bp->b_vflags &= ~BV_INFREECNT; old = atomic_fetchadd_int(&numfreebuffers, -1); KASSERT(old > 0, ("numfreebuffers dropped to %d", old - 1)); @@ -746,6 +750,8 @@ bremfreel(struct buf *bp) if ((bp->b_flags & B_INVAL) || (bp->b_flags & B_DELWRI) == 0) { KASSERT((bp->b_vflags & BV_INFREECNT) != 0, ("buf %p not counted in numfreebuffers", bp)); + if (bp->b_bufobj != NULL) + mtx_assert(BO_MTX(bp->b_bufobj), MA_OWNED); bp->b_vflags &= ~BV_INFREECNT; old = atomic_fetchadd_int(&numfreebuffers, -1); KASSERT(old > 0, ("numfreebuffers dropped to %d", old - 1)); @@ -1390,8 +1396,16 @@ brelse(struct buf *bp) /* enqueue */ mtx_lock(&bqlock); /* Handle delayed bremfree() processing. */ - if (bp->b_flags & B_REMFREE) + if (bp->b_flags & B_REMFREE) { + struct bufobj *bo; + + bo = bp->b_bufobj; + if (bo != NULL) + BO_LOCK(bo); bremfreel(bp); + if (bo != NULL) + BO_UNLOCK(bo); + } if (bp->b_qindex != QUEUE_NONE) panic("brelse: free buffer onto another queue???"); @@ -1452,8 +1466,16 @@ brelse(struct buf *bp) * if B_INVAL is set ). */ - if (!(bp->b_flags & B_DELWRI)) + if (!(bp->b_flags & B_DELWRI)) { + struct bufobj *bo; + + bo = bp->b_bufobj; + if (bo != NULL) + BO_LOCK(bo); bufcountwakeup(bp); + if (bo != NULL) + BO_UNLOCK(bo); + } /* * Something we can maybe free or reuse @@ -1482,6 +1504,8 @@ brelse(struct buf *bp) void bqrelse(struct buf *bp) { + struct bufobj *bo; + CTR3(KTR_BUF, "bqrelse(%p) vp %p flags %X", bp, bp->b_vp, bp->b_flags); KASSERT(!(bp->b_flags & (B_CLUSTER|B_PAGING)), ("bqrelse: inappropriate B_PAGING or B_CLUSTER bp %p", bp)); @@ -1492,10 +1516,15 @@ bqrelse(struct buf *bp) return; } + bo = bp->b_bufobj; if (bp->b_flags & B_MANAGED) { if (bp->b_flags & B_REMFREE) { mtx_lock(&bqlock); + if (bo != NULL) + BO_LOCK(bo); bremfreel(bp); + if (bo != NULL) + BO_UNLOCK(bo); mtx_unlock(&bqlock); } bp->b_flags &= ~(B_ASYNC | B_NOCACHE | B_AGE | B_RELBUF); @@ -1505,8 +1534,13 @@ bqrelse(struct buf *bp) mtx_lock(&bqlock); /* Handle delayed bremfree() processing. */ - if (bp->b_flags & B_REMFREE) + if (bp->b_flags & B_REMFREE) { + if (bo != NULL) + BO_LOCK(bo); bremfreel(bp); + if (bo != NULL) + BO_UNLOCK(bo); + } if (bp->b_qindex != QUEUE_NONE) panic("bqrelse: free buffer onto another queue???"); /* buffers with stale but valid contents */ @@ -1541,8 +1575,13 @@ bqrelse(struct buf *bp) } mtx_unlock(&bqlock); - if ((bp->b_flags & B_INVAL) || !(bp->b_flags & B_DELWRI)) + if ((bp->b_flags & B_INVAL) || !(bp->b_flags & B_DELWRI)) { + if (bo != NULL) + BO_LOCK(bo); bufcountwakeup(bp); + if (bo != NULL) + BO_UNLOCK(bo); + } /* * Something we can maybe free or reuse. @@ -1878,7 +1917,11 @@ restart: KASSERT((bp->b_flags & B_DELWRI) == 0, ("delwri buffer %p found in queue %d", bp, qindex)); + if (bp->b_bufobj != NULL) + BO_LOCK(bp->b_bufobj); bremfreel(bp); + if (bp->b_bufobj != NULL) + BO_UNLOCK(bp->b_bufobj); mtx_unlock(&bqlock); if (qindex == QUEUE_CLEAN) { @@ -2613,7 +2656,9 @@ loop: bp->b_flags &= ~B_CACHE; else if ((bp->b_flags & (B_VMIO | B_INVAL)) == 0) bp->b_flags |= B_CACHE; + BO_LOCK(bo); bremfree(bp); + BO_UNLOCK(bo); /* * check for size inconsistancies for non-VMIO case. Modified: stable/8/sys/kern/vfs_subr.c ============================================================================== --- stable/8/sys/kern/vfs_subr.c Mon Sep 13 20:11:18 2010 (r212582) +++ stable/8/sys/kern/vfs_subr.c Mon Sep 13 20:29:09 2010 (r212583) @@ -1249,13 +1249,17 @@ flushbuflist( struct bufv *bufv, int fla */ if (((bp->b_flags & (B_DELWRI | B_INVAL)) == B_DELWRI) && (flags & V_SAVE)) { + BO_LOCK(bo); bremfree(bp); + BO_UNLOCK(bo); bp->b_flags |= B_ASYNC; bwrite(bp); BO_LOCK(bo); return (EAGAIN); /* XXX: why not loop ? */ } + BO_LOCK(bo); bremfree(bp); + BO_UNLOCK(bo); bp->b_flags |= (B_INVAL | B_RELBUF); bp->b_flags &= ~B_ASYNC; brelse(bp); @@ -1307,7 +1311,9 @@ restart: BO_MTX(bo)) == ENOLCK) goto restart; + BO_LOCK(bo); bremfree(bp); + BO_UNLOCK(bo); bp->b_flags |= (B_INVAL | B_RELBUF); bp->b_flags &= ~B_ASYNC; brelse(bp); @@ -1329,7 +1335,9 @@ restart: LK_EXCLUSIVE | LK_SLEEPFAIL | LK_INTERLOCK, BO_MTX(bo)) == ENOLCK) goto restart; + BO_LOCK(bo); bremfree(bp); + BO_UNLOCK(bo); bp->b_flags |= (B_INVAL | B_RELBUF); bp->b_flags &= ~B_ASYNC; brelse(bp); @@ -1361,7 +1369,9 @@ restartsync: VNASSERT((bp->b_flags & B_DELWRI), vp, ("buf(%p) on dirty queue without DELWRI", bp)); + BO_LOCK(bo); bremfree(bp); + BO_UNLOCK(bo); bawrite(bp); BO_LOCK(bo); goto restartsync;