svn commit: r214986 - stable/7/sys/nfsclient

Konstantin Belousov kib at FreeBSD.org
Mon Nov 8 15:22:21 UTC 2010


Author: kib
Date: Mon Nov  8 15:22:20 2010
New Revision: 214986
URL: http://svn.freebsd.org/changeset/base/214986

Log:
  MFC r203072 (by rmacklem, to be reverted by MFC of r214026):
  Fix a race that can occur when nfs nfsiod threads are being created.
  
  MFC r212506:
  Do not fork nfsiod directly from the vop methods.
  
  MFC r214026:
  Do not synchronously start the nfsiod threads at all.
  
  Tested by:	jhb

Modified:
  stable/7/sys/nfsclient/nfs.h
  stable/7/sys/nfsclient/nfs_bio.c
  stable/7/sys/nfsclient/nfs_nfsiod.c
  stable/7/sys/nfsclient/nfs_subs.c
  stable/7/sys/nfsclient/nfs_vnops.c
  stable/7/sys/nfsclient/nfsnode.h
Directory Properties:
  stable/7/sys/   (props changed)
  stable/7/sys/cddl/contrib/opensolaris/   (props changed)
  stable/7/sys/contrib/dev/acpica/   (props changed)
  stable/7/sys/contrib/pf/   (props changed)

Modified: stable/7/sys/nfsclient/nfs.h
==============================================================================
--- stable/7/sys/nfsclient/nfs.h	Mon Nov  8 15:14:14 2010	(r214985)
+++ stable/7/sys/nfsclient/nfs.h	Mon Nov  8 15:22:20 2010	(r214986)
@@ -135,6 +135,7 @@ extern struct uma_zone *nfsmount_zone;
 extern struct callout nfs_callout;
 extern struct nfsstats nfsstats;
 extern struct mtx nfs_iod_mtx;
+extern struct task nfs_nfsiodnew_task;
 
 extern int nfs_numasync;
 extern unsigned int nfs_iodmax;
@@ -305,7 +306,8 @@ 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);
+void	nfs_nfsiodnew(void);
+void	nfs_nfsiodnew_tq(__unused void *, 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/7/sys/nfsclient/nfs_bio.c
==============================================================================
--- stable/7/sys/nfsclient/nfs_bio.c	Mon Nov  8 15:14:14 2010	(r214985)
+++ stable/7/sys/nfsclient/nfs_bio.c	Mon Nov  8 15:22:20 2010	(r214986)
@@ -1375,7 +1375,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;
 		}
@@ -1383,20 +1383,16 @@ again:
 	/*
 	 * Try to create one if none are free.
 	 */
-	if (!gotiod) {
-		iod = nfs_nfsiodnew();
-		if (iod != -1)
-			gotiod = TRUE;
-	}
-
-	if (gotiod) {
+	if (!gotiod)
+		nfs_nfsiodnew();
+	else {
 		/*
 		 * Found one, so wake it up and tell it which
 		 * mount to process.
 		 */
 		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]);
