changing semantics of the va_filerev (code review)

[Rick Macklem]

In summary, the nfsv4 server needs 3 changes to the FreeBSD kernel:
1 - Sharing of nfssvc(). (This was just checked into FreeBSD-CURRENT.)
2 - Some calls that recall delegations must be done before local
     VOP_RENAME() and VOP_ADVLOCK(). I am waiting for comments to a
     vague post on this before I mail my first stab at coding this.
3 - Support for the Change attribute, which is what this post is about.

Once the above 3 things are resolved, the code should drop in without
further changes outside of its subtree.

As background, I believe va_filerev/i_modrev was added for nqnfs
long long ago. Since it is not exposed to userland via the stat structure,
I don't believe anything outside of the kernel uses it. Inside the kernel,
the only thing that currently uses it is the nfs server, which uses it as
the cookie verifier. (It really doesn't use it, since a client 
regurgitates it back to the server as opaque bits in the next readdir rpc
and the server then ignores those bits. This is correct, since va_filerev 
is a bogus cookie verifier.) As such, I don't believe changing the 
semantics of va_filerev will break anything in FreeBSD.

I'd like to change the semantics of va_filerev so that it can be used
by the nfsv4 server as the Change attribute. To do this, it needs to
change in 2 ways:
- must change upon metadata changes as well as data changes
- must persist across server reboots (ie. be moved to spare space in
 	the on-disk i-node instead of in memory i-node)

Here is the patch to ufs for the above, that I have been using for some
time. Please review and comment.

Thanks, rick
--- ufs patch to change va_filerev semantics ---
--- ufs/ufs/inode.h.sav	2009-04-12 02:29:05.000000000 -0400
+++ ufs/ufs/inode.h	2009-03-20 12:18:20.000000000 -0400
@@ -74,7 +74,6 @@

  	struct	 fs *i_fs;	/* Associated filesystem superblock. */
  	struct	 dquot *i_dquot[MAXQUOTAS]; /* Dquot structures. */
-	u_quad_t i_modrev;	/* Revision level for NFS lease. */
  	/*
  	 * Side effects; used during directory lookup.
  	 */
--- ufs/ufs/dinode.h.sav	2009-04-12 02:29:40.000000000 -0400
+++ ufs/ufs/dinode.h	2008-08-25 17:31:55.000000000 -0400
@@ -145,7 +145,8 @@
  	ufs2_daddr_t	di_extb[NXADDR];/*  96: External attributes block. */
  	ufs2_daddr_t	di_db[NDADDR];	/* 112: Direct disk blocks. */
  	ufs2_daddr_t	di_ib[NIADDR];	/* 208: Indirect disk blocks. */
-	int64_t		di_spare[3];	/* 232: Reserved; currently unused */
+	u_int64_t	di_modrev;	/* 232: i_modrev for NFSv4 */
+	int64_t		di_spare[2];	/* 240: Reserved; currently unused */
  };

  /*
@@ -183,7 +184,7 @@
  	int32_t		di_gen;		/* 108: Generation number. */
  	u_int32_t	di_uid;		/* 112: File owner. */
  	u_int32_t	di_gid;		/* 116: File group. */
-	int32_t		di_spare[2];	/* 120: Reserved; currently unused */
+	u_int64_t	di_modrev;	/* 120: i_modrev for NFSv4 */
  };
  #define	di_ogid		di_u.oldids[1]
  #define	di_ouid		di_u.oldids[0]
--- ufs/ufs/ufs_vnops.c.sav	2009-04-12 02:28:41.000000000 -0400
+++ ufs/ufs/ufs_vnops.c	2009-03-10 16:47:11.000000000 -0400
@@ -157,11 +157,12 @@
  	if (ip->i_flag & IN_UPDATE) {
  		DIP_SET(ip, i_mtime, ts.tv_sec);
  		DIP_SET(ip, i_mtimensec, ts.tv_nsec);
-		ip->i_modrev++;
+		DIP_SET(ip, i_modrev, DIP(ip, i_modrev) + 1);
  	}
  	if (ip->i_flag & IN_CHANGE) {
  		DIP_SET(ip, i_ctime, ts.tv_sec);
  		DIP_SET(ip, i_ctimensec, ts.tv_nsec);
+		DIP_SET(ip, i_modrev, DIP(ip, i_modrev) + 1);
  	}

   out:
@@ -446,6 +447,7 @@
  		vap->va_ctime.tv_sec = ip->i_din1->di_ctime;
  		vap->va_ctime.tv_nsec = ip->i_din1->di_ctimensec;
  		vap->va_bytes = dbtob((u_quad_t)ip->i_din1->di_blocks);
+		vap->va_filerev = ip->i_din1->di_modrev;
  	} else {
  		vap->va_rdev = ip->i_din2->di_rdev;
  		vap->va_size = ip->i_din2->di_size;
@@ -456,12 +458,12 @@
  		vap->va_birthtime.tv_sec = ip->i_din2->di_birthtime;
  		vap->va_birthtime.tv_nsec = ip->i_din2->di_birthnsec;
  		vap->va_bytes = dbtob((u_quad_t)ip->i_din2->di_blocks);
+		vap->va_filerev = ip->i_din2->di_modrev;
  	}
  	vap->va_flags = ip->i_flags;
  	vap->va_gen = ip->i_gen;
  	vap->va_blocksize = vp->v_mount->mnt_stat.f_iosize;
  	vap->va_type = IFTOVT(ip->i_mode);
-	vap->va_filerev = ip->i_modrev;
  	return (0);
  }

@@ -2223,7 +2225,6 @@
  	ASSERT_VOP_LOCKED(vp, "ufs_vinit");
  	if (ip->i_number == ROOTINO)
  		vp->v_vflag |= VV_ROOT;
-	ip->i_modrev = init_va_filerev();
  	*vpp = vp;
  	return (0);
  }