- Consistently track ni_dvp and ni_vp with dvfslocked and vfslocked rather

than trying to optimize it into a single lock.  This adds more calls to
   lock giant with non smpsafe filesystems but is the only way to reliably
   hold the correct lock.
 - Remove an invalid assert in the mountedhere case in lookup and fix the
   code to properly deal with the scenario.  We can actually have a lookup
   that returns dp == dvp with mountedhere set with certain unmount races.

Tested by:	kris
Reported by:	kris/mohans
This commit is contained in:
jeff 2006-04-28 00:59:48 +00:00
parent af7045cc53
commit 3450f7fc51

View File

@ -358,15 +358,15 @@ lookup(ndp)
int dpunlocked = 0; /* dp has already been unlocked */ int dpunlocked = 0; /* dp has already been unlocked */
struct componentname *cnp = &ndp->ni_cnd; struct componentname *cnp = &ndp->ni_cnd;
struct thread *td = cnp->cn_thread; struct thread *td = cnp->cn_thread;
int vfslocked; int vfslocked; /* VFS Giant state for child */
int dvfslocked; /* VFS Giant state for parent */
int tvfslocked; int tvfslocked;
int dvfslocked;
/* /*
* Setup: break out flag bits into variables. * Setup: break out flag bits into variables.
*/ */
vfslocked = (ndp->ni_cnd.cn_flags & GIANTHELD) != 0; dvfslocked = (ndp->ni_cnd.cn_flags & GIANTHELD) != 0;
dvfslocked = 0; vfslocked = 0;
ndp->ni_cnd.cn_flags &= ~GIANTHELD; ndp->ni_cnd.cn_flags &= ~GIANTHELD;
wantparent = cnp->cn_flags & (LOCKPARENT | WANTPARENT); wantparent = cnp->cn_flags & (LOCKPARENT | WANTPARENT);
KASSERT(cnp->cn_nameiop == LOOKUP || wantparent, KASSERT(cnp->cn_nameiop == LOOKUP || wantparent,
@ -520,8 +520,8 @@ lookup(ndp)
} }
tdp = dp; tdp = dp;
dp = dp->v_mount->mnt_vnodecovered; dp = dp->v_mount->mnt_vnodecovered;
tvfslocked = vfslocked; tvfslocked = dvfslocked;
vfslocked = VFS_LOCK_GIANT(dp->v_mount); dvfslocked = VFS_LOCK_GIANT(dp->v_mount);
VREF(dp); VREF(dp);
vput(tdp); vput(tdp);
VFS_UNLOCK_GIANT(tvfslocked); VFS_UNLOCK_GIANT(tvfslocked);
@ -543,6 +543,7 @@ lookup(ndp)
ndp->ni_dvp = dp; ndp->ni_dvp = dp;
ndp->ni_vp = NULL; ndp->ni_vp = NULL;
ASSERT_VOP_LOCKED(dp, "lookup"); ASSERT_VOP_LOCKED(dp, "lookup");
VNASSERT(vfslocked == 0, dp, ("lookup: vfslocked %d", vfslocked));
/* /*
* If we have a shared lock we may need to upgrade the lock for the * If we have a shared lock we may need to upgrade the lock for the
* last operation. * last operation.
@ -570,8 +571,8 @@ lookup(ndp)
(dp->v_mount->mnt_flag & MNT_UNION)) { (dp->v_mount->mnt_flag & MNT_UNION)) {
tdp = dp; tdp = dp;
dp = dp->v_mount->mnt_vnodecovered; dp = dp->v_mount->mnt_vnodecovered;
tvfslocked = vfslocked; tvfslocked = dvfslocked;
vfslocked = VFS_LOCK_GIANT(dp->v_mount); dvfslocked = VFS_LOCK_GIANT(dp->v_mount);
VREF(dp); VREF(dp);
vput(tdp); vput(tdp);
VFS_UNLOCK_GIANT(tvfslocked); VFS_UNLOCK_GIANT(tvfslocked);
@ -628,6 +629,7 @@ lookup(ndp)
} }
dp = ndp->ni_vp; dp = ndp->ni_vp;
vfslocked = VFS_LOCK_GIANT(dp->v_mount);
/* /*
* Check to see if the vnode has been mounted on; * Check to see if the vnode has been mounted on;
@ -635,14 +637,13 @@ lookup(ndp)
*/ */
while (dp->v_type == VDIR && (mp = dp->v_mountedhere) && while (dp->v_type == VDIR && (mp = dp->v_mountedhere) &&
(cnp->cn_flags & NOCROSSMOUNT) == 0) { (cnp->cn_flags & NOCROSSMOUNT) == 0) {
KASSERT(dp != ndp->ni_dvp, ("XXX"));
if (vfs_busy(mp, 0, 0, td)) if (vfs_busy(mp, 0, 0, td))
continue; continue;
vput(dp); vput(dp);
VFS_UNLOCK_GIANT(dvfslocked); VFS_UNLOCK_GIANT(vfslocked);
dvfslocked = vfslocked;
vfslocked = VFS_LOCK_GIANT(mp); vfslocked = VFS_LOCK_GIANT(mp);
VOP_UNLOCK(ndp->ni_dvp, 0, td); if (dp != ndp->ni_dvp)
VOP_UNLOCK(ndp->ni_dvp, 0, td);
error = VFS_ROOT(mp, cnp->cn_lkflags, &tdp, td); error = VFS_ROOT(mp, cnp->cn_lkflags, &tdp, td);
vfs_unbusy(mp, td); vfs_unbusy(mp, td);
vn_lock(ndp->ni_dvp, cnp->cn_lkflags | LK_RETRY, td); vn_lock(ndp->ni_dvp, cnp->cn_lkflags | LK_RETRY, td);
@ -704,7 +705,8 @@ lookup(ndp)
else else
vrele(ndp->ni_dvp); vrele(ndp->ni_dvp);
VFS_UNLOCK_GIANT(dvfslocked); VFS_UNLOCK_GIANT(dvfslocked);
dvfslocked = 0; dvfslocked = vfslocked; /* dp becomes dvp in dirloop */
vfslocked = 0;
goto dirloop; goto dirloop;
} }
/* /*