@@ -1409,7 +1405,7 @@ again:
 	if (!gotiod) {
 		if (nmp->nm_bufqiods > 0) {
 			NFS_DPF(ASYNCIO,
-				("nfs_asyncio: %d iods are already processing mount %p\n",
+		("nfs_asyncio: %d iods are already processing mount %p\n",
 				 nmp->nm_bufqiods, nmp));
 			gotiod = TRUE;
 		}
@@ -1424,9 +1420,9 @@ again:
 		 * Ensure that the queue never grows too large.  We still want
 		 * to asynchronize so we block rather then return EIO.
 		 */
-		while (nmp->nm_bufqlen >= 2*nfs_numasync) {
+		while (nmp->nm_bufqlen >= 2 * nfs_numasync) {
 			NFS_DPF(ASYNCIO,
-				("nfs_asyncio: waiting for mount %p queue to drain\n", nmp));
+		("nfs_asyncio: waiting for mount %p queue to drain\n", nmp));
 			nmp->nm_bufqwant = TRUE;
  			error = nfs_msleep(td, &nmp->nm_bufq, &nfs_iod_mtx, 
 					   slpflag | PRIBIO,
@@ -1434,7 +1430,7 @@ again:
 			if (error) {
 				error2 = nfs_sigintr(nmp, NULL, td);
 				if (error2) {
-					mtx_unlock(&nfs_iod_mtx);					
+					mtx_unlock(&nfs_iod_mtx);
 					return (error2);
 				}
 				if (slpflag == PCATCH) {
@@ -1446,17 +1442,13 @@ again:
 			 * We might have lost our iod while sleeping,
 			 * so check and loop if nescessary.
 			 */
-			if (nmp->nm_bufqiods == 0) {
-				NFS_DPF(ASYNCIO,
-					("nfs_asyncio: no iods after mount %p queue was drained, looping\n", nmp));
-				goto again;
-			}
+			goto again;
 		}
 
 		/* We might have lost our nfsiod */
 		if (nmp->nm_bufqiods == 0) {
 			NFS_DPF(ASYNCIO,
-				("nfs_asyncio: no iods after mount %p queue was drained, looping\n", nmp));
+("nfs_asyncio: no iods after mount %p queue was drained, looping\n", nmp));
 			goto again;
 		}
 

Modified: stable/7/sys/nfsclient/nfs_nfsiod.c
==============================================================================
--- stable/7/sys/nfsclient/nfs_nfsiod.c	Mon Nov  8 15:14:14 2010	(r214985)
+++ stable/7/sys/nfsclient/nfs_nfsiod.c	Mon Nov  8 15:22:20 2010	(r214986)
@@ -59,6 +59,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/fcntl.h>
 #include <sys/lockf.h>
 #include <sys/mutex.h>
+#include <sys/taskqueue.h>
 
 #include <netinet/in.h>
 #include <netinet/tcp.h>
@@ -92,6 +93,8 @@ unsigned int nfs_iodmax = 20;
 /* Minimum number of nfsiod kthreads to keep as spares */
 static unsigned int nfs_iodmin = 0;
 
+static int nfs_nfsiodnew_sync(void);
+
 static int
 sysctl_iodmin(SYSCTL_HANDLER_ARGS)
 {
@@ -115,7 +118,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_sync();
 out:
 	mtx_unlock(&nfs_iod_mtx);	
 	return (0);
@@ -148,7 +151,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--;
 	}
@@ -159,37 +162,55 @@ out:
 SYSCTL_PROC(_vfs_nfs, OID_AUTO, iodmax, CTLTYPE_UINT | CTLFLAG_RW, 0,
     sizeof (nfs_iodmax), sysctl_iodmax, "IU", "");
 
-int
-nfs_nfsiodnew(void)
+static int
+nfs_nfsiodnew_sync(void)
 {
 	int error, i;
-	int newiod;
 
-	if (nfs_numasync >= nfs_iodmax)
-		return (-1);
-	newiod = -1;
-	for (i = 0; i < nfs_iodmax; i++)
+	mtx_assert(&nfs_iod_mtx, MA_OWNED);
+	for (i = 0; i < nfs_iodmax; i++) {
 		if (nfs_asyncdaemon[i] == 0) {
-			nfs_asyncdaemon[i]++;
-			newiod = i;
+			nfs_asyncdaemon[i] = 1;
 			break;
 		}
-	if (newiod == -1)
-		return (-1);
+	}
+	if (i == nfs_iodmax)
+		return (0);
 	mtx_unlock(&nfs_iod_mtx);
-	error = kthread_create(nfssvc_iod, nfs_asyncdaemon + i, NULL, RFHIGHPID,
-	    0, "nfsiod %d", newiod);
+	error = kthread_create(nfssvc_iod, nfs_asyncdaemon + i, NULL,
+	    RFHIGHPID, 0, "nfsiod %d", i);
+	mtx_lock(&nfs_iod_mtx);
+	if (error == 0) {
+		nfs_numasync++;
+		nfs_iodwant[i] = NFSIOD_AVAILABLE;
+	} else
+		nfs_asyncdaemon[i] = 0;
+	return (error);
+}
+
+void
+nfs_nfsiodnew_tq(__unused void *arg, int pending)
+{
+
 	mtx_lock(&nfs_iod_mtx);
-	if (error)
-		return (-1);
-	nfs_numasync++;
-	return (newiod);
+	while (pending > 0) {
+		pending--;
+		nfs_nfsiodnew_sync();
+	}
+	mtx_unlock(&nfs_iod_mtx);
+}
+
+void
+nfs_nfsiodnew(void)
+{
+
+	mtx_assert(&nfs_iod_mtx, MA_OWNED);
+	taskqueue_enqueue(taskqueue_thread, &nfs_nfsiodnew_task);
 }
 
 static void
 nfsiod_setup(void *dummy)
 {
-	int i;
 	int error;
 
 	TUNABLE_INT_FETCH("vfs.nfs.iodmin", &nfs_iodmin);
@@ -198,8 +219,8 @@ nfsiod_setup(void *dummy)
 	if (nfs_iodmin > NFS_MAXASYNCDAEMON)
 		nfs_iodmin = NFS_MAXASYNCDAEMON;
 
-	for (i = 0; i < nfs_iodmin; i++) {
-		error = nfs_nfsiodnew();
+	while (nfs_numasync < nfs_iodmin) {
+		error = nfs_nfsiodnew_sync();
 		if (error == -1)
 			panic("nfsiod_setup: nfs_nfsiodnew failed");
 	}
@@ -235,7 +256,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.
@@ -302,7 +324,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/7/sys/nfsclient/nfs_subs.c
==============================================================================
--- stable/7/sys/nfsclient/nfs_subs.c	Mon Nov  8 15:14:14 2010	(r214985)
+++ stable/7/sys/nfsclient/nfs_subs.c	Mon Nov  8 15:22:20 2010	(r214986)
@@ -57,6 +57,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/sysent.h>
 #include <sys/syscall.h>
 #include <sys/sysproto.h>
+#include <sys/taskqueue.h>
 
 #include <vm/vm.h>
 #include <vm/vm_object.h>
@@ -103,6 +104,7 @@ struct nfs_reqq	nfs_reqq;
 struct mtx nfs_reqq_mtx;
 struct nfs_bufq	nfs_bufq;
 static struct mtx nfs_xid_mtx;
+struct task	nfs_nfsiodnew_task;
 
 /*
  * and the reverse mapping from generic to Version 2 procedure numbers
@@ -422,7 +424,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 */
@@ -435,6 +437,7 @@ nfs_init(struct vfsconf *vfsp)
 	mtx_init(&nfs_reqq_mtx, "NFS reqq lock", NULL, MTX_DEF);
 	mtx_init(&nfs_iod_mtx, "NFS iod lock", NULL, MTX_DEF);
 	mtx_init(&nfs_xid_mtx, "NFS xid lock", NULL, MTX_DEF);
+	TASK_INIT(&nfs_nfsiodnew_task, 0, nfs_nfsiodnew_tq, NULL);
 
 	nfs_pbuf_freecnt = nswbuf / 2 + 1;
 
@@ -454,11 +457,15 @@ nfs_uninit(struct vfsconf *vfsp)
 	/*
 	 * Tell all nfsiod processes to exit. Clear nfs_iodmax, and wakeup
 	 * any sleeping nfsiods so they check nfs_iodmax and exit.
+	 * Drain nfsiodnew task before we wait for them to finish.
 	 */
 	mtx_lock(&nfs_iod_mtx);
 	nfs_iodmax = 0;
+	mtx_unlock(&nfs_iod_mtx);
+	taskqueue_drain(taskqueue_thread, &nfs_nfsiodnew_task);
+	mtx_lock(&nfs_iod_mtx);
 	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/7/sys/nfsclient/nfs_vnops.c
==============================================================================
--- stable/7/sys/nfsclient/nfs_vnops.c	Mon Nov  8 15:14:14 2010	(r214985)
+++ stable/7/sys/nfsclient/nfs_vnops.c	Mon Nov  8 15:22:20 2010	(r214986)
@@ -195,7 +195,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/7/sys/nfsclient/nfsnode.h
==============================================================================
--- stable/7/sys/nfsclient/nfsnode.h	Mon Nov  8 15:14:14 2010	(r214985)
+++ stable/7/sys/nfsclient/nfsnode.h	Mon Nov  8 15:22:20 2010	(r214986)
@@ -173,10 +173,20 @@ 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 two 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.
+ */
+enum nfsiod_state {
+	NFSIOD_NOT_AVAILABLE = 0,
+	NFSIOD_AVAILABLE = 1,
+};
+
+/*
  * 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)


More information about the svn-src-stable-7 mailing list