From 4b16aa8469a814628eea6f791dad4a9f50ae6413 Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Sat, 27 Nov 2010 22:20:04 -0800 Subject: [PATCH] kernel - Fix NFS stall and likely also x86-64 seg-fault issue * The nfs node hash code was not MPSAFE due to its use of a global hash table. This could lead to a stall condition due to the global hash table losing track of its manual lock. For now use a token to interlock hash table access and replace the manual lock with a lockmgr lock. * This appears to have also fixed the long-standing random seg-fault issue with x86-64, where a buildworld loop would seg-fault every once in a while for no apparent reason. /usr/src on my test box has always been NFS mounted but was never considered a possible contributor to the problem. If NFS loses track of related nfs nodes or races operations on underlying vnodes while expecting their storage to remain stable it is possible for a random vnode to become corrupt. How this could translate to a completely random seg-fault is not entirely understood but I surmise the unrelated cc1's binary mmap could become corrupt. Reported-by: Thomas Nikolajsen (nfs issue) --- sys/vfs/nfs/nfs_node.c | 63 +++++++++++++++++++----------------------- 1 file changed, 29 insertions(+), 34 deletions(-) diff --git a/sys/vfs/nfs/nfs_node.c b/sys/vfs/nfs/nfs_node.c index 999454a194..02f87690b8 100644 --- a/sys/vfs/nfs/nfs_node.c +++ b/sys/vfs/nfs/nfs_node.c @@ -59,6 +59,8 @@ static vm_zone_t nfsnode_zone; static LIST_HEAD(nfsnodehashhead, nfsnode) *nfsnodehashtbl; static u_long nfsnodehash; +static lwkt_token nfsnhash_token = LWKT_TOKEN_MP_INITIALIZER(nfsnhash_token); +static struct lock nfsnhash_lock; #define TRUE 1 #define FALSE 0 @@ -74,6 +76,7 @@ nfs_nhinit(void) { nfsnode_zone = zinit("NFSNODE", sizeof(struct nfsnode), 0, 0, 1); nfsnodehashtbl = hashinit(desiredvnodes, M_NFSHASH, &nfsnodehash); + lockinit(&nfsnhash_lock, "nfsnht", 0, 0); } /* @@ -82,7 +85,6 @@ nfs_nhinit(void) * In all cases, a pointer to a * nfsnode structure is returned. */ -static int nfs_node_hash_lock; int nfs_nget(struct mount *mntp, nfsfh_t *fhp, int fhsize, struct nfsnode **npp) @@ -105,6 +107,8 @@ nfs_nget(struct mount *mntp, nfsfh_t *fhp, int fhsize, struct nfsnode **npp) else lkflags = 0; + lwkt_gettoken(&nfsnhash_token); + retry: nhpp = NFSNOHASH(fnv_32_buf(fhp->fh_bytes, fhsize, FNV1_32_INIT)); loop: @@ -129,20 +133,16 @@ loop: goto loop; } *npp = np; + lwkt_reltoken(&nfsnhash_token); return(0); } + /* * Obtain a lock to prevent a race condition if the getnewvnode() * or MALLOC() below happens to block. */ - if (nfs_node_hash_lock) { - while (nfs_node_hash_lock) { - nfs_node_hash_lock = -1; - tsleep(&nfs_node_hash_lock, 0, "nfsngt", 0); - } + if (lockmgr(&nfsnhash_lock, LK_EXCLUSIVE | LK_SLEEPFAIL)) goto loop; - } - nfs_node_hash_lock = 1; /* * Allocate before getnewvnode since doing so afterward @@ -153,11 +153,10 @@ loop: error = getnewvnode(VT_NFS, mntp, &nvp, 0, 0); if (error) { - if (nfs_node_hash_lock < 0) - wakeup(&nfs_node_hash_lock); - nfs_node_hash_lock = 0; + lockmgr(&nfsnhash_lock, LK_RELEASE); *npp = 0; zfree(nfsnode_zone, np); + lwkt_reltoken(&nfsnhash_token); return (error); } vp = nvp; @@ -173,9 +172,7 @@ loop: bcmp((caddr_t)fhp, (caddr_t)np2->n_fhp, fhsize)) continue; vx_put(vp); - if (nfs_node_hash_lock < 0) - wakeup(&nfs_node_hash_lock); - nfs_node_hash_lock = 0; + lockmgr(&nfsnhash_lock, LK_RELEASE); zfree(nfsnode_zone, np); goto retry; } @@ -193,9 +190,8 @@ loop: * nvp is locked & refd so effectively so is np. */ *npp = np; - if (nfs_node_hash_lock < 0) - wakeup(&nfs_node_hash_lock); - nfs_node_hash_lock = 0; + lockmgr(&nfsnhash_lock, LK_RELEASE); + lwkt_reltoken(&nfsnhash_token); return (0); } @@ -226,6 +222,9 @@ nfs_nget_nonblock(struct mount *mntp, nfsfh_t *fhp, int fhsize, lkflags = 0; vp = NULL; *npp = NULL; + + lwkt_gettoken(&nfsnhash_token); + retry: nhpp = NFSNOHASH(fnv_32_buf(fhp->fh_bytes, fhsize, FNV1_32_INIT)); loop: @@ -248,6 +247,7 @@ loop: goto loop; } *npp = np; + lwkt_reltoken(&nfsnhash_token); return(0); } @@ -264,14 +264,8 @@ loop: * Obtain a lock to prevent a race condition if the getnewvnode() * or MALLOC() below happens to block. */ - if (nfs_node_hash_lock) { - while (nfs_node_hash_lock) { - nfs_node_hash_lock = -1; - tsleep(&nfs_node_hash_lock, 0, "nfsngt", 0); - } + if (lockmgr(&nfsnhash_lock, LK_EXCLUSIVE | LK_SLEEPFAIL)) goto loop; - } - nfs_node_hash_lock = 1; /* * Entry not found, allocate a new entry. @@ -284,10 +278,9 @@ loop: error = getnewvnode(VT_NFS, mntp, &nvp, 0, 0); if (error) { - if (nfs_node_hash_lock < 0) - wakeup(&nfs_node_hash_lock); - nfs_node_hash_lock = 0; + lockmgr(&nfsnhash_lock, LK_RELEASE); zfree(nfsnode_zone, np); + lwkt_reltoken(&nfsnhash_token); return (error); } vp = nvp; @@ -305,9 +298,7 @@ loop: continue; } vx_put(vp); - if (nfs_node_hash_lock < 0) - wakeup(&nfs_node_hash_lock); - nfs_node_hash_lock = 0; + lockmgr(&nfsnhash_lock, LK_RELEASE); zfree(nfsnode_zone, np); goto retry; } @@ -326,10 +317,9 @@ loop: */ *npp = np; error = 0; - if (nfs_node_hash_lock < 0) - wakeup(&nfs_node_hash_lock); - nfs_node_hash_lock = 0; + lockmgr(&nfsnhash_lock, LK_RELEASE); fail: + lwkt_reltoken(&nfsnhash_token); return (error); } @@ -397,16 +387,21 @@ nfs_reclaim(struct vop_reclaim_args *ap) if (prtactive && vp->v_sysref.refcnt > 1) vprint("nfs_reclaim: pushing active", vp); - lwkt_gettoken(&nmp->nm_token); + /* + * Remove from hash table + */ + lwkt_gettoken(&nfsnhash_token); if (np->n_hash.le_prev != NULL) LIST_REMOVE(np, n_hash); + lwkt_reltoken(&nfsnhash_token); /* * Free up any directory cookie structures and * large file handle structures that might be associated with * this nfs node. */ + lwkt_gettoken(&nmp->nm_token); if (vp->v_type == VDIR) { dp = np->n_cookies.lh_first; while (dp) { -- 2.41.0