From owner-svn-src-all@FreeBSD.ORG Wed Feb 10 16:16:50 2010 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 80004106568B; Wed, 10 Feb 2010 16:16:50 +0000 (UTC) (envelope-from rmacklem@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 6C8EF8FC1B; Wed, 10 Feb 2010 16:16:50 +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 o1AGGocv039918; Wed, 10 Feb 2010 16:16:50 GMT (envelope-from rmacklem@svn.freebsd.org) Received: (from rmacklem@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id o1AGGo4f039911; Wed, 10 Feb 2010 16:16:50 GMT (envelope-from rmacklem@svn.freebsd.org) Message-Id: <201002101616.o1AGGo4f039911@svn.freebsd.org> From: Rick Macklem Date: Wed, 10 Feb 2010 16:16:50 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org X-SVN-Group: stable-8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r203756 - stable/8/sys/nfsclient X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 10 Feb 2010 16:16:50 -0000 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)