svn commit: r276441 - stable/9/sys/fs/nfsserver
Rick Macklem
rmacklem at FreeBSD.org
Wed Dec 31 01:34:38 UTC 2014
Author: rmacklem
Date: Wed Dec 31 01:34:37 2014
New Revision: 276441
URL: https://svnweb.freebsd.org/changeset/base/276441
Log:
MFC: r276193
A deadlock in the NFSv4 server with vfs.nfsd.enable_locallocks=1
was reported via email. This was caused by a LOR between the
sleep lock used to serialize the local locking (nfsrv_locklf())
and locking the vnode. I believe this patch fixes the problem
by delaying relocking of the vnode until the sleep lock is
unlocked (nfsrv_unlocklf()). To avoid nfsvno_advlock() having the side
effect of unlocking the vnode, unlocking the vnode was moved to before
the functions that call nfsvno_advlock().
It shouldn't affect the execution of the default case where
vfs.nfsd.enable_locallocks=0.
Modified:
stable/9/sys/fs/nfsserver/nfs_nfsdport.c
stable/9/sys/fs/nfsserver/nfs_nfsdstate.c
Directory Properties:
stable/9/sys/ (props changed)
stable/9/sys/fs/ (props changed)
Modified: stable/9/sys/fs/nfsserver/nfs_nfsdport.c
==============================================================================
--- stable/9/sys/fs/nfsserver/nfs_nfsdport.c Wed Dec 31 01:29:44 2014 (r276440)
+++ stable/9/sys/fs/nfsserver/nfs_nfsdport.c Wed Dec 31 01:34:37 2014 (r276441)
@@ -2969,12 +2969,7 @@ nfsvno_advlock(struct vnode *vp, int fty
if (nfsrv_dolocallocks == 0)
goto out;
-
- /* Check for VI_DOOMED here, so that VOP_ADVLOCK() isn't performed. */
- if ((vp->v_iflag & VI_DOOMED) != 0) {
- error = EPERM;
- goto out;
- }
+ ASSERT_VOP_UNLOCKED(vp, "nfsvno_advlock: vp locked");
fl.l_whence = SEEK_SET;
fl.l_type = ftype;
@@ -2998,14 +2993,12 @@ nfsvno_advlock(struct vnode *vp, int fty
fl.l_pid = (pid_t)0;
fl.l_sysid = (int)nfsv4_sysid;
- NFSVOPUNLOCK(vp, 0);
if (ftype == F_UNLCK)
error = VOP_ADVLOCK(vp, (caddr_t)td->td_proc, F_UNLCK, &fl,
(F_POSIX | F_REMOTE));
else
error = VOP_ADVLOCK(vp, (caddr_t)td->td_proc, F_SETLK, &fl,
(F_POSIX | F_REMOTE));
- NFSVOPLOCK(vp, LK_EXCLUSIVE | LK_RETRY);
out:
NFSEXITCODE(error);
Modified: stable/9/sys/fs/nfsserver/nfs_nfsdstate.c
==============================================================================
--- stable/9/sys/fs/nfsserver/nfs_nfsdstate.c Wed Dec 31 01:29:44 2014 (r276440)
+++ stable/9/sys/fs/nfsserver/nfs_nfsdstate.c Wed Dec 31 01:34:37 2014 (r276441)
@@ -1168,6 +1168,8 @@ nfsrv_freeallnfslocks(struct nfsstate *s
vnode_t tvp = NULL;
uint64_t first, end;
+ if (vp != NULL)
+ ASSERT_VOP_UNLOCKED(vp, "nfsrv_freeallnfslocks: vnode locked");
lop = LIST_FIRST(&stp->ls_lock);
while (lop != LIST_END(&stp->ls_lock)) {
nlop = LIST_NEXT(lop, lo_lckowner);
@@ -1187,9 +1189,10 @@ nfsrv_freeallnfslocks(struct nfsstate *s
if (gottvp == 0) {
if (nfsrv_dolocallocks == 0)
tvp = NULL;
- else if (vp == NULL && cansleep != 0)
+ else if (vp == NULL && cansleep != 0) {
tvp = nfsvno_getvp(&lfp->lf_fh);
- else
+ NFSVOPUNLOCK(tvp, 0);
+ } else
tvp = vp;
gottvp = 1;
}
@@ -1210,7 +1213,7 @@ nfsrv_freeallnfslocks(struct nfsstate *s
lop = nlop;
}
if (vp == NULL && tvp != NULL)
- vput(tvp);
+ vrele(tvp);
}
/*
@@ -1321,7 +1324,7 @@ nfsrv_lockctrl(vnode_t vp, struct nfssta
struct nfsclient *clp = NULL;
u_int32_t bits;
int error = 0, haslock = 0, ret, reterr;
- int getlckret, delegation = 0, filestruct_locked;
+ int getlckret, delegation = 0, filestruct_locked, vnode_unlocked = 0;
fhandle_t nfh;
uint64_t first, end;
uint32_t lock_flags;
@@ -1411,6 +1414,11 @@ tryagain:
* locking rolled back.
*/
NFSUNLOCKSTATE();
+ if (vnode_unlocked == 0) {
+ ASSERT_VOP_ELOCKED(vp, "nfsrv_lockctrl1");
+ vnode_unlocked = 1;
+ NFSVOPUNLOCK(vp, 0);
+ }
reterr = nfsrv_locallock(vp, lfp,
(new_lop->lo_flags & (NFSLCK_READ | NFSLCK_WRITE)),
new_lop->lo_first, new_lop->lo_end, cfp, p);
@@ -1569,6 +1577,11 @@ tryagain:
if (filestruct_locked != 0) {
/* Roll back local locks. */
NFSUNLOCKSTATE();
+ if (vnode_unlocked == 0) {
+ ASSERT_VOP_ELOCKED(vp, "nfsrv_lockctrl2");
+ vnode_unlocked = 1;
+ NFSVOPUNLOCK(vp, 0);
+ }
nfsrv_locallock_rollback(vp, lfp, p);
NFSLOCKSTATE();
nfsrv_unlocklf(lfp);
@@ -1646,6 +1659,12 @@ tryagain:
if (filestruct_locked != 0) {
/* Roll back local locks. */
NFSUNLOCKSTATE();
+ if (vnode_unlocked == 0) {
+ ASSERT_VOP_ELOCKED(vp,
+ "nfsrv_lockctrl3");
+ vnode_unlocked = 1;
+ NFSVOPUNLOCK(vp, 0);
+ }
nfsrv_locallock_rollback(vp, lfp, p);
NFSLOCKSTATE();
nfsrv_unlocklf(lfp);
@@ -1665,6 +1684,8 @@ tryagain:
bits = tstp->ls_flags;
bits >>= NFSLCK_SHIFT;
if (new_stp->ls_flags & bits & NFSLCK_ACCESSBITS) {
+ KASSERT(vnode_unlocked == 0,
+ ("nfsrv_lockctrl: vnode unlocked1"));
ret = nfsrv_clientconflict(tstp->ls_clp, &haslock,
vp, p);
if (ret == 1) {
@@ -1696,6 +1717,8 @@ tryagain:
* For setattr, just get rid of all the Delegations for other clients.
*/
if (new_stp->ls_flags & NFSLCK_SETATTR) {
+ KASSERT(vnode_unlocked == 0,
+ ("nfsrv_lockctrl: vnode unlocked2"));
ret = nfsrv_cleandeleg(vp, lfp, clp, &haslock, p);
if (ret) {
/*
@@ -1746,14 +1769,26 @@ tryagain:
(new_lop->lo_flags & NFSLCK_WRITE) &&
(clp != tstp->ls_clp ||
(tstp->ls_flags & NFSLCK_DELEGREAD)))) {
+ ret = 0;
if (filestruct_locked != 0) {
/* Roll back local locks. */
NFSUNLOCKSTATE();
+ if (vnode_unlocked == 0) {
+ ASSERT_VOP_ELOCKED(vp, "nfsrv_lockctrl4");
+ NFSVOPUNLOCK(vp, 0);
+ }
nfsrv_locallock_rollback(vp, lfp, p);
NFSLOCKSTATE();
nfsrv_unlocklf(lfp);
+ NFSUNLOCKSTATE();
+ NFSVOPLOCK(vp, LK_EXCLUSIVE | LK_RETRY);
+ vnode_unlocked = 0;
+ if ((vp->v_iflag & VI_DOOMED) != 0)
+ ret = NFSERR_SERVERFAULT;
+ NFSLOCKSTATE();
}
- ret = nfsrv_delegconflict(tstp, &haslock, p, vp);
+ if (ret == 0)
+ ret = nfsrv_delegconflict(tstp, &haslock, p, vp);
if (ret) {
/*
* nfsrv_delegconflict unlocks state when it
@@ -1790,6 +1825,11 @@ tryagain:
stateidp->other[2] = stp->ls_stateid.other[2];
if (filestruct_locked != 0) {
NFSUNLOCKSTATE();
+ if (vnode_unlocked == 0) {
+ ASSERT_VOP_ELOCKED(vp, "nfsrv_lockctrl5");
+ vnode_unlocked = 1;
+ NFSVOPUNLOCK(vp, 0);
+ }
/* Update the local locks. */
nfsrv_localunlock(vp, lfp, first, end, p);
NFSLOCKSTATE();
@@ -1820,14 +1860,29 @@ tryagain:
FREE((caddr_t)other_lop, M_NFSDLOCK);
other_lop = NULL;
}
- ret = nfsrv_clientconflict(lop->lo_stp->ls_clp,&haslock,vp,p);
+ if (vnode_unlocked != 0)
+ ret = nfsrv_clientconflict(lop->lo_stp->ls_clp, &haslock,
+ NULL, p);
+ else
+ ret = nfsrv_clientconflict(lop->lo_stp->ls_clp, &haslock,
+ vp, p);
if (ret == 1) {
if (filestruct_locked != 0) {
+ if (vnode_unlocked == 0) {
+ ASSERT_VOP_ELOCKED(vp, "nfsrv_lockctrl6");
+ NFSVOPUNLOCK(vp, 0);
+ }
/* Roll back local locks. */
nfsrv_locallock_rollback(vp, lfp, p);
NFSLOCKSTATE();
nfsrv_unlocklf(lfp);
NFSUNLOCKSTATE();
+ NFSVOPLOCK(vp, LK_EXCLUSIVE | LK_RETRY);
+ vnode_unlocked = 0;
+ if ((vp->v_iflag & VI_DOOMED) != 0) {
+ error = NFSERR_SERVERFAULT;
+ goto out;
+ }
}
/*
* nfsrv_clientconflict() unlocks state when it
@@ -1861,6 +1916,11 @@ tryagain:
if (filestruct_locked != 0 && ret == 0) {
/* Roll back local locks. */
NFSUNLOCKSTATE();
+ if (vnode_unlocked == 0) {
+ ASSERT_VOP_ELOCKED(vp, "nfsrv_lockctrl7");
+ vnode_unlocked = 1;
+ NFSVOPUNLOCK(vp, 0);
+ }
nfsrv_locallock_rollback(vp, lfp, p);
NFSLOCKSTATE();
nfsrv_unlocklf(lfp);
@@ -1937,6 +1997,11 @@ out:
nfsv4_unlock(&nfsv4rootfs_lock, 1);
NFSUNLOCKV4ROOTMUTEX();
}
+ if (vnode_unlocked != 0) {
+ NFSVOPLOCK(vp, LK_EXCLUSIVE | LK_RETRY);
+ if (error == 0 && (vp->v_iflag & VI_DOOMED) != 0)
+ error = NFSERR_SERVERFAULT;
+ }
if (other_lop)
FREE((caddr_t)other_lop, M_NFSDLOCK);
NFSEXITCODE2(error, nd);
@@ -2958,11 +3023,14 @@ nfsrv_openupdate(vnode_t vp, struct nfss
/* Get the lf lock */
nfsrv_locklf(lfp);
NFSUNLOCKSTATE();
+ ASSERT_VOP_ELOCKED(vp, "nfsrv_openupdate");
+ NFSVOPUNLOCK(vp, 0);
if (nfsrv_freeopen(stp, vp, 1, p) == 0) {
NFSLOCKSTATE();
nfsrv_unlocklf(lfp);
NFSUNLOCKSTATE();
}
+ NFSVOPLOCK(vp, LK_EXCLUSIVE | LK_RETRY);
} else {
(void) nfsrv_freeopen(stp, NULL, 0, p);
NFSUNLOCKSTATE();
@@ -4271,7 +4339,7 @@ static int
nfsrv_clientconflict(struct nfsclient *clp, int *haslockp, vnode_t vp,
NFSPROC_T *p)
{
- int gotlock, lktype;
+ int gotlock, lktype = 0;
/*
* If lease hasn't expired, we can't fix it.
@@ -4281,8 +4349,10 @@ nfsrv_clientconflict(struct nfsclient *c
return (0);
if (*haslockp == 0) {
NFSUNLOCKSTATE();
- lktype = NFSVOPISLOCKED(vp);
- NFSVOPUNLOCK(vp, 0);
+ if (vp != NULL) {
+ lktype = NFSVOPISLOCKED(vp);
+ NFSVOPUNLOCK(vp, 0);
+ }
NFSLOCKV4ROOTMUTEX();
nfsv4_relref(&nfsv4rootfs_lock);
do {
@@ -4291,11 +4361,12 @@ nfsrv_clientconflict(struct nfsclient *c
} while (!gotlock);
NFSUNLOCKV4ROOTMUTEX();
*haslockp = 1;
- NFSVOPLOCK(vp, lktype | LK_RETRY);
- if ((vp->v_iflag & VI_DOOMED) != 0)
- return (2);
- else
- return (1);
+ if (vp != NULL) {
+ NFSVOPLOCK(vp, lktype | LK_RETRY);
+ if ((vp->v_iflag & VI_DOOMED) != 0)
+ return (2);
+ }
+ return (1);
}
NFSUNLOCKSTATE();
@@ -4336,7 +4407,7 @@ nfsrv_delegconflict(struct nfsstate *stp
vnode_t vp)
{
struct nfsclient *clp = stp->ls_clp;
- int gotlock, error, lktype, retrycnt, zapped_clp;
+ int gotlock, error, lktype = 0, retrycnt, zapped_clp;
nfsv4stateid_t tstateid;
fhandle_t tfh;
@@ -4453,8 +4524,10 @@ nfsrv_delegconflict(struct nfsstate *stp
*/
if (*haslockp == 0) {
NFSUNLOCKSTATE();
- lktype = NFSVOPISLOCKED(vp);
- NFSVOPUNLOCK(vp, 0);
+ if (vp != NULL) {
+ lktype = NFSVOPISLOCKED(vp);
+ NFSVOPUNLOCK(vp, 0);
+ }
NFSLOCKV4ROOTMUTEX();
nfsv4_relref(&nfsv4rootfs_lock);
do {
@@ -4463,14 +4536,16 @@ nfsrv_delegconflict(struct nfsstate *stp
} while (!gotlock);
NFSUNLOCKV4ROOTMUTEX();
*haslockp = 1;
- NFSVOPLOCK(vp, lktype | LK_RETRY);
- if ((vp->v_iflag & VI_DOOMED) != 0) {
- *haslockp = 0;
- NFSLOCKV4ROOTMUTEX();
- nfsv4_unlock(&nfsv4rootfs_lock, 1);
- NFSUNLOCKV4ROOTMUTEX();
- error = NFSERR_PERM;
- goto out;
+ if (vp != NULL) {
+ NFSVOPLOCK(vp, lktype | LK_RETRY);
+ if ((vp->v_iflag & VI_DOOMED) != 0) {
+ *haslockp = 0;
+ NFSLOCKV4ROOTMUTEX();
+ nfsv4_unlock(&nfsv4rootfs_lock, 1);
+ NFSUNLOCKV4ROOTMUTEX();
+ error = NFSERR_PERM;
+ goto out;
+ }
}
error = -1;
goto out;
More information about the svn-src-stable-9
mailing list