Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 22 Jan 2010 17:13:09 -0500 (EST)
From:      Rick Macklem <rmacklem@uoguelph.ca>
To:        Mikolaj Golub <to.my.trociny@gmail.com>
Cc:        freebsd-fs@FreeBSD.org, freebsd-stable@FreeBSD.org
Subject:   Re: FreeBSD NFS client/Linux NFS server issue
Message-ID:  <Pine.GSO.4.63.1001221705400.6098@muncher.cs.uoguelph.ca>
In-Reply-To: <Pine.GSO.4.63.1001221400590.29868@muncher.cs.uoguelph.ca>
References:  <86ocl272mb.fsf@kopusha.onet> <86tyuqnz9x.fsf@zhuzha.ua1> <86zl4awmon.fsf@zhuzha.ua1> <86vdeywmha.fsf@zhuzha.ua1> <86vdeuuo2y.fsf@zhuzha.ua1> <Pine.GSO.4.63.1001221400590.29868@muncher.cs.uoguelph.ca>

next in thread | previous in thread | raw e-mail | index | archive | help


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;



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