Should I merge fix for PR#64091 (NFS data corruption)?

Stephen McKay smckay at internode.on.net
Sun Apr 18 07:41:42 PDT 2004


Hi all!

This one only got fixed recently in -current, and I've only given it
a few hours of heavy testing, but the fix for PR#64091 looks to work
just as well on -stable, and I'd like to merge it.

The test programs in the PR provoke the bug in no more than a couple
of minutes in my tests (4.10-beta client, 4.9-release server).  With
the attached patch (taken nearly literally from -current), it's been
happy for about 8 hours now.

I appreciate that nobody wants NFS destabilised so close to the (probably)
last ever 4.x release, so I'd like people to try to poke holes in this
before I pester re@ for permission to merge.

Give the test programs in http://www.freebsd.org/cgi/query-pr.cgi?pr=64091
a go on a 4.10-beta client, then try it with the attached patch.

I suppose I'm particularly looking for anything that gets worse after the
patch, but any result good or bad is of interest.  In my tests, it fixes
the problem and doesn't noticeably increase network traffic or cause any
other problems.  I'd just like to be sure. :-)

Stephen.

PS NWANTED (in nfsnode.h) is totally bogus.  Luckily it is only used
in commented out code.  Should really be cleaned up though.

------8<------ ------8<------ ------8<------ ------8<------
Index: nfs_bio.c
===================================================================
RCS file: /cvs/src/sys/nfs/Attic/nfs_bio.c,v
retrieving revision 1.83.2.4
diff -u -r1.83.2.4 nfs_bio.c
--- nfs_bio.c	29 Dec 2002 18:19:53 -0000	1.83.2.4
+++ nfs_bio.c	18 Apr 2004 11:50:03 -0000
@@ -401,13 +401,15 @@
 			error = VOP_GETATTR(vp, &vattr, cred, p);
 			if (error)
 				return (error);
-			if (np->n_mtime != vattr.va_mtime.tv_sec) {
+			if ((np->n_flag & NSIZECHANGED)
+			    || np->n_mtime != vattr.va_mtime.tv_sec) {
 				if (vp->v_type == VDIR)
 					nfs_invaldir(vp);
 				error = nfs_vinvalbuf(vp, V_SAVE, cred, p, 1);
 				if (error)
 					return (error);
 				np->n_mtime = vattr.va_mtime.tv_sec;
+				np->n_flag &= ~NSIZECHANGED;
 			}
 		}
 	}
Index: nfs_subs.c
===================================================================
RCS file: /cvs/src/sys/nfs/Attic/nfs_subs.c,v
retrieving revision 1.90.2.2
diff -u -r1.90.2.2 nfs_subs.c
--- nfs_subs.c	25 Oct 2001 19:18:53 -0000	1.90.2.2
+++ nfs_subs.c	18 Apr 2004 11:50:03 -0000
@@ -1335,12 +1335,19 @@
 				vap->va_size = np->n_size;
 				np->n_attrstamp = 0;
 			} else if (np->n_flag & NMODIFIED) {
-				if (vap->va_size < np->n_size)
+				/*
+				 * We've modified the file: Use the larger
+				 * of our size, and the server's size.
+				 */
+				if (vap->va_size < np->n_size) {
 					vap->va_size = np->n_size;
-				else
+				} else {
 					np->n_size = vap->va_size;
+					np->n_flag |= NSIZECHANGED;
+				}
 			} else {
 				np->n_size = vap->va_size;
+				np->n_flag |= NSIZECHANGED;
 			}
 			vnode_pager_setsize(vp, np->n_size);
 		} else {
Index: nfsnode.h
===================================================================
RCS file: /cvs/src/sys/nfs/Attic/nfsnode.h,v
retrieving revision 1.32.2.1
diff -u -r1.32.2.1 nfsnode.h
--- nfsnode.h	26 Jun 2001 04:20:11 -0000	1.32.2.1
+++ nfsnode.h	18 Apr 2004 11:50:03 -0000
@@ -144,6 +144,7 @@
 #define	NCHG		0x0400	/* Special file times changed */
 #define NLOCKED		0x0800  /* node is locked */
 #define NWANTED		0x0100  /* someone wants to lock */
+#define	NSIZECHANGED	0x2000  /* File size has changed: need cache inval */
 
 /*
  * Convert between nfsnode pointers and vnode pointers
------8<------ ------8<------ ------8<------ ------8<------


More information about the freebsd-stable mailing list