Reimplement the NFS ACCESS RPC cache as an "accelerator" rather than a true

cache.  If the cached result lets us say "yes", then go with that.  If
we're not sure, or we think the answer might be "no", go to the wire to be
certain.    This avoids all of the possible false veto cases, and allows us
to key the cached value with just the UID for which the cached value holds,
reducing the bloat of the nfsnode structure from 104 bytes to just 12 bytes.

Since the "yes" case is by far the most common, this should still provide
a substantial performance improvement.  Also default the cache to on, with
a conservative timeout (2 seconds).  This improves performance if NFS is
loaded as a KLD module, as there's not (yet) code to parse an option out
of the module arguments to set it, and sysctl doesn't work (yet) for OIDs
in modules.

The 'accelerator' mode was suggested by Bjoern Groenvall (bg@sics.se)

Feedback on this would be appreciated as testing has been necessarily
limited by Comdex, and it would be valuable to have this in 2.2.8.

4 files changed, 28 insertions, 88 deletions

diff --git a/sys/nfs/nfs_vnops.c b/sys/nfs/nfs_vnops.c
index c2c20d7..4ca9feb 100644
--- a/sys/nfs/nfs_vnops.c
+++ b/sys/nfs/nfs_vnops.c
@@ -34,7 +34,7 @@
  * SUCH DAMAGE.
  *
  *	@(#)nfs_vnops.c	8.16 (Berkeley) 5/27/95
- * $Id: nfs_vnops.c,v 1.112 1998/11/13 02:39:09 msmith Exp $
+ * $Id: nfs_vnops.c,v 1.113 1998/11/13 22:58:48 msmith Exp $
  */
 
 
@@ -248,7 +248,7 @@ struct nfsmount *nfs_iodmount[NFS_MAXASYNCDAEMON];
 int nfs_numasync = 0;
 #define	DIRHDSIZ	(sizeof (struct dirent) - (MAXNAMLEN + 1))
 
-static int	nfsaccess_cache_timeout;
+static int	nfsaccess_cache_timeout = 2;
 SYSCTL_INT(_vfs_nfs, OID_AUTO, access_cache_timeout, CTLFLAG_RW, 
 	   &nfsaccess_cache_timeout, 0, "NFS ACCESS cache timeout");
 
@@ -261,23 +261,6 @@ SYSCTL_INT(_vfs_nfs, OID_AUTO, access_cache_fills, CTLFLAG_RD,
 	   &nfsaccess_cache_fills, 0, "NFS ACCESS cache fill count");
 
 /*
- * Compare two ucred structures, returns zero on equality, nonzero
- * otherwise.
- */
-static int
-nfsa_ucredcmp(struct ucred *c1, struct ucred *c2)
-{
-    int		i;
-    
-    if ((c1->cr_uid != c2->cr_uid) || (c1->cr_ngroups != c2->cr_ngroups))
-	return(1);
-    for (i = 0; i < c1->cr_ngroups; i++)
-	if (c1->cr_groups[i] != c2->cr_groups[i])
-	    return(1);
-    return(0);
-}
-
-/*
  * nfs access vnode op.
  * For nfs version 2, just return ok. File accesses may fail later.
  * For nfs version 3, use the access rpc to check accessibility. If file modes
@@ -352,13 +335,18 @@ nfs_access(ap)
 			wmode = mode;
 		}
 
-		/* do we have a cached result? */
+		/*
+		 * Does our cached result allow us to give a definite yes to
+		 * this request?
+		 */
 		if ((time_second < (np->n_modestamp + nfsaccess_cache_timeout)) &&
-		    !nfsa_ucredcmp(ap->a_cred, &np->n_modecred)) {
+		    (ap->a_cred->cr_uid == np->n_modeuid) &&
+		    ((np->n_mode & mode) == mode)) {
 			nfsaccess_cache_hits++;
-			if ((np->n_mode & mode) != mode)
-				error = EACCES;
 		} else {
+			/*
+			 * Either a no, or a don't know.  Go to the wire.
+			 */
 			nfsstats.rpccnt[NFSPROC_ACCESS]++;
 			nfsm_reqhead(vp, NFSPROC_ACCESS, NFSX_FH(v3) + NFSX_UNSIGNED);
 			nfsm_fhtom(vp, v3);
@@ -380,7 +368,7 @@ nfs_access(ap)
 					/* cache the result */
 					nfsaccess_cache_fills++;
 					np->n_mode = rmode;
-					np->n_modecred = *ap->a_cred;
+					np->n_modeuid = ap->a_cred->cr_uid;
 					np->n_modestamp = time_second;
 				}
 			}
