Re2: NFS Stability

[Raphael Marmier]

Just tried it. Bad news:

dragonfly# /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
Raphael

Matthew Dillon wrote:
    Scratch that last patch.  Try this one instead.  I blew a check.

    If other people would like to help, what we are trying to do here
    is test an NFS client with the 'fsx' program and 'dotest' script found in
    /usr/src/test/stress/fsx.  To run the test you must have two machines
    and run it in a directory on the client that is mounted from the server
    r/w.
					-Matt

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 05:42: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,34 @@
  				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);
+
+	/*
+	 * Sanity check if a truncation was issued.  This should only occur
+	 * if multiple processes are racing on the same file.
+	 */
+	if (error == 0 && vap->va_size != VNOVAL && 
+	    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);