Author: rmacklem
Date: Wed Dec 31 00:40:10 2014
New Revision: 276437
URL: https://svnweb.freebsd.org/changeset/base/276437

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/10/sys/fs/nfsserver/nfs_nfsdport.c
  stable/10/sys/fs/nfsserver/nfs_nfsdstate.c
Directory Properties:
  stable/10/   (props changed)

Modified: stable/10/sys/fs/nfsserver/nfs_nfsdport.c
==============================================================================
--- stable/10/sys/fs/nfsserver/nfs_nfsdport.c   Wed Dec 31 00:34:37 2014        
(r276436)
+++ stable/10/sys/fs/nfsserver/nfs_nfsdport.c   Wed Dec 31 00:40:10 2014        
(r276437)
@@ -2965,12 +2965,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;
@@ -2994,14 +2989,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/10/sys/fs/nfsserver/nfs_nfsdstate.c
==============================================================================
--- stable/10/sys/fs/nfsserver/nfs_nfsdstate.c  Wed Dec 31 00:34:37 2014        
(r276436)
+++ stable/10/sys/fs/nfsserver/nfs_nfsdstate.c  Wed Dec 31 00:40:10 2014        
(r276437)
@@ -1344,6 +1344,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);
@@ -1363,9 +1365,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;
                }
@@ -1386,7 +1389,7 @@ nfsrv_freeallnfslocks(struct nfsstate *s
                lop = nlop;
        }
        if (vp == NULL && tvp != NULL)
-               vput(tvp);
+               vrele(tvp);
 }
 
 /*
@@ -1497,7 +1500,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;
@@ -1587,6 +1590,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);
@@ -1748,6 +1756,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);
@@ -1825,6 +1838,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);
@@ -1844,6 +1863,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) {
@@ -1875,6 +1896,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) {
                        /*
@@ -1925,14 +1948,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
@@ -1971,6 +2006,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();
@@ -2001,14 +2041,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
@@ -2042,6 +2097,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);
@@ -2120,6 +2180,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);
@@ -3227,11 +3292,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();
@@ -4619,7 +4687,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.
@@ -4629,8 +4697,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 {
@@ -4639,11 +4709,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();
 
@@ -4684,7 +4755,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;
 
@@ -4801,8 +4872,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 {
@@ -4811,14 +4884,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;
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to