@@ -700,11 +688,6 @@ nfs_setattr(ap)
 		 ap->a_p, 1)) == EINTR)
 		return (error);
 	error = nfs_setattrrpc(vp, vap, ap->a_cred, ap->a_p);
-	/* 
-	 * Attributes on server may have changed, make no assumptions about 
-	 * the server's reaction to these changes.
-	 */
-	np->n_modestamp = 0;
 	if (error && vap->va_size != VNOVAL) {
 		np->n_size = np->n_vattr.va_size = tsize;
 		vnode_pager_setsize(vp, np->n_size);
@@ -1562,7 +1545,6 @@ nfs_remove(ap)
 		error = nfs_sillyrename(dvp, vp, cnp);
 	zfree(namei_zone, cnp->cn_pnbuf);
 	np->n_attrstamp = 0;
-	np->n_modestamp = 0;
 	return (error);
 }
 
@@ -1667,18 +1649,6 @@ nfs_rename(ap)
 		cache_purge(fdvp);
 	}
 
-	/* 
-	 * We can't presume too much on the server's access control method(s),
-	 * and it may use rules based on filenames or locations.  Moving to a
-	 * more restrictive location would be harmless, but moving to a less
-	 * restrictive location you would be forced to wait for the cache
-	 * entry to time out.
-	 */
-	if (fvp->v_data)
-		VTONFS(fvp)->n_modestamp = 0;
-	if (tvp != NULL)
-		VTONFS(tvp)->n_modestamp = 0;
-
 out:
 	if (tdvp == tvp)
 		vrele(tdvp);
diff --git a/sys/nfs/nfsnode.h b/sys/nfs/nfsnode.h
index b242216..49adeb2 100644
--- a/sys/nfs/nfsnode.h
+++ b/sys/nfs/nfsnode.h
@@ -34,7 +34,7 @@
  * SUCH DAMAGE.
  *
  *	@(#)nfsnode.h	8.9 (Berkeley) 5/14/95
- * $Id: nfsnode.h,v 1.26 1998/05/31 18:32:23 peter Exp $
+ * $Id: nfsnode.h,v 1.27 1998/11/13 02:39:09 msmith Exp $
  */
 
 
@@ -94,7 +94,7 @@ struct nfsnode {
 	struct vattr		n_vattr;	/* Vnode attribute cache */
 	time_t			n_attrstamp;	/* Attr. cache timestamp */
 	u_int32_t		n_mode;		/* ACCESS mode cache */
-	struct ucred		n_modecred;	/* credentials having mode */
+	uid_t			n_modeuid;	/* credentials having mode */
 	time_t			n_modestamp;	/* mode cache timestamp */
 	time_t			n_mtime;	/* Prev modify time. */
 	time_t			n_ctime;	/* Prev create time. */
diff --git a/sys/nfsclient/nfs_vnops.c b/sys/nfsclient/nfs_vnops.c
index c2c20d7..4ca9feb 100644
--- a/sys/nfsclient/nfs_vnops.c
+++ b/sys/nfsclient/nfs_vnops.c
@@ -34,7 +34,7 @@
  * SUCH DAMAGE.
  *
  *	@(#)nfs_vnops.c	8.16 (Berkeley) 5/27/95
- * $Id: nfs_vnops.c,v 1.112 1998/11/13 02:39:09 msmith Exp $
+ * $Id: nfs_vnops.c,v 1.113 1998/11/13 22:58:48 msmith Exp $
  */
 
 
@@ -248,7 +248,7 @@ struct nfsmount *nfs_iodmount[NFS_MAXASYNCDAEMON];
 int nfs_numasync = 0;
 #define	DIRHDSIZ	(sizeof (struct dirent) - (MAXNAMLEN + 1))
 
-static int	nfsaccess_cache_timeout;
+static int	nfsaccess_cache_timeout = 2;
 SYSCTL_INT(_vfs_nfs, OID_AUTO, access_cache_timeout, CTLFLAG_RW, 
 	   &nfsaccess_cache_timeout, 0, "NFS ACCESS cache timeout");
 
@@ -261,23 +261,6 @@ SYSCTL_INT(_vfs_nfs, OID_AUTO, access_cache_fills, CTLFLAG_RD,
 	   &nfsaccess_cache_fills, 0, "NFS ACCESS cache fill count");
 
 /*
- * Compare two ucred structures, returns zero on equality, nonzero
- * otherwise.
- */
-static int
-nfsa_ucredcmp(struct ucred *c1, struct ucred *c2)
-{
-    int		i;
-    
-    if ((c1->cr_uid != c2->cr_uid) || (c1->cr_ngroups != c2->cr_ngroups))
-	return(1);
-    for (i = 0; i < c1->cr_ngroups; i++)
-	if (c1->cr_groups[i] != c2->cr_groups[i])
-	    return(1);
-    return(0);
-}
-
-/*
  * nfs access vnode op.
  * For nfs version 2, just return ok. File accesses may fail later.
  * For nfs version 3, use the access rpc to check accessibility. If file modes
@@ -352,13 +335,18 @@ nfs_access(ap)
 			wmode = mode;
 		}
 
-		/* do we have a cached result? */
+		/*
+		 * Does our cached result allow us to give a definite yes to
+		 * this request?
+		 */
 		if ((time_second < (np->n_modestamp + nfsaccess_cache_timeout)) &&
-		    !nfsa_ucredcmp(ap->a_cred, &np->n_modecred)) {
+		    (ap->a_cred->cr_uid == np->n_modeuid) &&
+		    ((np->n_mode & mode) == mode)) {
 			nfsaccess_cache_hits++;
-			if ((np->n_mode & mode) != mode)
-				error = EACCES;
 		} else {
+			/*
+			 * Either a no, or a don't know.  Go to the wire.
+			 */
 			nfsstats.rpccnt[NFSPROC_ACCESS]++;
 			nfsm_reqhead(vp, NFSPROC_ACCESS, NFSX_FH(v3) + NFSX_UNSIGNED);
 			nfsm_fhtom(vp, v3);
@@ -380,7 +368,7 @@ nfs_access(ap)
 					/* cache the result */
 					nfsaccess_cache_fills++;
 					np->n_mode = rmode;
-					np->n_modecred = *ap->a_cred;
+					np->n_modeuid = ap->a_cred->cr_uid;
 					np->n_modestamp = time_second;
 				}
 			}
@@ -700,11 +688,6 @@ nfs_setattr(ap)
 		 ap->a_p, 1)) == EINTR)
 		return (error);
 	error = nfs_setattrrpc(vp, vap, ap->a_cred, ap->a_p);
