From f6576f194e677d80e7f63cdd115a3eee2daf3c87 Mon Sep 17 00:00:00 2001 From: Jeff Roberson Date: Mon, 28 Mar 2005 09:29:58 +0000 Subject: - We no longer have to bother with PDIRUNLOCK, lookup() handles it for us. - Network filesystems are written with a special idiom that checks the cache first, and may even unlock dvp before discovering that a network round-trip is required to resolve the name. I believe dvp is prevented from being recycled even in the forced unmount case by the shared lock on the mount point. If not, this code should grow checks for VI_DOOMED after it relocks dvp or it will access NULL v_data fields. Sponsored by: Isilon Systems, Inc. --- sys/fs/smbfs/smbfs_vnops.c | 75 +++++++++++++-------------------------------- sys/nfs4client/nfs4_vnops.c | 57 ++++++---------------------------- sys/nfsclient/nfs_vnops.c | 55 ++++++--------------------------- 3 files changed, 41 insertions(+), 146 deletions(-) (limited to 'sys') diff --git a/sys/fs/smbfs/smbfs_vnops.c b/sys/fs/smbfs/smbfs_vnops.c index 9b27a527bcf1..94f3a0c5cf60 100644 --- a/sys/fs/smbfs/smbfs_vnops.c +++ b/sys/fs/smbfs/smbfs_vnops.c @@ -1050,10 +1050,6 @@ smbfs_pathcheck(struct smbmount *smp, const char *name, int nmlen, int nameiop) return 0; } -#ifndef PDIRUNLOCK -#define PDIRUNLOCK 0 -#endif - /* * Things go even weird without fixed inode numbers... */ @@ -1080,11 +1076,10 @@ smbfs_lookup(ap) int flags = cnp->cn_flags; int nameiop = cnp->cn_nameiop; int nmlen = cnp->cn_namelen; - int lockparent, wantparent, error, islastcn, isdot; + int wantparent, error, islastcn, isdot; int killit; SMBVDEBUG("\n"); - cnp->cn_flags &= ~PDIRUNLOCK; if (dvp->v_type != VDIR) return ENOTDIR; if ((flags & ISDOTDOT) && (dvp->v_vflag & VV_ROOT)) { @@ -1108,7 +1103,6 @@ smbfs_lookup(ap) return EROFS; if ((error = VOP_ACCESS(dvp, VEXEC, cnp->cn_cred, td)) != 0) return error; - lockparent = flags & LOCKPARENT; wantparent = flags & (LOCKPARENT|WANTPARENT); smp = VFSTOSMBFS(mp); dnp = VTOSMB(dvp); @@ -1128,27 +1122,19 @@ smbfs_lookup(ap) vhold(*vpp); vp = *vpp; - mp_fixme("Unlocked v_id access."); if (dvp == vp) { /* lookup on current */ vref(vp); error = 0; SMBVDEBUG("cached '.'\n"); } else if (flags & ISDOTDOT) { VOP_UNLOCK(dvp, 0, td); /* unlock parent */ - cnp->cn_flags |= PDIRUNLOCK; error = vget(vp, LK_EXCLUSIVE, td); - if (!error && lockparent && islastcn) { - error = vn_lock(dvp, LK_EXCLUSIVE, td); - if (error == 0) - cnp->cn_flags &= ~PDIRUNLOCK; - } - } else { + if (error) + if (vn_lock(dvp, LK_EXCLUSIVE, td)) + panic("smbfs_lookup: Can't " + "relock directory."); + } else error = vget(vp, LK_EXCLUSIVE, td); - if (!lockparent || error || !islastcn) { - VOP_UNLOCK(dvp, 0, td); - cnp->cn_flags |= PDIRUNLOCK; - } - } if (!error) { killit = 0; error = VOP_GETATTR(vp, &vattr, cnp->cn_cred, td); @@ -1174,20 +1160,25 @@ smbfs_lookup(ap) return (0); } cache_purge(vp); - if (killit) + /* + * XXX This is not quite right, if '.' is + * inconsistent, we really need to start the lookup + * all over again. Hopefully there is some other + * guarantee that prevents this case from happening. + */ + if (killit && vp != dvp) vgone(vp); - vput(vp); - if (lockparent && dvp != vp && islastcn) - VOP_UNLOCK(dvp, 0, td); + if (vp != dvp) + vput(vp); + else + vrele(vp); + if (flags & ISDOTDOT) + if (vn_lock(dvp, LK_EXCLUSIVE, td)) + panic("smbfs_lookup: Can't " + "relock directory."); } vdrop(vp); - error = vn_lock(dvp, LK_EXCLUSIVE, td); *vpp = NULLVP; - if (error) { - cnp->cn_flags |= PDIRUNLOCK; - return (error); - } - cnp->cn_flags &= ~PDIRUNLOCK; } /* * entry is not in the cache or has been expired @@ -1217,10 +1208,6 @@ smbfs_lookup(ap) if (error) return error; cnp->cn_flags |= SAVENAME; - if (!lockparent) { - VOP_UNLOCK(dvp, 0, td); - cnp->cn_flags |= PDIRUNLOCK; - } return (EJUSTRETURN); } return ENOENT; @@ -1244,10 +1231,6 @@ smbfs_lookup(ap) return error; *vpp = vp; cnp->cn_flags |= SAVENAME; - if (!lockparent) { - VOP_UNLOCK(dvp, 0, td); - cnp->cn_flags |= PDIRUNLOCK; - } return 0; } if (nameiop == RENAME && islastcn && wantparent) { @@ -1261,10 +1244,6 @@ smbfs_lookup(ap) return error; *vpp = vp; cnp->cn_flags |= SAVENAME; - if (!lockparent) { - VOP_UNLOCK(dvp, 0, td); - cnp->cn_flags |= PDIRUNLOCK; - } return 0; } if (flags & ISDOTDOT) { @@ -1274,14 +1253,6 @@ smbfs_lookup(ap) vn_lock(dvp, LK_EXCLUSIVE | LK_RETRY, td); return error; } - if (lockparent && islastcn) { - error = vn_lock(dvp, LK_EXCLUSIVE, td); - if (error) { - cnp->cn_flags |= PDIRUNLOCK; - vput(vp); - return error; - } - } *vpp = vp; } else if (isdot) { vref(dvp); @@ -1292,10 +1263,6 @@ smbfs_lookup(ap) return error; *vpp = vp; SMBVDEBUG("lookup: getnewvp!\n"); - if (!lockparent || !islastcn) { - VOP_UNLOCK(dvp, 0, td); - cnp->cn_flags |= PDIRUNLOCK; - } } if ((cnp->cn_flags & MAKEENTRY)/* && !islastcn*/) { /* VTOSMB(*vpp)->n_ctime = VTOSMB(*vpp)->n_vattr.va_ctime.tv_sec;*/ diff --git a/sys/nfs4client/nfs4_vnops.c b/sys/nfs4client/nfs4_vnops.c index ca1f3032dd69..be6bd858d38c 100644 --- a/sys/nfs4client/nfs4_vnops.c +++ b/sys/nfs4client/nfs4_vnops.c @@ -937,7 +937,7 @@ nfs4_lookup(struct vop_lookup_args *ap) long len; nfsfh_t *fhp; struct nfsnode *np; - int lockparent, wantparent, error = 0, fhsize; + int wantparent, error = 0, fhsize; struct thread *td = cnp->cn_thread; struct nfs4_compound cp; struct nfs4_oparg_getattr ga, dga; @@ -945,13 +945,11 @@ nfs4_lookup(struct vop_lookup_args *ap) struct nfs4_oparg_getfh gfh; *vpp = NULLVP; - cnp->cn_flags &= ~PDIRUNLOCK; if ((flags & ISLASTCN) && (dvp->v_mount->mnt_flag & MNT_RDONLY) && (cnp->cn_nameiop == DELETE || cnp->cn_nameiop == RENAME)) return (EROFS); if (dvp->v_type != VDIR) return (ENOTDIR); - lockparent = flags & LOCKPARENT; wantparent = flags & (LOCKPARENT|WANTPARENT); nmp = VFSTONFS(dvp->v_mount); np = VTONFS(dvp); @@ -977,20 +975,11 @@ nfs4_lookup(struct vop_lookup_args *ap) error = 0; } else if (flags & ISDOTDOT) { VOP_UNLOCK(dvp, 0, td); - cnp->cn_flags |= PDIRUNLOCK; error = vget(newvp, LK_EXCLUSIVE, td); - if (!error && lockparent && (flags & ISLASTCN)) { - error = vn_lock(dvp, LK_EXCLUSIVE, td); - if (error == 0) - cnp->cn_flags &= ~PDIRUNLOCK; - } - } else { + if (error) + vn_lock(dvp, LK_EXCLUSIVE|LK_RETRY, td); + } else error = vget(newvp, LK_EXCLUSIVE, td); - if (!lockparent || error || !(flags & ISLASTCN)) { - VOP_UNLOCK(dvp, 0, td); - cnp->cn_flags |= PDIRUNLOCK; - } - } if (!error) { if (!VOP_GETATTR(newvp, &vattr, cnp->cn_cred, td) && vattr.va_ctime.tv_sec == VTONFS(newvp)->n_ctime) { @@ -1002,18 +991,15 @@ nfs4_lookup(struct vop_lookup_args *ap) return (0); } cache_purge(newvp); - vput(newvp); - if (lockparent && dvp != newvp && (flags & ISLASTCN)) - VOP_UNLOCK(dvp, 0, td); + if (newvp != dvp) + vput(newvp); + else + vrele(newvp); + if (flags & ISDOTDOT) + vn_lock(dvp, LK_EXCLUSIVE|LK_RETRY, td); } vdrop(newvp); - error = vn_lock(dvp, LK_EXCLUSIVE, td); *vpp = NULLVP; - if (error) { - cnp->cn_flags |= PDIRUNLOCK; - return (error); - } - cnp->cn_flags &= ~PDIRUNLOCK; } error = 0; @@ -1079,10 +1065,6 @@ nfs4_lookup(struct vop_lookup_args *ap) *vpp = newvp; cnp->cn_flags |= SAVENAME; - if (!lockparent) { - VOP_UNLOCK(dvp, 0, td); - cnp->cn_flags |= PDIRUNLOCK; - } return (0); } @@ -1097,16 +1079,6 @@ nfs4_lookup(struct vop_lookup_args *ap) newvp = NFSTOV(np); nfs4_vnop_loadattrcache(newvp, &ga.fa, NULL); - - if (lockparent && (flags & ISLASTCN)) { - error = vn_lock(dvp, LK_EXCLUSIVE, td); - if (error) { - cnp->cn_flags |= PDIRUNLOCK; - vput(newvp); - return (error); - } - } else - cnp->cn_flags |= PDIRUNLOCK; } else if (NFS_CMPFH(np, fhp, fhsize)) { VREF(dvp); newvp = dvp; @@ -1114,11 +1086,6 @@ nfs4_lookup(struct vop_lookup_args *ap) error = nfs_nget(dvp->v_mount, fhp, fhsize, &np); if (error) return (error); - - if (!lockparent || !(flags & ISLASTCN)) { - cnp->cn_flags |= PDIRUNLOCK; - VOP_UNLOCK(dvp, 0, td); - } newvp = NFSTOV(np); /* Fill in np used by open. */ @@ -1152,10 +1119,6 @@ nfsmout: } if ((cnp->cn_nameiop == CREATE || cnp->cn_nameiop == RENAME) && (flags & ISLASTCN) && error == ENOENT) { - if (!lockparent) { - VOP_UNLOCK(dvp, 0, td); - cnp->cn_flags |= PDIRUNLOCK; - } if (dvp->v_mount->mnt_flag & MNT_RDONLY) error = EROFS; else diff --git a/sys/nfsclient/nfs_vnops.c b/sys/nfsclient/nfs_vnops.c index d3059a26f7bf..2dfb3456bdb8 100644 --- a/sys/nfsclient/nfs_vnops.c +++ b/sys/nfsclient/nfs_vnops.c @@ -776,18 +776,16 @@ nfs_lookup(struct vop_lookup_args *ap) long len; nfsfh_t *fhp; struct nfsnode *np; - int lockparent, wantparent, error = 0, attrflag, fhsize; + int wantparent, error = 0, attrflag, fhsize; int v3 = NFS_ISV3(dvp); struct thread *td = cnp->cn_thread; *vpp = NULLVP; - cnp->cn_flags &= ~PDIRUNLOCK; if ((flags & ISLASTCN) && (dvp->v_mount->mnt_flag & MNT_RDONLY) && (cnp->cn_nameiop == DELETE || cnp->cn_nameiop == RENAME)) return (EROFS); if (dvp->v_type != VDIR) return (ENOTDIR); - lockparent = flags & LOCKPARENT; wantparent = flags & (LOCKPARENT|WANTPARENT); nmp = VFSTONFS(dvp->v_mount); np = VTONFS(dvp); @@ -810,20 +808,11 @@ nfs_lookup(struct vop_lookup_args *ap) error = 0; } else if (flags & ISDOTDOT) { VOP_UNLOCK(dvp, 0, td); - cnp->cn_flags |= PDIRUNLOCK; error = vget(newvp, LK_EXCLUSIVE, td); - if (!error && lockparent && (flags & ISLASTCN)) { - error = vn_lock(dvp, LK_EXCLUSIVE, td); - if (error == 0) - cnp->cn_flags &= ~PDIRUNLOCK; - } - } else { + if (error) + vn_lock(dvp, LK_EXCLUSIVE|LK_RETRY, td); + } else error = vget(newvp, LK_EXCLUSIVE, td); - if (!lockparent || error || !(flags & ISLASTCN)) { - VOP_UNLOCK(dvp, 0, td); - cnp->cn_flags |= PDIRUNLOCK; - } - } if (!error) { if (!VOP_GETATTR(newvp, &vattr, cnp->cn_cred, td) && vattr.va_ctime.tv_sec == VTONFS(newvp)->n_ctime) { @@ -835,18 +824,15 @@ nfs_lookup(struct vop_lookup_args *ap) return (0); } cache_purge(newvp); - vput(newvp); - if (lockparent && dvp != newvp && (flags & ISLASTCN)) - VOP_UNLOCK(dvp, 0, td); + if (dvp != newvp) + vput(newvp); + else + vrele(newvp); + if (flags & ISDOTDOT) + vn_lock(dvp, LK_EXCLUSIVE|LK_RETRY, td); } vdrop(newvp); - error = vn_lock(dvp, LK_EXCLUSIVE, td); *vpp = NULLVP; - if (error) { - cnp->cn_flags |= PDIRUNLOCK; - return (error); - } - cnp->cn_flags &= ~PDIRUNLOCK; } error = 0; newvp = NULLVP; @@ -889,10 +875,6 @@ nfs_lookup(struct vop_lookup_args *ap) *vpp = newvp; m_freem(mrep); cnp->cn_flags |= SAVENAME; - if (!lockparent) { - VOP_UNLOCK(dvp, 0, td); - cnp->cn_flags |= PDIRUNLOCK; - } return (0); } @@ -904,15 +886,6 @@ nfs_lookup(struct vop_lookup_args *ap) return (error); } newvp = NFSTOV(np); - if (lockparent && (flags & ISLASTCN)) { - error = vn_lock(dvp, LK_EXCLUSIVE, td); - if (error) { - cnp->cn_flags |= PDIRUNLOCK; - vput(newvp); - return (error); - } - } else - cnp->cn_flags |= PDIRUNLOCK; } else if (NFS_CMPFH(np, fhp, fhsize)) { VREF(dvp); newvp = dvp; @@ -922,10 +895,6 @@ nfs_lookup(struct vop_lookup_args *ap) m_freem(mrep); return (error); } - if (!lockparent || !(flags & ISLASTCN)) { - cnp->cn_flags |= PDIRUNLOCK; - VOP_UNLOCK(dvp, 0, td); - } newvp = NFSTOV(np); } if (v3) { @@ -950,10 +919,6 @@ nfsmout: } if ((cnp->cn_nameiop == CREATE || cnp->cn_nameiop == RENAME) && (flags & ISLASTCN) && error == ENOENT) { - if (!lockparent) { - VOP_UNLOCK(dvp, 0, td); - cnp->cn_flags |= PDIRUNLOCK; - } if (dvp->v_mount->mnt_flag & MNT_RDONLY) error = EROFS; else -- cgit v1.2.3