From owner-svn-src-all@FreeBSD.ORG Sat Sep 11 01:08:16 2010 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 86ADE106566B; Sat, 11 Sep 2010 01:08:16 +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 74DF98FC16; Sat, 11 Sep 2010 01:08:16 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id o8B18GdF008423; Sat, 11 Sep 2010 01:08:16 GMT (envelope-from rmacklem@svn.freebsd.org) Received: (from rmacklem@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id o8B18Ghx008418; Sat, 11 Sep 2010 01:08:16 GMT (envelope-from rmacklem@svn.freebsd.org) Message-Id: <201009110108.o8B18Ghx008418@svn.freebsd.org> From: Rick Macklem Date: Sat, 11 Sep 2010 01:08:16 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org X-SVN-Group: stable-8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r212446 - in stable/8/sys/fs: nfs 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: Sat, 11 Sep 2010 01:08:16 -0000 Author: rmacklem Date: Sat Sep 11 01:08:16 2010 New Revision: 212446 URL: http://svn.freebsd.org/changeset/base/212446 Log: MFC: r211951 The timer routine in the experimental NFS server did not acquire the correct mutex when checking nfsv4root_lock. Although this could be fixed by adding mutex lock/unlock calls, zack.kirsch at isilon.com suggested a better fix that uses a non-blocking acquisition of a reference count on nfsv4root_lock. This fix allows the weird NFSLOCKSTATE(); NFSUNLOCKSTATE(); synchronization to be deleted. This patch applies this fix. Modified: stable/8/sys/fs/nfs/nfs_commonsubs.c stable/8/sys/fs/nfs/nfs_var.h stable/8/sys/fs/nfsserver/nfs_nfsdsocket.c stable/8/sys/fs/nfsserver/nfs_nfsdstate.c Directory Properties: stable/8/sys/ (props changed) stable/8/sys/amd64/include/xen/ (props changed) stable/8/sys/cddl/contrib/opensolaris/ (props changed) stable/8/sys/contrib/dev/acpica/ (props changed) stable/8/sys/contrib/pf/ (props changed) stable/8/sys/dev/xen/xenpci/ (props changed) Modified: stable/8/sys/fs/nfs/nfs_commonsubs.c ============================================================================== --- stable/8/sys/fs/nfs/nfs_commonsubs.c Fri Sep 10 23:55:46 2010 (r212445) +++ stable/8/sys/fs/nfs/nfs_commonsubs.c Sat Sep 11 01:08:16 2010 (r212446) @@ -1835,6 +1835,21 @@ nfsv4_getref(struct nfsv4lock *lp, int * } /* + * Get a reference as above, but return failure instead of sleeping if + * an exclusive lock is held. + */ +APPLESTATIC int +nfsv4_getref_nonblock(struct nfsv4lock *lp) +{ + + if ((lp->nfslock_lock & NFSV4LOCK_LOCK) != 0) + return (0); + + lp->nfslock_usecnt++; + return (1); +} + +/* * Test for a lock. Return 1 if locked, 0 otherwise. */ APPLESTATIC int Modified: stable/8/sys/fs/nfs/nfs_var.h ============================================================================== --- stable/8/sys/fs/nfs/nfs_var.h Fri Sep 10 23:55:46 2010 (r212445) +++ stable/8/sys/fs/nfs/nfs_var.h Sat Sep 11 01:08:16 2010 (r212446) @@ -251,6 +251,7 @@ int nfsv4_lock(struct nfsv4lock *, int, void nfsv4_unlock(struct nfsv4lock *, int); void nfsv4_relref(struct nfsv4lock *); void nfsv4_getref(struct nfsv4lock *, int *, void *); +int nfsv4_getref_nonblock(struct nfsv4lock *); int nfsv4_testlock(struct nfsv4lock *); int nfsrv_mtostr(struct nfsrv_descript *, char *, int); int nfsrv_checkutf8(u_int8_t *, int); Modified: stable/8/sys/fs/nfsserver/nfs_nfsdsocket.c ============================================================================== --- stable/8/sys/fs/nfsserver/nfs_nfsdsocket.c Fri Sep 10 23:55:46 2010 (r212445) +++ stable/8/sys/fs/nfsserver/nfs_nfsdsocket.c Sat Sep 11 01:08:16 2010 (r212446) @@ -533,8 +533,6 @@ nfsrvd_compound(struct nfsrv_descript *n NFSV4ROOTLOCKMUTEXPTR); NFSUNLOCKV4ROOTMUTEX(); if (igotlock) { - NFSLOCKSTATE(); /* to avoid a race with */ - NFSUNLOCKSTATE(); /* nfsrv_servertimer() */ /* * If I got the lock, I can update the stable storage file. * Done when the grace period is over or a client has long Modified: stable/8/sys/fs/nfsserver/nfs_nfsdstate.c ============================================================================== --- stable/8/sys/fs/nfsserver/nfs_nfsdstate.c Fri Sep 10 23:55:46 2010 (r212445) +++ stable/8/sys/fs/nfsserver/nfs_nfsdstate.c Sat Sep 11 01:08:16 2010 (r212446) @@ -164,8 +164,6 @@ nfsrv_setclient(struct nfsrv_descript *n NFSV4ROOTLOCKMUTEXPTR); } while (!igotlock); NFSUNLOCKV4ROOTMUTEX(); - NFSLOCKSTATE(); /* to avoid a race with */ - NFSUNLOCKSTATE(); /* nfsrv_servertimer() */ /* * Search for a match in the client list. @@ -416,8 +414,6 @@ nfsrv_getclient(nfsquad_t clientid, int NFSV4ROOTLOCKMUTEXPTR); } while (!igotlock); NFSUNLOCKV4ROOTMUTEX(); - NFSLOCKSTATE(); /* to avoid a race with */ - NFSUNLOCKSTATE(); /* nfsrv_servertimer() */ } else if (opflags != CLOPS_RENEW) { NFSLOCKSTATE(); } @@ -547,8 +543,6 @@ nfsrv_adminrevoke(struct nfsd_clid *revo NFSV4ROOTLOCKMUTEXPTR); } while (!igotlock); NFSUNLOCKV4ROOTMUTEX(); - NFSLOCKSTATE(); /* to avoid a race with */ - NFSUNLOCKSTATE(); /* nfsrv_servertimer() */ /* * Search for a match in the client list. @@ -824,11 +818,8 @@ nfsrv_dumplocks(vnode_t vp, struct nfsd_ /* * Server timer routine. It can scan any linked list, so long - * as it holds the spin lock and there is no exclusive lock on + * as it holds the spin/mutex lock and there is no exclusive lock on * nfsv4rootfs_lock. - * Must be called by a kernel thread and not a timer interrupt, - * so that it only runs when the nfsd threads are sleeping on a - * uniprocessor and uses the State spin lock for an SMP system. * (For OpenBSD, a kthread is ok. For FreeBSD, I think it is ok * to do this from a callout, since the spin locks work. For * Darwin, I'm not sure what will work correctly yet.) @@ -839,7 +830,7 @@ nfsrv_servertimer(void) { struct nfsclient *clp, *nclp; struct nfsstate *stp, *nstp; - int i; + int got_ref, i; /* * Make sure nfsboottime is set. This is used by V3 as well @@ -867,13 +858,14 @@ nfsrv_servertimer(void) } /* - * Return now if an nfsd thread has the exclusive lock on - * nfsv4rootfs_lock. The dirty trick here is that we have - * the spin lock already and the nfsd threads do a: - * NFSLOCKSTATE, NFSUNLOCKSTATE after getting the exclusive - * lock, so they won't race with code after this check. + * Try and get a reference count on the nfsv4rootfs_lock so that + * no nfsd thread can acquire an exclusive lock on it before this + * call is done. If it is already exclusively locked, just return. */ - if (nfsv4rootfs_lock.nfslock_lock & NFSV4LOCK_LOCK) { + NFSLOCKV4ROOTMUTEX(); + got_ref = nfsv4_getref_nonblock(&nfsv4rootfs_lock); + NFSUNLOCKV4ROOTMUTEX(); + if (got_ref == 0) { NFSUNLOCKSTATE(); return; } @@ -945,6 +937,9 @@ nfsrv_servertimer(void) } } NFSUNLOCKSTATE(); + NFSLOCKV4ROOTMUTEX(); + nfsv4_relref(&nfsv4rootfs_lock); + NFSUNLOCKV4ROOTMUTEX(); } /* @@ -4224,8 +4219,6 @@ nfsrv_clientconflict(struct nfsclient *c NFSV4ROOTLOCKMUTEXPTR); } while (!gotlock); NFSUNLOCKV4ROOTMUTEX(); - NFSLOCKSTATE(); /* to avoid a race with */ - NFSUNLOCKSTATE(); /* nfsrv_servertimer() */ *haslockp = 1; NFSVOPLOCK(vp, LK_EXCLUSIVE | LK_RETRY, p); return (1); @@ -4390,8 +4383,6 @@ nfsrv_delegconflict(struct nfsstate *stp NFSV4ROOTLOCKMUTEXPTR); } while (!gotlock); NFSUNLOCKV4ROOTMUTEX(); - NFSLOCKSTATE(); /* to avoid a race with */ - NFSUNLOCKSTATE(); /* nfsrv_servertimer() */ *haslockp = 1; NFSVOPLOCK(vp, LK_EXCLUSIVE | LK_RETRY, p); return (-1);