From owner-svn-src-stable@FreeBSD.ORG Mon Nov 8 15:22:21 2010 Return-Path: Delivered-To: svn-src-stable@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 008181065788; Mon, 8 Nov 2010 15:22:21 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id D8DB88FC27; Mon, 8 Nov 2010 15:22:20 +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 oA8FMKhd037634; Mon, 8 Nov 2010 15:22:20 GMT (envelope-from kib@svn.freebsd.org) Received: (from kib@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id oA8FMKSd037627; Mon, 8 Nov 2010 15:22:20 GMT (envelope-from kib@svn.freebsd.org) Message-Id: <201011081522.oA8FMKSd037627@svn.freebsd.org> From: Konstantin Belousov Date: Mon, 8 Nov 2010 15:22:20 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-7@freebsd.org X-SVN-Group: stable-7 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r214986 - stable/7/sys/nfsclient X-BeenThere: svn-src-stable@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for all the -stable branches of the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 08 Nov 2010 15:22:21 -0000 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 #include #include +#include #include #include @@ -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 #include #include +#include #include #include @@ -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)