-	/* 
-	 * Attributes on server may have changed, make no assumptions about 
-	 * the server's reaction to these changes.
-	 */
-	np->n_modestamp = 0;
 	if (error && vap->va_size != VNOVAL) {
 		np->n_size = np->n_vattr.va_size = tsize;
 		vnode_pager_setsize(vp, np->n_size);
@@ -1562,7 +1545,6 @@ nfs_remove(ap)
 		error = nfs_sillyrename(dvp, vp, cnp);
 	zfree(namei_zone, cnp->cn_pnbuf);
 	np->n_attrstamp = 0;
-	np->n_modestamp = 0;
 	return (error);
 }
 
@@ -1667,18 +1649,6 @@ nfs_rename(ap)
 		cache_purge(fdvp);
 	}
 
-	/* 
-	 * We can't presume too much on the server's access control method(s),
-	 * and it may use rules based on filenames or locations.  Moving to a
-	 * more restrictive location would be harmless, but moving to a less
-	 * restrictive location you would be forced to wait for the cache
-	 * entry to time out.
-	 */
-	if (fvp->v_data)
-		VTONFS(fvp)->n_modestamp = 0;
-	if (tvp != NULL)
-		VTONFS(tvp)->n_modestamp = 0;
-
 out:
 	if (tdvp == tvp)
 		vrele(tdvp);
diff --git a/sys/nfsclient/nfsnode.h b/sys/nfsclient/nfsnode.h
index b242216..49adeb2 100644
--- a/sys/nfsclient/nfsnode.h
+++ b/sys/nfsclient/nfsnode.h
@@ -34,7 +34,7 @@
  * SUCH DAMAGE.
  *
  *	@(#)nfsnode.h	8.9 (Berkeley) 5/14/95
- * $Id: nfsnode.h,v 1.26 1998/05/31 18:32:23 peter Exp $
+ * $Id: nfsnode.h,v 1.27 1998/11/13 02:39:09 msmith Exp $
  */
 
 
@@ -94,7 +94,7 @@ struct nfsnode {
 	struct vattr		n_vattr;	/* Vnode attribute cache */
 	time_t			n_attrstamp;	/* Attr. cache timestamp */
 	u_int32_t		n_mode;		/* ACCESS mode cache */
-	struct ucred		n_modecred;	/* credentials having mode */
+	uid_t			n_modeuid;	/* credentials having mode */
 	time_t			n_modestamp;	/* mode cache timestamp */
 	time_t			n_mtime;	/* Prev modify time. */
 	time_t			n_ctime;	/* Prev create time. */