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

Andre Albsmeier andre.albsmeier at siemens.com
Tue Jun 22 06:51:29 GMT 2004


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 at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-stable
> To unsubscribe, send any mail to "freebsd-stable-unsubscribe at freebsd.org"


More information about the freebsd-stable mailing list