Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 10 Feb 2010 16:16:50 +0000 (UTC)
From:      Rick Macklem <rmacklem@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org
Subject:   svn commit: r203756 - stable/8/sys/nfsclient
Message-ID:  <201002101616.o1AGGo4f039911@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: rmacklem
Date: Wed Feb 10 16:16:50 2010
New Revision: 203756
URL: http://svn.freebsd.org/changeset/base/203756

Log:
  MFC: r203072
  Fix a race that can occur when nfs nfsiod threads are being created.
  Without this patch it was possible for a different thread that calls
  nfs_asyncio() to snitch a newly created nfsiod thread that was
  intended for another caller of nfs_asyncio(), because the nfs_iod_mtx
  mutex was unlocked while the new nfsiod thread was created. This patch
  labels the newly created nfsiod, so that it is not taken by another
  caller of nfs_asyncio(). This is believed to fix the problem reported
  on the freebsd-stable email list under the subject:
  FreeBSD NFS client/Linux NFS server issue.
  
  Tested by:	to DOT my DOT trociny AT gmail DOT com
  Reviewed by:	jhb

Modified:
  stable/8/sys/nfsclient/nfs.h
  stable/8/sys/nfsclient/nfs_bio.c
  stable/8/sys/nfsclient/nfs_nfsiod.c
  stable/8/sys/nfsclient/nfs_subs.c
  stable/8/sys/nfsclient/nfs_vnops.c
  stable/8/sys/nfsclient/nfsnode.h
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)
  stable/8/sys/netinet/   (props changed)

