Date: Tue, 22 Jun 2004 08:51:11 +0200 From: Andre Albsmeier <andre.albsmeier@siemens.com> To: Stephen McKay <smckay@internode.on.net> Cc: freebsd-stable@freebsd.org Subject: Re: Should I merge fix for PR#64091 (NFS data corruption)? Message-ID: <20040622065111.GA25252@curry.mchp.siemens.de> In-Reply-To: <200404181438.i3IEclNf010673@dungeon.home> References: <200404181438.i3IEclNf010673@dungeon.home>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 19-Apr-2004 at 00:38:47 +1000, Stephen McKay wrote: > 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. Yes, please. I have been running the patch on -STABLE for a while now and haven't seen any bad effects so far. -Andre > > 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<------ > _______________________________________________ > freebsd-stable@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-stable > To unsubscribe, send any mail to "freebsd-stable-unsubscribe@freebsd.org"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040622065111.GA25252>