svn commit: r212446 - in stable/8/sys/fs: nfs nfsserver

Rick Macklem rmacklem at FreeBSD.org
Sat Sep 11 01:08:16 UTC 2010


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);


More information about the svn-src-stable mailing list