d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix LORs between vn_start_write() and vn_lock() in nfsrv_copymr().

Browse Source 
When coding the pNFS server, I added vn_start_write() calls in nfsrv_copymr()
done while the vnodes were locked, not realizing I had introduced LORs and
possible deadlock when an exported file system on the MDS is suspended.
This patch fixes the LORs by moving the vn_start_write() calls up to before
where the vnodes are locked. For "tvp", the vn_start_write() probaby isn't
necessary, because NFS mounts can't be suspended. However, I think doing
so is harmless.
Thanks go to kib@ for letting me know that I had introduced these LORs.
This patch only affects the behaviour of the pNFS server when pnfsdscopymr(8)
is used to recover a mirrored DS.
This commit is contained in:
rmacklem 2018-08-18 19:14:06 +00:00
parent dce7d8625a
commit 453e23bd43
1 changed files with 26 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
sys/fs/nfsserver/nfs_nfsdstate.c
View File

@ -7979,7 +7979,7 @@ nfsrv_copymr(vnode_t vp, vnode_t fvp, vnode_t dvp, struct nfsdevice *ds,
struct nfslayouthash *lhyp;
struct nfslayout *lyp, *nlyp;
struct nfslayouthead thl;
struct mount *mp;
struct mount *mp, *tvmp;
struct acl *aclp;
struct vattr va;
struct timespec mtime;
@ -8042,6 +8042,7 @@ nfsrv_copymr(vnode_t vp, vnode_t fvp, vnode_t dvp, struct nfsdevice *ds,
NFSDRECALLUNLOCK();
ret = 0;
mp = tvmp = NULL;
didprintf = 0;
TAILQ_INIT(&thl);
/* Unlock the MDS vp, so that a LayoutReturn can be done on it. */
@ -8115,6 +8116,20 @@ nfsrv_copymr(vnode_t vp, vnode_t fvp, vnode_t dvp, struct nfsdevice *ds,
TAILQ_FOREACH_SAFE(lyp, &thl, lay_list, nlyp)
nfsrv_freelayout(&thl, lyp);
/*
* Do the vn_start_write() calls here, before the MDS vnode is
* locked and the tvp is created (locked) in the NFS file system
* that dvp is in.
* For tvmp, this probably isn't necessary, since it will be an
* NFS mount and they are not suspendable at this time.
*/
if (ret == 0)
ret = vn_start_write(vp, &mp, V_WAIT | PCATCH);
if (ret == 0) {
tvmp = dvp->v_mount;
ret = vn_start_write(NULL, &tvmp, V_WAIT | PCATCH);
}
/*
* LK_EXCLUSIVE lock the MDS vnode, so that any
* proxied writes through the MDS will be blocked until we have
@ -8123,7 +8138,7 @@ nfsrv_copymr(vnode_t vp, vnode_t fvp, vnode_t dvp, struct nfsdevice *ds,
* changed until the copy is complete.
*/
NFSVOPLOCK(vp, LK_EXCLUSIVE | LK_RETRY);
if ((vp->v_iflag & VI_DOOMED) != 0) {
if (ret == 0 && (vp->v_iflag & VI_DOOMED) != 0) {
NFSD_DEBUG(4, "nfsrv_copymr: lk_exclusive doomed\n");
ret = ESTALE;
}
@ -8148,10 +8163,7 @@ nfsrv_copymr(vnode_t vp, vnode_t fvp, vnode_t dvp, struct nfsdevice *ds,
nfsrv_zeropnfsdat = malloc(PNFSDS_COPYSIZ, M_TEMP,
M_WAITOK | M_ZERO);
rdpos = wrpos = 0;
mp = NULL;
ret = vn_start_write(tvp, &mp, V_WAIT | PCATCH);
if (ret == 0)
ret = VOP_GETATTR(fvp, &va, cred);
ret = VOP_GETATTR(fvp, &va, cred);
aresid = 0;
while (ret == 0 && aresid == 0) {
ret = vn_rdwr(UIO_READ, fvp, dat, PNFSDS_COPYSIZ,
@ -8191,8 +8203,6 @@ nfsrv_copymr(vnode_t vp, vnode_t fvp, vnode_t dvp, struct nfsdevice *ds,
ret = 0;
}
if (mp != NULL)
vn_finished_write(mp);
if (ret == 0)
ret = VOP_FSYNC(tvp, MNT_WAIT, p);
@ -8210,18 +8220,16 @@ nfsrv_copymr(vnode_t vp, vnode_t fvp, vnode_t dvp, struct nfsdevice *ds,
acl_free(aclp);
free(dat, M_TEMP);
}
if (tvmp != NULL)
vn_finished_write(tvmp);
/* Update the extended attributes for the newly created DS file. */
if (ret == 0) {
mp = NULL;
ret = vn_start_write(vp, &mp, V_WAIT | PCATCH);
if (ret == 0)
ret = vn_extattr_set(vp, IO_NODELOCKED,
EXTATTR_NAMESPACE_SYSTEM, "pnfsd.dsfile",
sizeof(*wpf) * mirrorcnt, (char *)wpf, p);
if (mp != NULL)
vn_finished_write(mp);
}
if (ret == 0)
ret = vn_extattr_set(vp, IO_NODELOCKED,
EXTATTR_NAMESPACE_SYSTEM, "pnfsd.dsfile",
sizeof(*wpf) * mirrorcnt, (char *)wpf, p);
if (mp != NULL)
vn_finished_write(mp);
/* Get rid of the dontlist entry, so that Layouts can be issued. */
NFSDDONTLISTLOCK();