Diff below implements most of the missing locking goo for NFS without
enabling it. It does the following:
- Add a missing PDIRUNLOCK in nfs_lookup()
- Change vrele(9) into vput(9) where necessary. nfs_nget() will in
future return a locked NFSnode. Just like ffs_vget() returns a
locked vnode. So every time it is called we need a corresponding
vput(9).
- Make sure VN_KNOTE() is always called with a valid reference, before
vrele(9) or vput(9).
- Substitute an XXX by an assert in nfs_removeit().
However as long as nfs_lock/unlock are noops, this diff should not
introduce any change in behavior. But please do test this diff :)
Ok?
Index: nfs/nfs_vnops.c
===================================================================
RCS file: /cvs/src/sys/nfs/nfs_vnops.c,v
retrieving revision 1.171
diff -u -p -r1.171 nfs_vnops.c
--- nfs/nfs_vnops.c 22 Feb 2017 11:42:46 -0000 1.171
+++ nfs/nfs_vnops.c 9 Apr 2018 14:00:09 -0000
@@ -769,6 +769,7 @@ nfs_lookup(void *v)
flags = cnp->cn_flags;
*vpp = NULLVP;
+ newvp = NULLVP;
if ((flags & ISLASTCN) && (dvp->v_mount->mnt_flag & MNT_RDONLY) &&
(cnp->cn_nameiop == DELETE || cnp->cn_nameiop == RENAME))
return (EROFS);
@@ -836,8 +837,10 @@ nfs_lookup(void *v)
if (cnp->cn_nameiop != LOOKUP && (flags & ISLASTCN))
cnp->cn_flags |= SAVENAME;
if ((!lockparent || !(flags & ISLASTCN)) &&
- newvp != dvp)
+ newvp != dvp) {
VOP_UNLOCK(dvp, p);
+ cnp->cn_flags |= PDIRUNLOCK;
+ }
return (0);
}
cache_purge(newvp);
@@ -1318,10 +1321,6 @@ nfs_mknodrpc(struct vnode *dvp, struct v
if (!error) {
nfsm_mtofh(dvp, newvp, info.nmi_v3, gotvp);
if (!gotvp) {
- if (newvp) {
- vrele(newvp);
- newvp = NULL;
- }
error = nfs_lookitup(dvp, cnp->cn_nameptr,
cnp->cn_namelen, cnp->cn_cred, cnp->cn_proc, &np);
if (!error)
@@ -1335,7 +1334,7 @@ nfs_mknodrpc(struct vnode *dvp, struct v
nfsmout:
if (error) {
if (newvp)
- vrele(newvp);
+ vput(newvp);
} else {
if (cnp->cn_flags & MAKEENTRY)
nfs_cache_enter(dvp, newvp, cnp);
@@ -1345,7 +1344,7 @@ nfsmout:
VTONFS(dvp)->n_flag |= NMODIFIED;
if (!wccflag)
NFS_INVALIDATE_ATTRCACHE(VTONFS(dvp));
- vrele(dvp);
+ vput(dvp);
return (error);
}
@@ -1362,7 +1361,7 @@ nfs_mknod(void *v)
error = nfs_mknodrpc(ap->a_dvp, &newvp, ap->a_cnp, ap->a_vap);
if (!error)
- vrele(newvp);
+ vput(newvp);
VN_KNOTE(ap->a_dvp, NOTE_WRITE);
@@ -1430,10 +1429,6 @@ again:
if (!error) {
nfsm_mtofh(dvp, newvp, info.nmi_v3, gotvp);
if (!gotvp) {
- if (newvp) {
- vrele(newvp);
- newvp = NULL;
- }
error = nfs_lookitup(dvp, cnp->cn_nameptr,
cnp->cn_namelen, cnp->cn_cred, cnp->cn_proc, &np);
if (!error)
@@ -1451,7 +1446,7 @@ nfsmout:
goto again;
}
if (newvp)
- vrele(newvp);
+ vput(newvp);
} else if (info.nmi_v3 && (fmode & O_EXCL))
error = nfs_setattrrpc(newvp, vap, cnp->cn_cred, cnp->cn_proc);
if (!error) {
@@ -1464,7 +1459,7 @@ nfsmout:
if (!wccflag)
NFS_INVALIDATE_ATTRCACHE(VTONFS(dvp));
VN_KNOTE(ap->a_dvp, NOTE_WRITE);
- vrele(dvp);
+ vput(dvp);
return (error);
}
@@ -1530,12 +1525,13 @@ nfs_remove(void *v)
error = nfs_sillyrename(dvp, vp, cnp);
pool_put(&namei_pool, cnp->cn_pnbuf);
NFS_INVALIDATE_ATTRCACHE(np);
- vrele(dvp);
- vrele(vp);
-
VN_KNOTE(vp, NOTE_DELETE);
VN_KNOTE(dvp, NOTE_WRITE);
-
+ if (dvp == vp)
+ vrele(vp);
+ else
+ vput(vp);
+ vput(dvp);
return (error);
}
@@ -1545,9 +1541,9 @@ nfs_remove(void *v)
int
nfs_removeit(struct sillyrename *sp)
{
+ KASSERT(VOP_ISLOCKED(sp->s_dvp));
/*
* Make sure that the directory vnode is still valid.
- * XXX we should lock sp->s_dvp here.
*
* NFS can potentially try to nuke a silly *after* the directory
* has already been pushed out on a forced unmount. Since the silly
@@ -1628,7 +1624,7 @@ nfs_rename(void *v)
if (tvp && tvp->v_usecount > 1 && !VTONFS(tvp)->n_sillyrename &&
tvp->v_type != VDIR && !nfs_sillyrename(tdvp, tvp, tcnp)) {
VN_KNOTE(tvp, NOTE_DELETE);
- vrele(tvp);
+ vput(tvp);
tvp = NULL;
}
@@ -1827,13 +1823,13 @@ nfs_symlink(void *v)
nfsmout:
if (newvp)
- vrele(newvp);
+ vput(newvp);
pool_put(&namei_pool, cnp->cn_pnbuf);
VTONFS(dvp)->n_flag |= NMODIFIED;
if (!wccflag)
NFS_INVALIDATE_ATTRCACHE(VTONFS(dvp));
VN_KNOTE(dvp, NOTE_WRITE);
- vrele(dvp);
+ vput(dvp);
return (error);
}
@@ -1904,7 +1900,7 @@ nfsmout:
}
if (error) {
if (newvp)
- vrele(newvp);
+ vput(newvp);
} else {
VN_KNOTE(dvp, NOTE_WRITE|NOTE_LINK);
if (cnp->cn_flags & MAKEENTRY)
@@ -1912,7 +1908,7 @@ nfsmout:
*ap->a_vpp = newvp;
}
pool_put(&namei_pool, cnp->cn_pnbuf);
- vrele(dvp);
+ vput(dvp);
return (error);
}
@@ -1936,7 +1932,7 @@ nfs_rmdir(void *v)
if (dvp == vp) {
vrele(dvp);
- vrele(dvp);
+ vput(dvp);
pool_put(&namei_pool, cnp->cn_pnbuf);
return (EINVAL);
}
@@ -1964,8 +1960,8 @@ nfsmout:
VN_KNOTE(vp, NOTE_DELETE);
cache_purge(vp);
- vrele(vp);
- vrele(dvp);
+ vput(vp);
+ vput(dvp);
/*
* Kludge: Map ENOENT => 0 assuming that you have a reply to a retry.
*/
@@ -2492,7 +2488,10 @@ nfs_readdirplusrpc(struct vnode *vp, str
nfsm_adv(nfsm_rndup(i));
}
if (newvp != NULLVP) {
- vrele(newvp);
+ if (newvp == vp)
+ vrele(newvp);
+ else
+ vput(newvp);
newvp = NULLVP;
}
nfsm_dissect(tl, u_int32_t *, NFSX_UNSIGNED);
@@ -2533,8 +2532,12 @@ nfs_readdirplusrpc(struct vnode *vp, str
}
nfsmout:
- if (newvp != NULLVP)
- vrele(newvp);
+ if (newvp != NULLVP) {
+ if (newvp == vp)
+ vrele(newvp);
+ else
+ vput(newvp);
+ }
return (error);
}
@@ -2659,7 +2662,7 @@ nfs_lookitup(struct vnode *dvp, char *na
nfsm_postop_attr(newvp, attrflag);
if (!attrflag && *npp == NULL) {
m_freem(info.nmi_mrep);
- vrele(newvp);
+ vput(newvp);
return (ENOENT);
}
} else
@@ -2670,7 +2673,7 @@ nfsmout:
if (npp && *npp == NULL) {
if (error) {
if (newvp)
- vrele(newvp);
+ vput(newvp);
} else
*npp = np;
}
