From owner-freebsd-stable@FreeBSD.ORG Fri Jan 22 22:02:36 2010 Return-Path: Delivered-To: freebsd-stable@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A77C01065672; Fri, 22 Jan 2010 22:02:36 +0000 (UTC) (envelope-from rmacklem@uoguelph.ca) Received: from esa-annu.mail.uoguelph.ca (esa-annu.mail.uoguelph.ca [131.104.91.36]) by mx1.freebsd.org (Postfix) with ESMTP id 3BE198FC19; Fri, 22 Jan 2010 22:02:35 +0000 (UTC) X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: ApoEAHWvWUuDaFvI/2dsb2JhbADYfIQ8BA X-IronPort-AV: E=Sophos;i="4.49,326,1262581200"; d="scan'208";a="62614338" Received: from darling.cs.uoguelph.ca ([131.104.91.200]) by esa-annu-pri.mail.uoguelph.ca with ESMTP; 22 Jan 2010 17:02:35 -0500 Received: from localhost (localhost.localdomain [127.0.0.1]) by darling.cs.uoguelph.ca (Postfix) with ESMTP id 565B394016A; Fri, 22 Jan 2010 17:02:35 -0500 (EST) X-Virus-Scanned: amavisd-new at darling.cs.uoguelph.ca Received: from darling.cs.uoguelph.ca ([127.0.0.1]) by localhost (darling.cs.uoguelph.ca [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id BqvrPt5TaKse; Fri, 22 Jan 2010 17:02:33 -0500 (EST) Received: from muncher.cs.uoguelph.ca (muncher.cs.uoguelph.ca [131.104.91.102]) by darling.cs.uoguelph.ca (Postfix) with ESMTP id DC38F940062; Fri, 22 Jan 2010 17:02:33 -0500 (EST) Received: from localhost (rmacklem@localhost) by muncher.cs.uoguelph.ca (8.11.7p3+Sun/8.11.6) with ESMTP id o0MMD9o07073; Fri, 22 Jan 2010 17:13:09 -0500 (EST) X-Authentication-Warning: muncher.cs.uoguelph.ca: rmacklem owned process doing -bs Date: Fri, 22 Jan 2010 17:13:09 -0500 (EST) From: Rick Macklem X-X-Sender: rmacklem@muncher.cs.uoguelph.ca To: Mikolaj Golub In-Reply-To: Message-ID: References: <86ocl272mb.fsf@kopusha.onet> <86tyuqnz9x.fsf@zhuzha.ua1> <86zl4awmon.fsf@zhuzha.ua1> <86vdeywmha.fsf@zhuzha.ua1> <86vdeuuo2y.fsf@zhuzha.ua1> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: freebsd-fs@FreeBSD.org, freebsd-stable@FreeBSD.org Subject: Re: FreeBSD NFS client/Linux NFS server issue X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 22 Jan 2010 22:02:36 -0000 On Fri, 22 Jan 2010, Rick Macklem wrote: > > There should probably be some sort of 3 way handshake between > the code in nfs_asyncio() after calling nfs_nfsnewiod() and the > code near the beginning of nfssvc_iod(), but I think the following > somewhat cheesy fix might do the trick: > [stuff deleted] I know it's a little weird to reply to my own posting, but I think this might be a reasonable patch (I have only tested it for a few minutes at this point). I basically redefined nfs_iodwant[] as a tri-state variable (although it was a struct proc *, it was only tested NULL/non-NULL). 0 - was NULL 1 - was non-NULL -1 - just created by nfs_asyncio() and will be used by it I'll keep testing it, but hopefully someone else can test and/or review it... rick ps: Mikolaj, I'm a sysadmin so I understand the problems with production systems, but if you do get a chance to test it somehow, that would be great. pss: This is against -current, but hopefully stable/7 can be patched about the same. --- patch for nfsiod race against -current --- --- nfsclient/nfs.h.sav 2010-01-22 16:21:53.000000000 -0500 +++ nfsclient/nfs.h 2010-01-22 16:22:04.000000000 -0500 @@ -252,7 +252,7 @@ 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 *); --- nfsclient/nfsnode.h.sav 2010-01-22 14:56:34.000000000 -0500 +++ nfsclient/nfsnode.h 2010-01-22 14:56:52.000000000 -0500 @@ -180,7 +180,7 @@ * Queue head for nfsiod's */ extern TAILQ_HEAD(nfs_bufq, buf) nfs_bufq; -extern struct proc *nfs_iodwant[NFS_MAXASYNCDAEMON]; +extern int nfs_iodwant[NFS_MAXASYNCDAEMON]; extern struct nfsmount *nfs_iodmount[NFS_MAXASYNCDAEMON]; #if defined(_KERNEL) --- nfsclient/nfs_bio.c.sav 2010-01-22 14:57:28.000000000 -0500 +++ nfsclient/nfs_bio.c 2010-01-22 16:17:24.000000000 -0500 @@ -1377,7 +1377,7 @@ * Find a free iod to process this request. */ for (iod = 0; iod < nfs_numasync; iod++) - if (nfs_iodwant[iod]) { + if (nfs_iodwant[iod] > 0) { gotiod = TRUE; break; } @@ -1386,7 +1386,7 @@ * 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 @@ */ NFS_DPF(ASYNCIO, ("nfs_asyncio: waking iod %d for mount %p\n", iod, nmp)); - nfs_iodwant[iod] = NULL; + nfs_iodwant[iod] = 0; nfs_iodmount[iod] = nmp; nmp->nm_bufqiods++; wakeup(&nfs_iodwant[iod]); --- nfsclient/nfs_nfsiod.c.sav 2010-01-22 14:57:28.000000000 -0500 +++ nfsclient/nfs_nfsiod.c 2010-01-22 16:32:31.000000000 -0500 @@ -113,7 +113,7 @@ * 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 @@ */ iod = nfs_numasync - 1; for (i = 0; i < nfs_numasync - nfs_iodmax; i++) { - if (nfs_iodwant[iod]) + if (nfs_iodwant[iod] > 0) wakeup(&nfs_iodwant[iod]); iod--; } @@ -160,7 +160,7 @@ "Max number of nfsiod kthreads"); int -nfs_nfsiodnew(void) +nfs_nfsiodnew(int set_iodwant) { int error, i; int newiod; @@ -176,12 +176,17 @@ } if (newiod == -1) return (-1); + if (set_iodwant > 0) + nfs_iodwant[i] = -1; 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] = 0; return (-1); + } nfs_numasync++; return (newiod); } @@ -199,7 +204,7 @@ 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 @@ goto finish; if (nmp) nmp->nm_bufqiods--; - nfs_iodwant[myiod] = curthread->td_proc; + if (nfs_iodwant[myiod] == 0) + nfs_iodwant[myiod] = 1; nfs_iodmount[myiod] = NULL; /* * Always keep at least nfs_iodmin kthreads. @@ -303,7 +309,7 @@ nfs_asyncdaemon[myiod] = 0; if (nmp) nmp->nm_bufqiods--; - nfs_iodwant[myiod] = NULL; + nfs_iodwant[myiod] = 0; nfs_iodmount[myiod] = NULL; /* Someone may be waiting for the last nfsiod to terminate. */ if (--nfs_numasync == 0) --- nfsclient/nfs_subs.c.sav 2010-01-22 14:57:28.000000000 -0500 +++ nfsclient/nfs_subs.c 2010-01-22 16:35:10.000000000 -0500 @@ -347,7 +347,7 @@ nfs_ticks = 1; /* Ensure async daemons disabled */ for (i = 0; i < NFS_MAXASYNCDAEMON; i++) { - nfs_iodwant[i] = NULL; + nfs_iodwant[i] = 0; nfs_iodmount[i] = NULL; } nfs_nhinit(); /* Init the nfsnode table */ @@ -375,7 +375,7 @@ mtx_lock(&nfs_iod_mtx); nfs_iodmax = 0; for (i = 0; i < nfs_numasync; i++) - if (nfs_iodwant[i]) + if (nfs_iodwant[i] > 0) wakeup(&nfs_iodwant[i]); /* The last nfsiod to exit will wake us up when nfs_numasync hits 0 */ while (nfs_numasync) --- nfsclient/nfs_vnops.c.sav 2010-01-22 14:57:28.000000000 -0500 +++ nfsclient/nfs_vnops.c 2010-01-22 15:01:38.000000000 -0500 @@ -212,7 +212,7 @@ * Global variables */ struct mtx nfs_iod_mtx; -struct proc *nfs_iodwant[NFS_MAXASYNCDAEMON]; +int nfs_iodwant[NFS_MAXASYNCDAEMON]; struct nfsmount *nfs_iodmount[NFS_MAXASYNCDAEMON]; int nfs_numasync = 0; vop_advlock_t *nfs_advlock_p = nfs_dolock;