Date: Mon, 19 Apr 2004 00:38:47 +1000 From: Stephen McKay <smckay@internode.on.net> To: freebsd-stable@freebsd.org Cc: Stephen McKay <smckay@internode.on.net> Subject: Should I merge fix for PR#64091 (NFS data corruption)? Message-ID: <200404181438.i3IEclNf010673@dungeon.home>
next in thread | raw e-mail | index | archive | help
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<------
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200404181438.i3IEclNf010673>