changeset 10532:f0c47e0a91ac

6853999 bump mi_srvset_cnt in remove_mi() 6868260 panic in nfs4delegreturn_cleanup_impl() even after fix for 6721281 6874901 race between find_nfs4_server_all() and nfs4_server_rele()
author Pavel Filipensky <Pavel.Filipensky@Sun.COM>
date Tue, 15 Sep 2009 04:26:04 +0100
parents 3dbb42515756
children 2bb744c642f6
files usr/src/uts/common/fs/nfs/nfs4_callback.c usr/src/uts/common/fs/nfs/nfs4_vfsops.c
diffstat 2 files changed, 54 insertions(+), 56 deletions(-) [+]
line wrap: on
line diff
--- a/usr/src/uts/common/fs/nfs/nfs4_callback.c	Tue Sep 15 10:38:49 2009 +0800
+++ b/usr/src/uts/common/fs/nfs/nfs4_callback.c	Tue Sep 15 04:26:04 2009 +0100
@@ -1296,14 +1296,6 @@
 	boolean_t need_rele = B_FALSE;
 
 	/*
-	 * Nothing todo if OPEN_DELEGATE_NONE. Return before calling
-	 * find_nfs4_server_all(), since we might be called from
-	 * nfs4_recov_tread() after nfs4_server_t has been freed.
-	 */
-	if (rp->r_deleg_type == OPEN_DELEGATE_NONE)
-		return;
-
-	/*
 	 * Caller must be holding mi_recovlock in read mode
 	 * to call here.  This is provided by start_op.
 	 * Delegation management requires to grab s_lock
@@ -1312,7 +1304,8 @@
 
 	if (np == NULL) {
 		np = find_nfs4_server_all(mi, 1);
-		ASSERT(np != NULL);
+		if (np == NULL)
+			return;
 		need_rele = B_TRUE;
 	} else {
 		mutex_enter(&np->s_lock);
@@ -1320,6 +1313,14 @@
 
 	mutex_enter(&rp->r_statev4_lock);
 
+	if (rp->r_deleg_type == OPEN_DELEGATE_NONE) {
+		mutex_exit(&rp->r_statev4_lock);
+		mutex_exit(&np->s_lock);
+		if (need_rele)
+			nfs4_server_rele(np);
+		return;
+	}
+
 	/*
 	 * Free the cred originally held when
 	 * the delegation was granted.  Caller must
--- a/usr/src/uts/common/fs/nfs/nfs4_vfsops.c	Tue Sep 15 10:38:49 2009 +0800
+++ b/usr/src/uts/common/fs/nfs/nfs4_vfsops.c	Tue Sep 15 04:26:04 2009 +0100
@@ -3432,6 +3432,7 @@
 	/* Now mark the mntinfo4's links as being removed */
 	mi->mi_clientid_prev = mi->mi_clientid_next = NULL;
 	mi->mi_srv = NULL;
+	mi->mi_srvset_cnt++;
 
 	VFS_RELE(mi->mi_vfsp);
 }
@@ -3476,11 +3477,10 @@
 	}
 
 	(void) nfs_rw_enter_sig(&mi->mi_recovlock, RW_READER, 0);
-	sp = mi->mi_srv;
-	if (sp != NULL) {
-		mutex_enter(&sp->s_lock);
+	if (sp = find_nfs4_server_all(mi, 1)) {
 		nfs4_remove_mi_from_server_nolock(mi, sp);
 		mutex_exit(&sp->s_lock);
+		nfs4_server_rele(sp);
 	}
 	nfs_rw_exit(&mi->mi_recovlock);
 }
