NFS Stability

[Matthew Dillon]

:sdrhodus wrote @ Wed, 30 Mar 2005 13:17:46 +0000:
:
:> I know that Matt and myself almost regularly do various NFS testing,
:> and lots of the time the testing does involve using fsx.  Though any
:> outside validity testing is always welcome.
:
:This happens on my NFS over tcp mount,
:the server is an via epia with 256MB RAM.
:
:$ /usr/src/test/stress/fsx/dotest
:Chance of close/open is 1 in 100000
:truncating to largest ever: 0x2abbd1
:truncating to largest ever: 0x3abe49
:truncating to largest ever: 0x3b1956
:truncating to largest ever: 0x3e2816
:nfs server morex:/data: not responding
:nfs server morex:/data: is alive again
:truncating to largest ever: 0x3e8ae7
:nfs server morex:/data: not responding
:nfs server morex:/data: is alive again
:truncating to largest ever: 0x3f959f
:nfs server morex:/data: not responding
:Segmentation fault
:
:
:Andy

    Ok, Andrew, see if FSX still fails with this patch.  I think some of 
    the recent work I did broke the ftruncate() code.  I was hoping that
    the recent TCP issue was the cause but I can get fsx to seg-fault
    occassionally still and it happens on a UDP mount so there's definitely
    a bug still in there.

    The issue here is that ftruncate() does some buffer flushing before
    it issues the setattr RPC.  The buffer flushing can feed back (because
    RPC responses from the server often include attribute information) and
    confuse the NFS client's notion of the file's size.

    I give this about a 70% chance of fixing the problem.  The code 
    interactions are very complex so I don't know for sure.

					-Matt
					Matthew Dillon 
					<dillon at xxxxxxxxxxxxx>

Index: nfs_vnops.c
===================================================================
RCS file: /cvs/src/sys/vfs/nfs/nfs_vnops.c,v
retrieving revision 1.38
diff -u -r1.38 nfs_vnops.c
--- nfs_vnops.c	17 Mar 2005 17:28:46 -0000	1.38
+++ nfs_vnops.c	6 Apr 2005 04:41:57 -0000
@@ -736,13 +736,23 @@
 				return (EROFS);
 
 			/*
-			 * We run vnode_pager_setsize() early (why?),
-			 * we must set np->n_size now to avoid vinvalbuf
-			 * V_SAVE races that might setsize a lower
-			 * value.
+			 * This is nasty.  The RPCs we send to flush pending
+			 * data often return attribute information which is
+			 * cached via a callback to nfs_loadattrcache(), which
+			 * has the effect of changing our notion of the file
+			 * size.  Due to flushed appends and other operations
+			 * the file size can be set to virtually anything, 
+			 * including values that do not match either the old
+			 * or intended file size.
+			 *
+			 * When this condition is detected we must loop to
+			 * try the operation again.  Hopefully no more
+			 * flushing is required on the loop so it works the
+			 * second time around.  THIS CASE ALMOST ALWAYS
+			 * HAPPENS!
 			 */
-
 			tsize = np->n_size;
+again:
 			error = nfs_meta_setsize(vp, ap->a_td, vap->va_size);
 
  			if (np->n_flag & NLMODIFIED) {
@@ -750,30 +760,28 @@
  				error = nfs_vinvalbuf(vp, 0, ap->a_td, 1);
  			    else
  				error = nfs_vinvalbuf(vp, V_SAVE, ap->a_td, 1);
- 			    if (error) {
-				np->n_size = tsize;
-				vnode_pager_setsize(vp, np->n_size);
- 				return (error);
-			    }
  			}
-			/* 
-			 * np->n_size has already been set to vap->va_size
-			 * in nfs_meta_setsize(). We must set it again since
-			 * nfs_loadattrcache() could be called through
-			 * nfs_meta_setsize() and could modify np->n_size.
-			 *
-			 * (note that nfs_loadattrcache() will have called
-			 * vnode_pager_setsize() for us in that case).
+			/*
+			 * note: this loop case almost always happens at 
+			 * least once per truncation.
 			 */
-			np->n_vattr.va_size = np->n_size = vap->va_size;
+			if (error == 0 && np->n_size != vap->va_size)
+				goto again;
+			np->n_vattr.va_size = vap->va_size;
 			break;
 		}
   	} else if ((vap->va_mtime.tv_sec != VNOVAL ||
 		vap->va_atime.tv_sec != VNOVAL) && (np->n_flag & NLMODIFIED) &&
 		vp->v_type == VREG &&
-  		(error = nfs_vinvalbuf(vp, V_SAVE, ap->a_td, 1)) == EINTR)
+  		(error = nfs_vinvalbuf(vp, V_SAVE, ap->a_td, 1)) == EINTR
+	) {
 		return (error);
+	}
 	error = nfs_setattrrpc(vp, vap, ap->a_cred, ap->a_td);
+	if (error == 0 && np->n_size != vap->va_size) {
+		printf("NFS ftruncate: server disagrees on the file size: %lld/%lld/%lld\n", tsize, vap->va_size, np->n_size);
+		goto again;
+	}
 	if (error && vap->va_size != VNOVAL) {
 		np->n_size = np->n_vattr.va_size = tsize;
 		vnode_pager_setsize(vp, np->n_size);