From owner-svn-src-all@FreeBSD.ORG Fri May 27 22:05:10 2011 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id C6BC7106564A; Fri, 27 May 2011 22:05:10 +0000 (UTC) (envelope-from rmacklem@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id B5B2D8FC14; Fri, 27 May 2011 22:05:10 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.4/8.14.4) with ESMTP id p4RM5AMG054903; Fri, 27 May 2011 22:05:10 GMT (envelope-from rmacklem@svn.freebsd.org) Received: (from rmacklem@localhost) by svn.freebsd.org (8.14.4/8.14.4/Submit) id p4RM5Aih054896; Fri, 27 May 2011 22:05:10 GMT (envelope-from rmacklem@svn.freebsd.org) Message-Id: <201105272205.p4RM5Aih054896@svn.freebsd.org> From: Rick Macklem Date: Fri, 27 May 2011 22:05:10 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r222389 - in head/sys/fs: nfs nfsclient nfsserver X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 27 May 2011 22:05:10 -0000 Author: rmacklem Date: Fri May 27 22:05:10 2011 New Revision: 222389 URL: http://svn.freebsd.org/changeset/base/222389 Log: Fix the new NFS client so that it handles NFSv4 state correctly during a forced dismount. This required that the exclusive and shared (refcnt) sleep lock functions check for MNTK_UMOUNTF before sleeping, so that they won't block while nfscl_umount() is getting rid of the state. As such, a "struct mount *" argument was added to the locking functions. I believe the only remaining case where a forced dismount can get hung in the kernel is when a thread is already attempting to do a TCP connect to a dead server when the krpc client structure called nr_client is NULL. This will only happen just after a "mount -u" with options that force a new TCP connection is done, so it shouldn't be a problem in practice. MFC after: 2 weeks Modified: head/sys/fs/nfs/nfs_commonsubs.c head/sys/fs/nfs/nfs_var.h head/sys/fs/nfsclient/nfs_clcomsubs.c head/sys/fs/nfsclient/nfs_clstate.c head/sys/fs/nfsserver/nfs_nfsdsocket.c head/sys/fs/nfsserver/nfs_nfsdstate.c Modified: head/sys/fs/nfs/nfs_commonsubs.c ============================================================================== --- head/sys/fs/nfs/nfs_commonsubs.c Fri May 27 21:45:21 2011 (r222388) +++ head/sys/fs/nfs/nfs_commonsubs.c Fri May 27 22:05:10 2011 (r222389) @@ -1726,11 +1726,13 @@ nfsmout: * Any usecnt must be decremented by calling nfsv4_relref() before * calling nfsv4_lock(). It was done this way, so nfsv4_lock() could * be called in a loop. - * The last argument is set to indicate if the call slept, iff not NULL. + * The isleptp argument is set to indicate if the call slept, iff not NULL + * and the mp argument indicates to check for a forced dismount, iff not + * NULL. */ APPLESTATIC int nfsv4_lock(struct nfsv4lock *lp, int iwantlock, int *isleptp, - void *mutex) + void *mutex, struct mount *mp) { if (isleptp) @@ -1751,6 +1753,10 @@ nfsv4_lock(struct nfsv4lock *lp, int iwa lp->nfslock_lock |= NFSV4LOCK_LOCKWANTED; } while (lp->nfslock_lock & (NFSV4LOCK_LOCK | NFSV4LOCK_LOCKWANTED)) { + if (mp != NULL && (mp->mnt_kern_flag & MNTK_UNMOUNTF) != 0) { + lp->nfslock_lock &= ~NFSV4LOCK_LOCKWANTED; + return (0); + } lp->nfslock_lock |= NFSV4LOCK_WANTED; if (isleptp) *isleptp = 1; @@ -1801,9 +1807,12 @@ nfsv4_relref(struct nfsv4lock *lp) * not wait for threads that want the exclusive lock. If priority needs * to be given to threads that need the exclusive lock, a call to nfsv4_lock() * with the 2nd argument == 0 should be done before calling nfsv4_getref(). + * If the mp argument is not NULL, check for MNTK_UNMOUNTF being set and + * return without getting a refcnt for that case. */ APPLESTATIC void -nfsv4_getref(struct nfsv4lock *lp, int *isleptp, void *mutex) +nfsv4_getref(struct nfsv4lock *lp, int *isleptp, void *mutex, + struct mount *mp) { if (isleptp) @@ -1813,12 +1822,16 @@ nfsv4_getref(struct nfsv4lock *lp, int * * Wait for a lock held. */ while (lp->nfslock_lock & NFSV4LOCK_LOCK) { + if (mp != NULL && (mp->mnt_kern_flag & MNTK_UNMOUNTF) != 0) + return; lp->nfslock_lock |= NFSV4LOCK_WANTED; if (isleptp) *isleptp = 1; (void) nfsmsleep(&lp->nfslock_lock, mutex, PZERO - 1, "nfsv4lck", NULL); } + if (mp != NULL && (mp->mnt_kern_flag & MNTK_UNMOUNTF) != 0) + return; lp->nfslock_usecnt++; } Modified: head/sys/fs/nfs/nfs_var.h ============================================================================== --- head/sys/fs/nfs/nfs_var.h Fri May 27 21:45:21 2011 (r222388) +++ head/sys/fs/nfs/nfs_var.h Fri May 27 22:05:10 2011 (r222389) @@ -247,10 +247,10 @@ int nfsv4_loadattr(struct nfsrv_descript struct nfsv3_pathconf *, struct statfs *, struct nfsstatfs *, struct nfsfsinfo *, NFSACL_T *, int, int *, u_int32_t *, u_int32_t *, NFSPROC_T *, struct ucred *); -int nfsv4_lock(struct nfsv4lock *, int, int *, void *); +int nfsv4_lock(struct nfsv4lock *, int, int *, void *, struct mount *); void nfsv4_unlock(struct nfsv4lock *, int); void nfsv4_relref(struct nfsv4lock *); -void nfsv4_getref(struct nfsv4lock *, int *, void *); +void nfsv4_getref(struct nfsv4lock *, int *, void *, struct mount *); int nfsv4_getref_nonblock(struct nfsv4lock *); int nfsv4_testlock(struct nfsv4lock *); int nfsrv_mtostr(struct nfsrv_descript *, char *, int); Modified: head/sys/fs/nfsclient/nfs_clcomsubs.c ============================================================================== --- head/sys/fs/nfsclient/nfs_clcomsubs.c Fri May 27 21:45:21 2011 (r222388) +++ head/sys/fs/nfsclient/nfs_clcomsubs.c Fri May 27 22:05:10 2011 (r222389) @@ -482,7 +482,7 @@ nfscl_lockexcl(struct nfsv4lock *lckp, v int igotlock; do { - igotlock = nfsv4_lock(lckp, 1, NULL, mutex); + igotlock = nfsv4_lock(lckp, 1, NULL, mutex, NULL); } while (!igotlock); } Modified: head/sys/fs/nfsclient/nfs_clstate.c ============================================================================== --- head/sys/fs/nfsclient/nfs_clstate.c Fri May 27 21:45:21 2011 (r222388) +++ head/sys/fs/nfsclient/nfs_clstate.c Fri May 27 22:05:10 2011 (r222389) @@ -687,11 +687,14 @@ nfscl_getcl(vnode_t vp, struct ucred *cr struct nfsclclient *clp; struct nfsclclient *newclp = NULL; struct nfscllockowner *lp, *nlp; - struct nfsmount *nmp = VFSTONFS(vnode_mount(vp)); + struct mount *mp; + struct nfsmount *nmp; char uuid[HOSTUUIDLEN]; int igotlock = 0, error, trystalecnt, clidinusedelay, i; u_int16_t idlen = 0; + mp = vnode_mount(vp); + nmp = VFSTONFS(mp); if (cred != NULL) { getcredhostuuid(cred, uuid, sizeof uuid); idlen = strlen(uuid); @@ -704,6 +707,17 @@ nfscl_getcl(vnode_t vp, struct ucred *cr M_WAITOK); } NFSLOCKCLSTATE(); + /* + * If a forced dismount is already in progress, don't + * allocate a new clientid and get out now. For the case where + * clp != NULL, this is a harmless optimization. + */ + if ((mp->mnt_kern_flag & MNTK_UNMOUNTF) != 0) { + NFSUNLOCKCLSTATE(); + if (newclp != NULL) + free(newclp, M_NFSCLCLIENT); + return (EBADF); + } clp = nmp->nm_clp; if (clp == NULL) { if (newclp == NULL) { @@ -736,9 +750,21 @@ nfscl_getcl(vnode_t vp, struct ucred *cr NFSLOCKCLSTATE(); while ((clp->nfsc_flags & NFSCLFLAGS_HASCLIENTID) == 0 && !igotlock) igotlock = nfsv4_lock(&clp->nfsc_lock, 1, NULL, - NFSCLSTATEMUTEXPTR); + NFSCLSTATEMUTEXPTR, mp); if (!igotlock) - nfsv4_getref(&clp->nfsc_lock, NULL, NFSCLSTATEMUTEXPTR); + nfsv4_getref(&clp->nfsc_lock, NULL, NFSCLSTATEMUTEXPTR, mp); + if (igotlock == 0 && (mp->mnt_kern_flag & MNTK_UNMOUNTF) != 0) { + /* + * Both nfsv4_lock() and nfsv4_getref() know to check + * for MNTK_UNMOUNTF and return without sleeping to + * wait for the exclusive lock to be released, since it + * might be held by nfscl_umount() and we need to get out + * now for that case and not wait until nfscl_umount() + * releases it. + */ + NFSUNLOCKCLSTATE(); + return (EBADF); + } NFSUNLOCKCLSTATE(); /* @@ -1713,6 +1739,7 @@ nfscl_cleanupkext(struct nfsclclient *cl } #endif /* APPLEKEXT || __FreeBSD__ */ +static int fake_global; /* Used to force visibility of MNTK_UNMOUNTF */ /* * Called from nfs umount to free up the clientid. */ @@ -1723,6 +1750,33 @@ nfscl_umount(struct nfsmount *nmp, NFSPR struct ucred *cred; int igotlock; + /* + * For the case that matters, this is the thread that set + * MNTK_UNMOUNTF, so it will see it set. The code that follows is + * done to ensure that any thread executing nfscl_getcl() after + * this time, will see MNTK_UNMOUNTF set. nfscl_getcl() uses the + * mutex for NFSLOCKCLSTATE(), so it is "m" for the following + * explanation, courtesy of Alan Cox. + * What follows is a snippet from Alan Cox's email at: + * http://docs.FreeBSD.org/cgi/ + * mid.cgi?BANLkTikR3d65zPHo9==08ZfJ2vmqZucEvw + * + * 1. Set MNTK_UNMOUNTF + * 2. Acquire a standard FreeBSD mutex "m". + * 3. Update some data structures. + * 4. Release mutex "m". + * + * Then, other threads that acquire "m" after step 4 has occurred will + * see MNTK_UNMOUNTF as set. But, other threads that beat thread X to + * step 2 may or may not see MNTK_UNMOUNTF as set. + */ + NFSLOCKCLSTATE(); + if ((nmp->nm_mountp->mnt_kern_flag & MNTK_UNMOUNTF) != 0) { + fake_global++; + NFSUNLOCKCLSTATE(); + NFSLOCKCLSTATE(); + } + clp = nmp->nm_clp; if (clp != NULL) { if ((clp->nfsc_flags & NFSCLFLAGS_INITED) == 0) @@ -1734,12 +1788,16 @@ nfscl_umount(struct nfsmount *nmp, NFSPR */ clp->nfsc_flags |= NFSCLFLAGS_UMOUNT; while (clp->nfsc_flags & NFSCLFLAGS_HASTHREAD) - (void) tsleep((caddr_t)clp, PWAIT, "nfsclumnt", hz); + (void)mtx_sleep(clp, NFSCLSTATEMUTEXPTR, PWAIT, + "nfsclumnt", hz); - NFSLOCKCLSTATE(); + /* + * Now, get the exclusive lock on the client state, so + * that no uses of the state are still in progress. + */ do { igotlock = nfsv4_lock(&clp->nfsc_lock, 1, NULL, - NFSCLSTATEMUTEXPTR); + NFSCLSTATEMUTEXPTR, NULL); } while (!igotlock); NFSUNLOCKCLSTATE(); @@ -1756,8 +1814,8 @@ nfscl_umount(struct nfsmount *nmp, NFSPR nmp->nm_clp = NULL; NFSFREECRED(cred); FREE((caddr_t)clp, M_NFSCLCLIENT); - } - + } else + NFSUNLOCKCLSTATE(); } /* @@ -1790,7 +1848,7 @@ nfscl_recover(struct nfsclclient *clp, s clp->nfsc_flags |= NFSCLFLAGS_RECVRINPROG; do { igotlock = nfsv4_lock(&clp->nfsc_lock, 1, NULL, - NFSCLSTATEMUTEXPTR); + NFSCLSTATEMUTEXPTR, NULL); } while (!igotlock); NFSUNLOCKCLSTATE(); @@ -2105,7 +2163,7 @@ nfscl_hasexpired(struct nfsclclient *clp clp->nfsc_flags |= NFSCLFLAGS_EXPIREIT; do { igotlock = nfsv4_lock(&clp->nfsc_lock, 1, NULL, - NFSCLSTATEMUTEXPTR); + NFSCLSTATEMUTEXPTR, NULL); } while (!igotlock && (clp->nfsc_flags & NFSCLFLAGS_EXPIREIT)); if ((clp->nfsc_flags & NFSCLFLAGS_EXPIREIT) == 0) { if (igotlock) @@ -2464,7 +2522,7 @@ tryagain: } while (!igotlock) { igotlock = nfsv4_lock(&clp->nfsc_lock, 1, - &islept, NFSCLSTATEMUTEXPTR); + &islept, NFSCLSTATEMUTEXPTR, NULL); if (islept) goto tryagain; } @@ -2556,14 +2614,18 @@ tryagain: } #endif /* APPLEKEXT || __FreeBSD__ */ + NFSLOCKCLSTATE(); if ((clp->nfsc_flags & NFSCLFLAGS_RECOVER) == 0) - (void) tsleep((caddr_t)clp, PWAIT, "nfscl", hz); + (void)mtx_sleep(clp, NFSCLSTATEMUTEXPTR, PWAIT, "nfscl", + hz); if (clp->nfsc_flags & NFSCLFLAGS_UMOUNT) { - NFSFREECRED(cred); clp->nfsc_flags &= ~NFSCLFLAGS_HASTHREAD; + NFSUNLOCKCLSTATE(); + NFSFREECRED(cred); wakeup((caddr_t)clp); return; } + NFSUNLOCKCLSTATE(); } } @@ -3864,7 +3926,7 @@ nfscl_removedeleg(vnode_t vp, NFSPROC_T islept = 0; while (!igotlock) { igotlock = nfsv4_lock(&clp->nfsc_lock, 1, - &islept, NFSCLSTATEMUTEXPTR); + &islept, NFSCLSTATEMUTEXPTR, NULL); if (islept) break; } @@ -3963,7 +4025,7 @@ nfscl_renamedeleg(vnode_t fvp, nfsv4stat islept = 0; while (!igotlock) { igotlock = nfsv4_lock(&clp->nfsc_lock, 1, - &islept, NFSCLSTATEMUTEXPTR); + &islept, NFSCLSTATEMUTEXPTR, NULL); if (islept) break; } @@ -4043,7 +4105,7 @@ nfscl_getref(struct nfsmount *nmp) NFSUNLOCKCLSTATE(); return (0); } - nfsv4_getref(&clp->nfsc_lock, NULL, NFSCLSTATEMUTEXPTR); + nfsv4_getref(&clp->nfsc_lock, NULL, NFSCLSTATEMUTEXPTR, NULL); NFSUNLOCKCLSTATE(); return (1); } Modified: head/sys/fs/nfsserver/nfs_nfsdsocket.c ============================================================================== --- head/sys/fs/nfsserver/nfs_nfsdsocket.c Fri May 27 21:45:21 2011 (r222388) +++ head/sys/fs/nfsserver/nfs_nfsdsocket.c Fri May 27 22:05:10 2011 (r222389) @@ -525,10 +525,10 @@ nfsrvd_compound(struct nfsrv_descript *n NFSLOCKV4ROOTMUTEX(); if (nfsrv_stablefirst.nsf_flags & NFSNSF_NEEDLOCK) igotlock = nfsv4_lock(&nfsv4rootfs_lock, 1, NULL, - NFSV4ROOTLOCKMUTEXPTR); + NFSV4ROOTLOCKMUTEXPTR, NULL); else igotlock = nfsv4_lock(&nfsv4rootfs_lock, 0, NULL, - NFSV4ROOTLOCKMUTEXPTR); + NFSV4ROOTLOCKMUTEXPTR, NULL); NFSUNLOCKV4ROOTMUTEX(); if (igotlock) { /* @@ -576,7 +576,7 @@ nfsrvd_compound(struct nfsrv_descript *n */ NFSLOCKV4ROOTMUTEX(); nfsv4_getref(&nfsv4rootfs_lock, NULL, - NFSV4ROOTLOCKMUTEXPTR); + NFSV4ROOTLOCKMUTEXPTR, NULL); NFSUNLOCKV4ROOTMUTEX(); } Modified: head/sys/fs/nfsserver/nfs_nfsdstate.c ============================================================================== --- head/sys/fs/nfsserver/nfs_nfsdstate.c Fri May 27 21:45:21 2011 (r222388) +++ head/sys/fs/nfsserver/nfs_nfsdstate.c Fri May 27 22:05:10 2011 (r222389) @@ -169,7 +169,7 @@ nfsrv_setclient(struct nfsrv_descript *n nfsv4_relref(&nfsv4rootfs_lock); do { igotlock = nfsv4_lock(&nfsv4rootfs_lock, 1, NULL, - NFSV4ROOTLOCKMUTEXPTR); + NFSV4ROOTLOCKMUTEXPTR, NULL); } while (!igotlock); NFSUNLOCKV4ROOTMUTEX(); @@ -419,7 +419,7 @@ nfsrv_getclient(nfsquad_t clientid, int nfsv4_relref(&nfsv4rootfs_lock); do { igotlock = nfsv4_lock(&nfsv4rootfs_lock, 1, NULL, - NFSV4ROOTLOCKMUTEXPTR); + NFSV4ROOTLOCKMUTEXPTR, NULL); } while (!igotlock); NFSUNLOCKV4ROOTMUTEX(); } else if (opflags != CLOPS_RENEW) { @@ -548,7 +548,7 @@ nfsrv_adminrevoke(struct nfsd_clid *revo NFSLOCKV4ROOTMUTEX(); do { igotlock = nfsv4_lock(&nfsv4rootfs_lock, 1, NULL, - NFSV4ROOTLOCKMUTEXPTR); + NFSV4ROOTLOCKMUTEXPTR, NULL); } while (!igotlock); NFSUNLOCKV4ROOTMUTEX(); @@ -608,7 +608,7 @@ nfsrv_dumpclients(struct nfsd_dumpclient * exclusive lock cannot be acquired while dumping the clients. */ NFSLOCKV4ROOTMUTEX(); - nfsv4_getref(&nfsv4rootfs_lock, NULL, NFSV4ROOTLOCKMUTEXPTR); + nfsv4_getref(&nfsv4rootfs_lock, NULL, NFSV4ROOTLOCKMUTEXPTR, NULL); NFSUNLOCKV4ROOTMUTEX(); NFSLOCKSTATE(); /* @@ -709,7 +709,7 @@ nfsrv_dumplocks(vnode_t vp, struct nfsd_ * exclusive lock on it cannot be acquired while dumping the locks. */ NFSLOCKV4ROOTMUTEX(); - nfsv4_getref(&nfsv4rootfs_lock, NULL, NFSV4ROOTLOCKMUTEXPTR); + nfsv4_getref(&nfsv4rootfs_lock, NULL, NFSV4ROOTLOCKMUTEXPTR, NULL); NFSUNLOCKV4ROOTMUTEX(); NFSLOCKSTATE(); if (!ret) @@ -4254,7 +4254,7 @@ nfsrv_clientconflict(struct nfsclient *c nfsv4_relref(&nfsv4rootfs_lock); do { gotlock = nfsv4_lock(&nfsv4rootfs_lock, 1, NULL, - NFSV4ROOTLOCKMUTEXPTR); + NFSV4ROOTLOCKMUTEXPTR, NULL); } while (!gotlock); NFSUNLOCKV4ROOTMUTEX(); *haslockp = 1; @@ -4422,7 +4422,7 @@ nfsrv_delegconflict(struct nfsstate *stp nfsv4_relref(&nfsv4rootfs_lock); do { gotlock = nfsv4_lock(&nfsv4rootfs_lock, 1, NULL, - NFSV4ROOTLOCKMUTEXPTR); + NFSV4ROOTLOCKMUTEXPTR, NULL); } while (!gotlock); NFSUNLOCKV4ROOTMUTEX(); *haslockp = 1; @@ -4616,7 +4616,7 @@ nfsd_recalldelegation(vnode_t vp, NFSPRO * exclusive lock cannot be acquired by another thread. */ NFSLOCKV4ROOTMUTEX(); - nfsv4_getref(&nfsv4rootfs_lock, NULL, NFSV4ROOTLOCKMUTEXPTR); + nfsv4_getref(&nfsv4rootfs_lock, NULL, NFSV4ROOTLOCKMUTEXPTR, NULL); NFSUNLOCKV4ROOTMUTEX(); /* @@ -5179,7 +5179,7 @@ nfsrv_locklf(struct nfslockfile *lfp) lfp->lf_usecount++; do { gotlock = nfsv4_lock(&lfp->lf_locallock_lck, 1, NULL, - NFSSTATEMUTEXPTR); + NFSSTATEMUTEXPTR, NULL); } while (gotlock == 0); lfp->lf_usecount--; }