diff options
author | jeff <jeff@FreeBSD.org> | 2013-05-31 00:43:41 +0000 |
---|---|---|
committer | jeff <jeff@FreeBSD.org> | 2013-05-31 00:43:41 +0000 |
commit | d7efebc4db8c1b875c915fb1a2759cb9df4f2956 (patch) | |
tree | 47c74a9a8f1d3fbea9e54f402b3cf363e92546a4 /sys/kern | |
parent | 7c988b2ec361d43cc84f05e89080a923a7101a76 (diff) | |
download | FreeBSD-src-d7efebc4db8c1b875c915fb1a2759cb9df4f2956.zip FreeBSD-src-d7efebc4db8c1b875c915fb1a2759cb9df4f2956.tar.gz |
- Convert the bufobj lock to rwlock.
- Use a shared bufobj lock in getblk() and inmem().
- Convert softdep's lk to rwlock to match the bufobj lock.
- Move INFREECNT to b_flags and protect it with the buf lock.
- Remove unnecessary locking around bremfree() and BKGRDINPROG.
Sponsored by: EMC / Isilon Storage Division
Discussed with: mckusick, kib, mdf
Diffstat (limited to 'sys/kern')
-rw-r--r-- | sys/kern/vfs_bio.c | 141 | ||||
-rw-r--r-- | sys/kern/vfs_cluster.c | 15 | ||||
-rw-r--r-- | sys/kern/vfs_default.c | 2 | ||||
-rw-r--r-- | sys/kern/vfs_subr.c | 32 |
4 files changed, 65 insertions, 125 deletions
diff --git a/sys/kern/vfs_bio.c b/sys/kern/vfs_bio.c index 60816b1..7bd8d7e 100644 --- a/sys/kern/vfs_bio.c +++ b/sys/kern/vfs_bio.c @@ -418,11 +418,9 @@ bufcountwakeup(struct buf *bp) { int old; - KASSERT((bp->b_vflags & BV_INFREECNT) == 0, + KASSERT((bp->b_flags & B_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; + bp->b_flags |= B_INFREECNT; old = atomic_fetchadd_int(&numfreebuffers, 1); KASSERT(old >= 0 && old < nbuf, ("numfreebuffers climbed to %d", old + 1)); @@ -670,11 +668,10 @@ bufinit(void) for (i = 0; i < nbuf; i++) { bp = &buf[i]; bzero(bp, sizeof *bp); - bp->b_flags = B_INVAL; /* we're just an empty header */ + bp->b_flags = B_INVAL | B_INFREECNT; bp->b_rcred = NOCRED; bp->b_wcred = NOCRED; bp->b_qindex = QUEUE_EMPTY; - bp->b_vflags = BV_INFREECNT; /* buf is counted as free */ bp->b_xflags = 0; LIST_INIT(&bp->b_dep); BUF_LOCKINIT(bp); @@ -848,16 +845,14 @@ bremfree(struct buf *bp) ("bremfree: buffer %p already marked for delayed removal.", bp)); KASSERT(bp->b_qindex != QUEUE_NONE, ("bremfree: buffer %p not on a queue.", bp)); - BUF_ASSERT_HELD(bp); + BUF_ASSERT_XLOCKED(bp); bp->b_flags |= B_REMFREE; /* Fixup numfreebuffers count. */ if ((bp->b_flags & B_INVAL) || (bp->b_flags & B_DELWRI) == 0) { - KASSERT((bp->b_vflags & BV_INFREECNT) != 0, + KASSERT((bp->b_flags & B_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; + bp->b_flags &= ~B_INFREECNT; old = atomic_fetchadd_int(&numfreebuffers, -1); KASSERT(old > 0, ("numfreebuffers dropped to %d", old - 1)); } @@ -892,7 +887,7 @@ bremfreel(struct buf *bp) bp, bp->b_vp, bp->b_flags); KASSERT(bp->b_qindex != QUEUE_NONE, ("bremfreel: buffer %p not on a queue.", bp)); - BUF_ASSERT_HELD(bp); + BUF_ASSERT_XLOCKED(bp); mtx_assert(&bqlock, MA_OWNED); TAILQ_REMOVE(&bufqueues[bp->b_qindex], bp, b_freelist); @@ -916,11 +911,9 @@ bremfreel(struct buf *bp) * numfreebuffers. */ if ((bp->b_flags & B_INVAL) || (bp->b_flags & B_DELWRI) == 0) { - KASSERT((bp->b_vflags & BV_INFREECNT) != 0, + KASSERT((bp->b_flags & B_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; + bp->b_flags &= ~B_INFREECNT; old = atomic_fetchadd_int(&numfreebuffers, -1); KASSERT(old > 0, ("numfreebuffers dropped to %d", old - 1)); } @@ -1476,15 +1469,10 @@ brelse(struct buf *bp) bp->b_flags &= ~B_RELBUF; else if (buf_vm_page_count_severe()) { /* - * The locking of the BO_LOCK is not necessary since - * BKGRDINPROG cannot be set while we hold the buf - * lock, it can only be cleared if it is already - * pending. + * BKGRDINPROG can only be set with the buf and bufobj + * locks both held. We tolerate a race to clear it here. */ - if (bp->b_vp) { - if (!(bp->b_vflags & BV_BKGRDINPROG)) - bp->b_flags |= B_RELBUF; - } else + if (!(bp->b_vflags & BV_BKGRDINPROG)) bp->b_flags |= B_RELBUF; } @@ -1603,16 +1591,9 @@ brelse(struct buf *bp) /* enqueue */ mtx_lock(&bqlock); /* Handle delayed bremfree() processing. */ - if (bp->b_flags & B_REMFREE) { - struct bufobj *bo; - - bo = bp->b_bufobj; - if (bo != NULL) - BO_LOCK(bo); + if (bp->b_flags & B_REMFREE) bremfreel(bp); - if (bo != NULL) - BO_UNLOCK(bo); - } + if (bp->b_qindex != QUEUE_NONE) panic("brelse: free buffer onto another queue???"); @@ -1676,16 +1657,8 @@ brelse(struct buf *bp) * if B_INVAL is set ). */ - if (!(bp->b_flags & B_DELWRI)) { - struct bufobj *bo; - - bo = bp->b_bufobj; - if (bo != NULL) - BO_LOCK(bo); + if (!(bp->b_flags & B_DELWRI)) bufcountwakeup(bp); - if (bo != NULL) - BO_UNLOCK(bo); - } /* * Something we can maybe free or reuse @@ -1730,11 +1703,7 @@ bqrelse(struct buf *bp) 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); @@ -1744,13 +1713,9 @@ bqrelse(struct buf *bp) mtx_lock(&bqlock); /* Handle delayed bremfree() processing. */ - if (bp->b_flags & B_REMFREE) { - if (bo != NULL) - BO_LOCK(bo); + if (bp->b_flags & B_REMFREE) 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 */ @@ -1762,13 +1727,11 @@ bqrelse(struct buf *bp) #endif } else { /* - * The locking of the BO_LOCK for checking of the - * BV_BKGRDINPROG is not necessary since the - * BV_BKGRDINPROG cannot be set while we hold the buf - * lock, it can only be cleared if it is already - * pending. + * BKGRDINPROG can only be set with the buf and bufobj + * locks both held. We tolerate a race to clear it here. */ - if (!buf_vm_page_count_severe() || (bp->b_vflags & BV_BKGRDINPROG)) { + if (!buf_vm_page_count_severe() || + (bp->b_vflags & BV_BKGRDINPROG)) { bp->b_qindex = QUEUE_CLEAN; TAILQ_INSERT_TAIL(&bufqueues[QUEUE_CLEAN], bp, b_freelist); @@ -1788,13 +1751,8 @@ bqrelse(struct buf *bp) } mtx_unlock(&bqlock); - if ((bp->b_flags & B_INVAL) || !(bp->b_flags & B_DELWRI)) { - if (bo != NULL) - BO_LOCK(bo); + if ((bp->b_flags & B_INVAL) || !(bp->b_flags & B_DELWRI)) bufcountwakeup(bp); - if (bo != NULL) - BO_UNLOCK(bo); - } /* * Something we can maybe free or reuse. @@ -1940,7 +1898,7 @@ vfs_bio_awrite(struct buf *bp) size = vp->v_mount->mnt_stat.f_iosize; maxcl = MAXPHYS / size; - BO_LOCK(bo); + BO_RLOCK(bo); for (i = 1; i < maxcl; i++) if (vfs_bio_clcheck(vp, size, lblkno + i, bp->b_blkno + ((i * size) >> DEV_BSHIFT)) == 0) @@ -1950,7 +1908,7 @@ vfs_bio_awrite(struct buf *bp) if (vfs_bio_clcheck(vp, size, lblkno - j, bp->b_blkno - ((j * size) >> DEV_BSHIFT)) == 0) break; - BO_UNLOCK(bo); + BO_RUNLOCK(bo); --j; ncl = i + j; /* @@ -2145,7 +2103,7 @@ getnewbuf_reuse_bp(struct buf *bp, int qindex) bp->b_flags &= B_UNMAPPED | B_KVAALLOC; bp->b_ioflags = 0; bp->b_xflags = 0; - KASSERT((bp->b_vflags & BV_INFREECNT) == 0, + KASSERT((bp->b_flags & B_INFREECNT) == 0, ("buf %p still counted as free?", bp)); bp->b_vflags = 0; bp->b_vp = NULL; @@ -2293,24 +2251,19 @@ restart: */ if (BUF_LOCK(bp, LK_EXCLUSIVE | LK_NOWAIT, NULL) != 0) continue; - if (bp->b_vp) { - BO_LOCK(bp->b_bufobj); - if (bp->b_vflags & BV_BKGRDINPROG) { - BO_UNLOCK(bp->b_bufobj); - BUF_UNLOCK(bp); - continue; - } - BO_UNLOCK(bp->b_bufobj); + /* + * BKGRDINPROG can only be set with the buf and bufobj + * locks both held. We tolerate a race to clear it here. + */ + if (bp->b_vflags & BV_BKGRDINPROG) { + BUF_UNLOCK(bp); + continue; } KASSERT(bp->b_qindex == qindex, ("getnewbuf: inconsistent queue %d bp %p", qindex, bp)); - 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); /* * NOTE: nbp is now entirely invalid. We can only restart @@ -2653,14 +2606,15 @@ flushbufqueues(struct vnode *lvp, int queue, int flushdeps) BUF_UNLOCK(bp); continue; } - BO_LOCK(bp->b_bufobj); + /* + * BKGRDINPROG can only be set with the buf and bufobj + * locks both held. We tolerate a race to clear it here. + */ if ((bp->b_vflags & BV_BKGRDINPROG) != 0 || (bp->b_flags & B_DELWRI) == 0) { - BO_UNLOCK(bp->b_bufobj); BUF_UNLOCK(bp); continue; } - BO_UNLOCK(bp->b_bufobj); if (bp->b_flags & B_INVAL) { bremfreel(bp); mtx_unlock(&bqlock); @@ -2737,9 +2691,9 @@ incore(struct bufobj *bo, daddr_t blkno) { struct buf *bp; - BO_LOCK(bo); + BO_RLOCK(bo); bp = gbincore(bo, blkno); - BO_UNLOCK(bo); + BO_RUNLOCK(bo); return (bp); } @@ -3053,7 +3007,7 @@ loop: mtx_unlock(&nblock); } - BO_LOCK(bo); + BO_RLOCK(bo); bp = gbincore(bo, blkno); if (bp != NULL) { int lockflags; @@ -3067,7 +3021,7 @@ loop: lockflags |= LK_NOWAIT; error = BUF_TIMELOCK(bp, lockflags, - BO_MTX(bo), "getblk", slpflag, slptimeo); + BO_LOCKPTR(bo), "getblk", slpflag, slptimeo); /* * If we slept and got the lock we have to restart in case @@ -3094,11 +3048,8 @@ loop: bp->b_flags |= B_CACHE; if (bp->b_flags & B_MANAGED) MPASS(bp->b_qindex == QUEUE_NONE); - else { - BO_LOCK(bo); + else bremfree(bp); - BO_UNLOCK(bo); - } /* * check for size inconsistencies for non-VMIO case. @@ -3193,7 +3144,7 @@ loop: * returned by getnewbuf() is locked. Note that the returned * buffer is also considered valid (not marked B_INVAL). */ - BO_UNLOCK(bo); + BO_RUNLOCK(bo); /* * If the user does not want us to create the buffer, bail out * here. @@ -4400,7 +4351,7 @@ bufobj_wrefl(struct bufobj *bo) { KASSERT(bo != NULL, ("NULL bo in bufobj_wref")); - ASSERT_BO_LOCKED(bo); + ASSERT_BO_WLOCKED(bo); bo->bo_numoutput++; } @@ -4434,11 +4385,11 @@ bufobj_wwait(struct bufobj *bo, int slpflag, int timeo) int error; KASSERT(bo != NULL, ("NULL bo in bufobj_wwait")); - ASSERT_BO_LOCKED(bo); + ASSERT_BO_WLOCKED(bo); error = 0; while (bo->bo_numoutput) { bo->bo_flag |= BO_WWAIT; - error = msleep(&bo->bo_numoutput, BO_MTX(bo), + error = msleep(&bo->bo_numoutput, BO_LOCKPTR(bo), slpflag | (PRIBIO + 1), "bo_wwait", timeo); if (error) break; @@ -4596,7 +4547,7 @@ DB_COMMAND(countfreebufs, db_coundfreebufs) for (i = 0; i < nbuf; i++) { bp = &buf[i]; - if ((bp->b_vflags & BV_INFREECNT) != 0) + if ((bp->b_flags & B_INFREECNT) != 0) nfree++; else used++; diff --git a/sys/kern/vfs_cluster.c b/sys/kern/vfs_cluster.c index d619960..b280317 100644 --- a/sys/kern/vfs_cluster.c +++ b/sys/kern/vfs_cluster.c @@ -133,7 +133,7 @@ cluster_read(struct vnode *vp, u_quad_t filesize, daddr_t lblkno, long size, return 0; } else { bp->b_flags &= ~B_RAM; - BO_LOCK(bo); + BO_RLOCK(bo); for (i = 1; i < maxra; i++) { /* * Stop if the buffer does not exist or it @@ -156,7 +156,7 @@ cluster_read(struct vnode *vp, u_quad_t filesize, daddr_t lblkno, long size, BUF_UNLOCK(rbp); } } - BO_UNLOCK(bo); + BO_RUNLOCK(bo); if (i >= maxra) { return 0; } @@ -396,17 +396,16 @@ cluster_rbuild(struct vnode *vp, u_quad_t filesize, daddr_t lbn, * (marked B_CACHE), or locked (may be doing a * background write), or if the buffer is not * VMIO backed. The clustering code can only deal - * with VMIO-backed buffers. + * with VMIO-backed buffers. The bo lock is not + * required for the BKGRDINPROG check since it + * can not be set without the buf lock. */ - BO_LOCK(bo); if ((tbp->b_vflags & BV_BKGRDINPROG) || (tbp->b_flags & B_CACHE) || (tbp->b_flags & B_VMIO) == 0) { - BO_UNLOCK(bo); bqrelse(tbp); break; } - BO_UNLOCK(bo); /* * The buffer must be completely invalid in order to @@ -790,7 +789,7 @@ cluster_wbuild(struct vnode *vp, long size, daddr_t start_lbn, int len, continue; } if (BUF_LOCK(tbp, - LK_EXCLUSIVE | LK_NOWAIT | LK_INTERLOCK, BO_MTX(bo))) { + LK_EXCLUSIVE | LK_NOWAIT | LK_INTERLOCK, BO_LOCKPTR(bo))) { ++start_lbn; --len; continue; @@ -891,7 +890,7 @@ cluster_wbuild(struct vnode *vp, long size, daddr_t start_lbn, int len, */ if (BUF_LOCK(tbp, LK_EXCLUSIVE | LK_NOWAIT | LK_INTERLOCK, - BO_MTX(bo))) + BO_LOCKPTR(bo))) break; if ((tbp->b_flags & (B_VMIO | B_CLUSTEROK | diff --git a/sys/kern/vfs_default.c b/sys/kern/vfs_default.c index ed071b0..2c01117 100644 --- a/sys/kern/vfs_default.c +++ b/sys/kern/vfs_default.c @@ -662,7 +662,7 @@ loop2: continue; if (BUF_LOCK(bp, LK_EXCLUSIVE | LK_INTERLOCK | LK_SLEEPFAIL, - BO_MTX(bo)) != 0) { + BO_LOCKPTR(bo)) != 0) { BO_LOCK(bo); goto loop1; } diff --git a/sys/kern/vfs_subr.c b/sys/kern/vfs_subr.c index 0da6764..1f4a838 100644 --- a/sys/kern/vfs_subr.c +++ b/sys/kern/vfs_subr.c @@ -1073,7 +1073,7 @@ alloc: */ bo = &vp->v_bufobj; bo->__bo_vnode = vp; - mtx_init(BO_MTX(bo), "bufobj interlock", NULL, MTX_DEF); + rw_init(BO_LOCKPTR(bo), "bufobj interlock"); bo->bo_ops = &buf_ops_bio; bo->bo_private = vp; TAILQ_INIT(&bo->bo_clean.bv_hd); @@ -1331,7 +1331,7 @@ flushbuflist(struct bufv *bufv, int flags, struct bufobj *bo, int slpflag, daddr_t lblkno; b_xflags_t xflags; - ASSERT_BO_LOCKED(bo); + ASSERT_BO_WLOCKED(bo); retval = 0; TAILQ_FOREACH_SAFE(bp, &bufv->bv_hd, b_bobufs, nbp) { @@ -1347,7 +1347,7 @@ flushbuflist(struct bufv *bufv, int flags, struct bufobj *bo, int slpflag, } retval = EAGAIN; error = BUF_TIMELOCK(bp, - LK_EXCLUSIVE | LK_SLEEPFAIL | LK_INTERLOCK, BO_MTX(bo), + LK_EXCLUSIVE | LK_SLEEPFAIL | LK_INTERLOCK, BO_LOCKPTR(bo), "flushbuf", slpflag, slptimeo); if (error) { BO_LOCK(bo); @@ -1369,17 +1369,13 @@ flushbuflist(struct bufv *bufv, int flags, struct bufobj *bo, int slpflag, */ 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); @@ -1426,12 +1422,10 @@ restart: continue; if (BUF_LOCK(bp, LK_EXCLUSIVE | LK_SLEEPFAIL | LK_INTERLOCK, - BO_MTX(bo)) == ENOLCK) + BO_LOCKPTR(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); @@ -1452,11 +1446,9 @@ restart: continue; if (BUF_LOCK(bp, LK_EXCLUSIVE | LK_SLEEPFAIL | LK_INTERLOCK, - BO_MTX(bo)) == ENOLCK) + BO_LOCKPTR(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); @@ -1484,15 +1476,13 @@ restartsync: */ if (BUF_LOCK(bp, LK_EXCLUSIVE | LK_SLEEPFAIL | LK_INTERLOCK, - BO_MTX(bo)) == ENOLCK) { + BO_LOCKPTR(bo)) == ENOLCK) { goto restart; } 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; @@ -1512,7 +1502,7 @@ buf_vlist_remove(struct buf *bp) struct bufv *bv; KASSERT(bp->b_bufobj != NULL, ("No b_bufobj %p", bp)); - ASSERT_BO_LOCKED(bp->b_bufobj); + ASSERT_BO_WLOCKED(bp->b_bufobj); KASSERT((bp->b_xflags & (BX_VNDIRTY|BX_VNCLEAN)) != (BX_VNDIRTY|BX_VNCLEAN), ("buf_vlist_remove: Buf %p is on two lists", bp)); @@ -1538,7 +1528,7 @@ buf_vlist_add(struct buf *bp, struct bufobj *bo, b_xflags_t xflags) struct buf *n; int error; - ASSERT_BO_LOCKED(bo); + ASSERT_BO_WLOCKED(bo); KASSERT((bp->b_xflags & (BX_VNDIRTY|BX_VNCLEAN)) == 0, ("buf_vlist_add: Buf %p has existing xflags %d", bp, bp->b_xflags)); bp->b_xflags |= xflags; @@ -1598,7 +1588,7 @@ bgetvp(struct vnode *vp, struct buf *bp) struct bufobj *bo; bo = &vp->v_bufobj; - ASSERT_BO_LOCKED(bo); + ASSERT_BO_WLOCKED(bo); VNASSERT(bp->b_vp == NULL, bp->b_vp, ("bgetvp: not free")); CTR3(KTR_BUF, "bgetvp(%p) vp %p flags %X", bp, vp, bp->b_flags); @@ -1657,7 +1647,7 @@ vn_syncer_add_to_worklist(struct bufobj *bo, int delay) { int slot; - ASSERT_BO_LOCKED(bo); + ASSERT_BO_WLOCKED(bo); mtx_lock(&sync_mtx); if (bo->bo_flag & BO_ONWORKLST) @@ -2422,7 +2412,7 @@ vdropl(struct vnode *vp) rangelock_destroy(&vp->v_rl); lockdestroy(vp->v_vnlock); mtx_destroy(&vp->v_interlock); - mtx_destroy(BO_MTX(bo)); + rw_destroy(BO_LOCKPTR(bo)); uma_zfree(vnode_zone, vp); } |