@@ -3778,17 +3778,31 @@
 }
 
 /*
- * find_nfs4_server() and find_nfs4_server_all() are used to find
- * a nfs4_server_t to which the mntinfo4_t was already linked.
- * this is in contrast to searching a new nfs4_server_t, by walking
- * the nfs4_server_lst list, needed e.g. when doing a failover.
- *
  * Locks the nfs4_server down if it is found and returns a reference that
  * must eventually be freed.
- *
- * Returns NULL it no match is found.  This means one of two things: either
- * mi is in the process of being mounted, or mi has been unmounted.
- *
+ */
+static nfs4_server_t *
+lookup_nfs4_server(nfs4_server_t *sp, int any_state)
+{
+	nfs4_server_t *np;
+
+	mutex_enter(&nfs4_server_lst_lock);
+	for (np = nfs4_server_lst.forw; np != &nfs4_server_lst; np = np->forw) {
+		mutex_enter(&np->s_lock);
+		if (np == sp && np->s_refcnt > 0 &&
+		    (np->s_thread_exit != NFS4_THREAD_EXIT || any_state)) {
+			mutex_exit(&nfs4_server_lst_lock);
+			np->s_refcnt++;
+			return (np);
+		}
+		mutex_exit(&np->s_lock);
+	}
+	mutex_exit(&nfs4_server_lst_lock);
+
+	return (NULL);
+}
+
+/*
  * The caller should be holding mi->mi_recovlock, and it should continue to
  * hold the lock until done with the returned nfs4_server_t.  Once
  * mi->mi_recovlock is released, there is no guarantee that the returned
@@ -3797,30 +3811,37 @@
 nfs4_server_t *
 find_nfs4_server(mntinfo4_t *mi)
 {
-	return (find_nfs4_server_all(mi, 0));
+	ASSERT(nfs_rw_lock_held(&mi->mi_recovlock, RW_READER) ||
+	    nfs_rw_lock_held(&mi->mi_recovlock, RW_WRITER));
+
+	return (lookup_nfs4_server(mi->mi_srv, 0));
 }
 
 /*
- * Same as above, but takes an "all" parameter which can be
+ * Same as above, but takes an "any_state" parameter which can be
  * set to 1 if the caller wishes to find nfs4_server_t's which
  * have been marked for termination by the exit of the renew
  * thread.  This should only be used by operations which are
  * cleaning up and will not cause an OTW op.
  */
 nfs4_server_t *
-find_nfs4_server_all(mntinfo4_t *mi, int all)
+find_nfs4_server_all(mntinfo4_t *mi, int any_state)
 {
-	nfs4_server_t *np = mi->mi_srv;
-
 	ASSERT(nfs_rw_lock_held(&mi->mi_recovlock, RW_READER) ||
 	    nfs_rw_lock_held(&mi->mi_recovlock, RW_WRITER));
 
-	if (np && (np->s_thread_exit != NFS4_THREAD_EXIT || all)) {
-		mutex_enter(&np->s_lock);
-		np->s_refcnt++;
-		return (np);
-	}
-	return (NULL);
+	return (lookup_nfs4_server(mi->mi_srv, any_state));
+}
+
+/*
+ * Lock sp, but only if it's still active (in the list and hasn't been
+ * flagged as exiting) or 'any_state' is non-zero.
+ * Returns TRUE if sp got locked and adds a reference to sp.
+ */
+bool_t
+nfs4_server_vlock(nfs4_server_t *sp, int any_state)
+{
+	return (lookup_nfs4_server(sp, any_state) != NULL);
 }
 
 /*
@@ -3879,30 +3900,6 @@
 }
 
 /*
- * Lock sp, but only if it's still active (in the list and hasn't been
- * flagged as exiting) or 'all' is non-zero.
- * Returns TRUE if sp got locked and adds a reference to sp.
- */
-bool_t
-nfs4_server_vlock(nfs4_server_t *sp, int all)
-{
-	nfs4_server_t *np;
-
-	mutex_enter(&nfs4_server_lst_lock);
-	for (np = nfs4_server_lst.forw; np != &nfs4_server_lst; np = np->forw) {
-		if (sp == np && (np->s_thread_exit != NFS4_THREAD_EXIT ||
-		    all != 0)) {
-			mutex_enter(&np->s_lock);
-			np->s_refcnt++;
-			mutex_exit(&nfs4_server_lst_lock);
-			return (TRUE);
-		}
-	}
-	mutex_exit(&nfs4_server_lst_lock);
-	return (FALSE);
-}
-
-/*
  * Fork off a thread to free the data structures for a mount.
  */