Modified: stable/8/sys/nfsclient/nfs.h
==============================================================================
--- stable/8/sys/nfsclient/nfs.h	Wed Feb 10 15:12:36 2010	(r203755)
+++ stable/8/sys/nfsclient/nfs.h	Wed Feb 10 16:16:50 2010	(r203756)
@@ -252,7 +252,7 @@ int	nfs_writerpc(struct vnode *, struct 
 int	nfs_commit(struct vnode *vp, u_quad_t offset, int cnt,
 	    struct ucred *cred, struct thread *td);
 int	nfs_readdirrpc(struct vnode *, struct uio *, struct ucred *);
-int	nfs_nfsiodnew(void);
+int	nfs_nfsiodnew(int);
 int	nfs_asyncio(struct nfsmount *, struct buf *, struct ucred *, struct thread *);
 int	nfs_doio(struct vnode *, struct buf *, struct ucred *, struct thread *);
 void	nfs_doio_directwrite (struct buf *);

Modified: stable/8/sys/nfsclient/nfs_bio.c
==============================================================================
--- stable/8/sys/nfsclient/nfs_bio.c	Wed Feb 10 15:12:36 2010	(r203755)
+++ stable/8/sys/nfsclient/nfs_bio.c	Wed Feb 10 16:16:50 2010	(r203756)
@@ -1377,7 +1377,7 @@ again:
 	 * Find a free iod to process this request.
 	 */
 	for (iod = 0; iod < nfs_numasync; iod++)
-		if (nfs_iodwant[iod]) {
+		if (nfs_iodwant[iod] == NFSIOD_AVAILABLE) {
 			gotiod = TRUE;
 			break;
 		}
@@ -1386,7 +1386,7 @@ again:
 	 * Try to create one if none are free.
 	 */
 	if (!gotiod) {
-		iod = nfs_nfsiodnew();
+		iod = nfs_nfsiodnew(1);
 		if (iod != -1)
 			gotiod = TRUE;
 	}
@@ -1398,7 +1398,7 @@ again:
 		 */
 		NFS_DPF(ASYNCIO, ("nfs_asyncio: waking iod %d for mount %p\n",
 		    iod, nmp));
-		nfs_iodwant[iod] = NULL;
+		nfs_iodwant[iod] = NFSIOD_NOT_AVAILABLE;
 		nfs_iodmount[iod] = nmp;
 		nmp->nm_bufqiods++;
 		wakeup(&nfs_iodwant[iod]);

Modified: stable/8/sys/nfsclient/nfs_nfsiod.c
==============================================================================
--- stable/8/sys/nfsclient/nfs_nfsiod.c	Wed Feb 10 15:12:36 2010	(r203755)
+++ stable/8/sys/nfsclient/nfs_nfsiod.c	Wed Feb 10 16:16:50 2010	(r203756)
@@ -113,7 +113,7 @@ sysctl_iodmin(SYSCTL_HANDLER_ARGS)
 	 * than the new minimum, create some more.
 	 */
 	for (i = nfs_iodmin - nfs_numasync; i > 0; i--)
-		nfs_nfsiodnew();
+		nfs_nfsiodnew(0);
 out:
 	mtx_unlock(&nfs_iod_mtx);	
 	return (0);
@@ -147,7 +147,7 @@ sysctl_iodmax(SYSCTL_HANDLER_ARGS)
 	 */
 	iod = nfs_numasync - 1;
 	for (i = 0; i < nfs_numasync - nfs_iodmax; i++) {
-		if (nfs_iodwant[iod])
+		if (nfs_iodwant[iod] == NFSIOD_AVAILABLE)
 			wakeup(&nfs_iodwant[iod]);
 		iod--;
 	}
@@ -160,7 +160,7 @@ SYSCTL_PROC(_vfs_nfs, OID_AUTO, iodmax, 
     "Max number of nfsiod kthreads");
 
 int
-nfs_nfsiodnew(void)
+nfs_nfsiodnew(int set_iodwant)
 {
 	int error, i;
 	int newiod;
@@ -176,12 +176,17 @@ nfs_nfsiodnew(void)
 		}
 	if (newiod == -1)
 		return (-1);
+	if (set_iodwant > 0)
+		nfs_iodwant[i] = NFSIOD_CREATED_FOR_NFS_ASYNCIO;
 	mtx_unlock(&nfs_iod_mtx);
 	error = kproc_create(nfssvc_iod, nfs_asyncdaemon + i, NULL, RFHIGHPID,
 	    0, "nfsiod %d", newiod);
 	mtx_lock(&nfs_iod_mtx);
-	if (error)
+	if (error) {
+		if (set_iodwant > 0)
+			nfs_iodwant[i] = NFSIOD_NOT_AVAILABLE;
 		return (-1);
+	}
 	nfs_numasync++;
 	return (newiod);
 }
@@ -199,7 +204,7 @@ nfsiod_setup(void *dummy)
 		nfs_iodmin = NFS_MAXASYNCDAEMON;
 
 	for (i = 0; i < nfs_iodmin; i++) {
-		error = nfs_nfsiodnew();
+		error = nfs_nfsiodnew(0);
 		if (error == -1)
 			panic("nfsiod_setup: nfs_nfsiodnew failed");
 	}
@@ -236,7 +241,8 @@ nfssvc_iod(void *instance)
 			goto finish;
 		if (nmp)
 			nmp->nm_bufqiods--;
-		nfs_iodwant[myiod] = curthread->td_proc;
+		if (nfs_iodwant[myiod] == NFSIOD_NOT_AVAILABLE)
+			nfs_iodwant[myiod] = NFSIOD_AVAILABLE;
 		nfs_iodmount[myiod] = NULL;
 		/*
 		 * Always keep at least nfs_iodmin kthreads.
@@ -303,7 +309,7 @@ finish:
 	nfs_asyncdaemon[myiod] = 0;
 	if (nmp)
 	    nmp->nm_bufqiods--;
-	nfs_iodwant[myiod] = NULL;
+	nfs_iodwant[myiod] = NFSIOD_NOT_AVAILABLE;
 	nfs_iodmount[myiod] = NULL;
 	/* Someone may be waiting for the last nfsiod to terminate. */
 	if (--nfs_numasync == 0)

Modified: stable/8/sys/nfsclient/nfs_subs.c
==============================================================================
--- stable/8/sys/nfsclient/nfs_subs.c	Wed Feb 10 15:12:36 2010	(r203755)
+++ stable/8/sys/nfsclient/nfs_subs.c	Wed Feb 10 16:16:50 2010	(r203756)
@@ -347,7 +347,7 @@ nfs_init(struct vfsconf *vfsp)
 		nfs_ticks = 1;
 	/* Ensure async daemons disabled */
 	for (i = 0; i < NFS_MAXASYNCDAEMON; i++) {
-		nfs_iodwant[i] = NULL;
+		nfs_iodwant[i] = NFSIOD_NOT_AVAILABLE;
 		nfs_iodmount[i] = NULL;
 	}
 	nfs_nhinit();			/* Init the nfsnode table */
@@ -375,7 +375,7 @@ nfs_uninit(struct vfsconf *vfsp)
 	mtx_lock(&nfs_iod_mtx);
 	nfs_iodmax = 0;
 	for (i = 0; i < nfs_numasync; i++)
-		if (nfs_iodwant[i])
+		if (nfs_iodwant[i] == NFSIOD_AVAILABLE)
 			wakeup(&nfs_iodwant[i]);
 	/* The last nfsiod to exit will wake us up when nfs_numasync hits 0 */
 	while (nfs_numasync)

Modified: stable/8/sys/nfsclient/nfs_vnops.c
==============================================================================
--- stable/8/sys/nfsclient/nfs_vnops.c	Wed Feb 10 15:12:36 2010	(r203755)
+++ stable/8/sys/nfsclient/nfs_vnops.c	Wed Feb 10 16:16:50 2010	(r203756)
@@ -212,7 +212,7 @@ static int	nfs_renameit(struct vnode *sd
  * Global variables
  */
 struct mtx 	nfs_iod_mtx;
-struct proc	*nfs_iodwant[NFS_MAXASYNCDAEMON];
+enum nfsiod_state nfs_iodwant[NFS_MAXASYNCDAEMON];
 struct nfsmount *nfs_iodmount[NFS_MAXASYNCDAEMON];
 int		 nfs_numasync = 0;
 vop_advlock_t	*nfs_advlock_p = nfs_dolock;

Modified: stable/8/sys/nfsclient/nfsnode.h
==============================================================================
--- stable/8/sys/nfsclient/nfsnode.h	Wed Feb 10 15:12:36 2010	(r203755)
+++ stable/8/sys/nfsclient/nfsnode.h	Wed Feb 10 16:16:50 2010	(r203756)
@@ -177,10 +177,23 @@ struct nfsnode {
 #define NFS_TIMESPEC_COMPARE(T1, T2)	(((T1)->tv_sec != (T2)->tv_sec) || ((T1)->tv_nsec != (T2)->tv_nsec))
 
 /*
+ * NFS iod threads can be in one of these three states once spawned.
+ * NFSIOD_NOT_AVAILABLE - Cannot be assigned an I/O operation at this time.
+ * NFSIOD_AVAILABLE - Available to be assigned an I/O operation.
+ * NFSIOD_CREATED_FOR_NFS_ASYNCIO - Newly created for nfs_asyncio() and
+ *	will be used by the thread that called nfs_asyncio().
+ */
+enum nfsiod_state {
+	NFSIOD_NOT_AVAILABLE = 0,
+	NFSIOD_AVAILABLE = 1,
+	NFSIOD_CREATED_FOR_NFS_ASYNCIO = 2,
+};
+
+/*
  * Queue head for nfsiod's
  */
 extern TAILQ_HEAD(nfs_bufq, buf) nfs_bufq;
-extern struct proc *nfs_iodwant[NFS_MAXASYNCDAEMON];
+extern enum nfsiod_state nfs_iodwant[NFS_MAXASYNCDAEMON];
 extern struct nfsmount *nfs_iodmount[NFS_MAXASYNCDAEMON];
 
 #if defined(_KERNEL)



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201002101616.o1AGGo4f039911>