From 48e5ecf491d29733fcfbee5fdb5536755b910c4b Mon Sep 17 00:00:00 2001 From: jeff Date: Sun, 5 Oct 2003 07:16:45 +0000 Subject: [PATCH] - Check the XLOCK before inspecting v_data. - Slightly rewrite the fsync loop to be more lock friendly. We must acquire the vnode interlock before dropping the mnt lock. We must also check XLOCK to prevent vclean() races. - Use LK_INTERLOCK in the vget() in ffs_sync to further prevent vclean() races. - Use a local variable to store the results of the nvp == TAILQ_NEXT test so that we do not access the vp after we've vrele()d it. - Add an XXX comment about UFS_UPDATE() not being protected by any lock here. I suspect that it should need the VOP lock. --- sys/ufs/ffs/ffs_vfsops.c | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/sys/ufs/ffs/ffs_vfsops.c b/sys/ufs/ffs/ffs_vfsops.c index 3123b3d126f4..8be447597ae5 100644 --- a/sys/ufs/ffs/ffs_vfsops.c +++ b/sys/ufs/ffs/ffs_vfsops.c @@ -503,6 +503,10 @@ loop: } nvp = TAILQ_NEXT(vp, v_nmntvnodes); VI_LOCK(vp); + if (vp->v_iflag & VI_XLOCK) { + VI_UNLOCK(vp); + continue; + } mtx_unlock(&mntvnode_mtx); /* * Step 4: invalidate all inactive vnodes. @@ -1108,6 +1112,7 @@ ffs_sync(mp, waitfor, cred, td) struct ufsmount *ump = VFSTOUFS(mp); struct fs *fs; int error, count, wait, lockreq, allerror = 0; + int restart; fs = ump->um_fs; if (fs->fs_fmod != 0 && fs->fs_ronly != 0) { /* XXX */ @@ -1123,8 +1128,10 @@ ffs_sync(mp, waitfor, cred, td) wait = 1; lockreq = LK_EXCLUSIVE; } + lockreq |= LK_INTERLOCK; mtx_lock(&mntvnode_mtx); loop: + restart = 0; for (vp = TAILQ_FIRST(&mp->mnt_nvnodelist); vp != NULL; vp = nvp) { /* * If the vnode that we are about to sync is no longer @@ -1140,31 +1147,40 @@ loop: * call unless there's a good chance that we have work to do. */ nvp = TAILQ_NEXT(vp, v_nmntvnodes); + VI_LOCK(vp); + if (vp->v_iflag & VI_XLOCK) { + VI_UNLOCK(vp); + continue; + } ip = VTOI(vp); if (vp->v_type == VNON || ((ip->i_flag & (IN_ACCESS | IN_CHANGE | IN_MODIFIED | IN_UPDATE)) == 0 && TAILQ_EMPTY(&vp->v_dirtyblkhd))) { + VI_UNLOCK(vp); continue; } + mtx_unlock(&mntvnode_mtx); if (vp->v_type != VCHR) { - mtx_unlock(&mntvnode_mtx); if ((error = vget(vp, lockreq, td)) != 0) { mtx_lock(&mntvnode_mtx); if (error == ENOENT) goto loop; - } else { - if ((error = VOP_FSYNC(vp, cred, waitfor, td)) != 0) - allerror = error; - VOP_UNLOCK(vp, 0, td); - vrele(vp); - mtx_lock(&mntvnode_mtx); + continue; } + if ((error = VOP_FSYNC(vp, cred, waitfor, td)) != 0) + allerror = error; + VOP_UNLOCK(vp, 0, td); + mtx_lock(&mntvnode_mtx); + if (TAILQ_NEXT(vp, v_nmntvnodes) != nvp) + restart = 1; + vrele(vp); } else { - mtx_unlock(&mntvnode_mtx); + VI_UNLOCK(vp); + /* XXX UFS_UPDATE is not protected by any lock. */ UFS_UPDATE(vp, wait); mtx_lock(&mntvnode_mtx); } - if (TAILQ_NEXT(vp, v_nmntvnodes) != nvp) + if (restart) goto loop; } mtx_unlock(&mntvnode_mtx);