Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 3 Mar 2012 01:06:55 +0000 (UTC)
From:      Rick Macklem <rmacklem@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r232420 - in head/sys: fs/nfsclient kern nfsclient sys
Message-ID:  <201203030106.q2316t1h055748@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: rmacklem
Date: Sat Mar  3 01:06:54 2012
New Revision: 232420
URL: http://svn.freebsd.org/changeset/base/232420

Log:
  Post r230394, the Lookup RPC counts for both NFS clients increased
  significantly. Upon investigation this was caused by name cache
  misses for lookups of "..". For name cache entries for non-".."
  directories, the cache entry serves double duty. It maps both the
  named directory plus ".." for the parent of the directory. As such,
  two ctime values (one for each of the directory and its parent) need
  to be saved in the name cache entry.
  This patch adds an entry for ctime of the parent directory to the
  name cache. It also adds an additional uma zone for large entries
  with this time value, in order to minimize memory wastage.
  As well, it fixes a couple of cases where the mtime of the parent
  directory was being saved instead of ctime for positive name cache
  entries. With this patch, Lookup RPC counts return to values similar
  to pre-r230394 kernels.
  
  Reported by:	bde
  Discussed with:	kib
  Reviewed by:	jhb
  MFC after:	2 weeks

Modified:
  head/sys/fs/nfsclient/nfs_clrpcops.c
  head/sys/fs/nfsclient/nfs_clvnops.c
  head/sys/kern/vfs_cache.c
  head/sys/nfsclient/nfs_vnops.c
  head/sys/sys/vnode.h

Modified: head/sys/fs/nfsclient/nfs_clrpcops.c
==============================================================================
--- head/sys/fs/nfsclient/nfs_clrpcops.c	Sat Mar  3 01:00:18 2012	(r232419)
+++ head/sys/fs/nfsclient/nfs_clrpcops.c	Sat Mar  3 01:06:54 2012	(r232420)
@@ -2951,10 +2951,12 @@ nfsrpc_readdirplus(vnode_t vp, struct ui
 	nfsattrbit_t attrbits, dattrbits;
 	size_t tresid;
 	u_int32_t *tl2 = NULL, fakefileno = 0xffffffff, rderr;
+	struct timespec dctime;
 
 	KASSERT(uiop->uio_iovcnt == 1 &&
 	    (uio_uio_resid(uiop) & (DIRBLKSIZ - 1)) == 0,
 	    ("nfs readdirplusrpc bad uio"));
+	timespecclear(&dctime);
 	*attrflagp = 0;
 	if (eofp != NULL)
 		*eofp = 0;
@@ -2997,6 +2999,7 @@ nfsrpc_readdirplus(vnode_t vp, struct ui
 #endif
 			if (error)
 			    return (error);
+			dctime = nfsva.na_ctime;
 			dotfileid = nfsva.na_fileid;
 			NFSCL_REQSTART(nd, NFSPROC_LOOKUPP, vp);
 			NFSM_BUILD(tl, u_int32_t *, 2 * NFSX_UNSIGNED);
@@ -3134,6 +3137,8 @@ nfsrpc_readdirplus(vnode_t vp, struct ui
 				error = nd->nd_repstat;
 			goto nfsmout;
 		}
+		if ((nd->nd_flag & ND_NFSV3) != 0 && *attrflagp != 0)
+			dctime = nap->na_ctime;
 		NFSM_DISSECT(tl, u_int32_t *, 3 * NFSX_UNSIGNED);
 		NFSLOCKNODE(dnp);
 		dnp->n_cookieverf.nfsuquad[0] = *tl++;
@@ -3316,10 +3321,14 @@ nfsrpc_readdirplus(vnode_t vp, struct ui
 					vtonfs_dtype(np->n_vattr.na_type);
 				    ndp->ni_vp = newvp;
 				    NFSCNHASH(cnp, HASHINIT);
-				    if (cnp->cn_namelen <= NCHNAMLEN) {
+				    if (cnp->cn_namelen <= NCHNAMLEN &&
+					(newvp->v_type != VDIR ||
+					 dctime.tv_sec != 0)) {
 					cache_enter_time(ndp->ni_dvp,
 					    ndp->ni_vp, cnp,
-					    &nfsva.na_ctime);
+					    &nfsva.na_ctime,
+					    newvp->v_type != VDIR ? NULL :
+					    &dctime);
 				    }
 				    if (unlocknewvp)
 					vput(newvp);

Modified: head/sys/fs/nfsclient/nfs_clvnops.c
==============================================================================
--- head/sys/fs/nfsclient/nfs_clvnops.c	Sat Mar  3 01:00:18 2012	(r232419)
+++ head/sys/fs/nfsclient/nfs_clvnops.c	Sat Mar  3 01:06:54 2012	(r232420)
@@ -1172,7 +1172,7 @@ nfs_lookup(struct vop_lookup_args *ap)
 			    &dnfsva.na_mtime, ==)) {
 				mtx_unlock(&np->n_mtx);
 				cache_enter_time(dvp, NULL, cnp,
-				    &dnfsva.na_mtime);
+				    &dnfsva.na_mtime, NULL);
 			} else
 				mtx_unlock(&np->n_mtx);
 		}
@@ -1271,9 +1271,10 @@ nfs_lookup(struct vop_lookup_args *ap)
 	if (cnp->cn_nameiop != LOOKUP && (flags & ISLASTCN))
 		cnp->cn_flags |= SAVENAME;
 	if ((cnp->cn_flags & MAKEENTRY) &&
-	    (cnp->cn_nameiop != DELETE || !(flags & ISLASTCN)) && attrflag) {
-		cache_enter_time(dvp, newvp, cnp, &nfsva.na_ctime);
-	}
+	    (cnp->cn_nameiop != DELETE || !(flags & ISLASTCN)) &&
+	    attrflag != 0 && (newvp->v_type != VDIR || dattrflag != 0))
+		cache_enter_time(dvp, newvp, cnp, &nfsva.na_ctime,
+		    newvp->v_type != VDIR ? NULL : &dnfsva.na_ctime);
 	*vpp = newvp;
 	return (0);
 }
@@ -1591,7 +1592,8 @@ again:
 	}
 	if (!error) {
 		if ((cnp->cn_flags & MAKEENTRY) && attrflag)
-			cache_enter_time(dvp, newvp, cnp, &nfsva.na_ctime);
+			cache_enter_time(dvp, newvp, cnp, &nfsva.na_ctime,
+			    NULL);
 		*ap->a_vpp = newvp;
 	} else if (NFS_ISV4(dvp)) {
 		error = nfscl_maperr(cnp->cn_thread, error, vap->va_uid,
@@ -1966,8 +1968,8 @@ nfs_link(struct vop_link_args *ap)
 	 * must care about lookup caching hit rate, so...
 	 */
 	if (VFSTONFS(vp->v_mount)->nm_negnametimeo != 0 &&
-	    (cnp->cn_flags & MAKEENTRY) && dattrflag) {
-		cache_enter_time(tdvp, vp, cnp, &dnfsva.na_mtime);
+	    (cnp->cn_flags & MAKEENTRY) && attrflag != 0 && error == 0) {
+		cache_enter_time(tdvp, vp, cnp, &nfsva.na_ctime, NULL);
 	}
 	if (error && NFS_ISV4(vp))
 		error = nfscl_maperr(cnp->cn_thread, error, (uid_t)0,
@@ -2033,21 +2035,21 @@ nfs_symlink(struct vop_symlink_args *ap)
 	if (dattrflag != 0) {
 		mtx_unlock(&dnp->n_mtx);
 		(void) nfscl_loadattrcache(&dvp, &dnfsva, NULL, NULL, 0, 1);
-		/*
-		 * If negative lookup caching is enabled, I might as well
-		 * add an entry for this node. Not necessary for correctness,
-		 * but if negative caching is enabled, then the system
-		 * must care about lookup caching hit rate, so...
-		 */
-		if (VFSTONFS(dvp->v_mount)->nm_negnametimeo != 0 &&
-		    (cnp->cn_flags & MAKEENTRY)) {
-			cache_enter_time(dvp, newvp, cnp, &dnfsva.na_mtime);
-		}
 	} else {
 		dnp->n_attrstamp = 0;
 		mtx_unlock(&dnp->n_mtx);
 		KDTRACE_NFS_ATTRCACHE_FLUSH_DONE(dvp);
 	}
+	/*
+	 * If negative lookup caching is enabled, I might as well
+	 * add an entry for this node. Not necessary for correctness,
+	 * but if negative caching is enabled, then the system
+	 * must care about lookup caching hit rate, so...
+	 */
+	if (VFSTONFS(dvp->v_mount)->nm_negnametimeo != 0 &&
+	    (cnp->cn_flags & MAKEENTRY) && attrflag != 0 && error == 0) {
+		cache_enter_time(dvp, newvp, cnp, &nfsva.na_ctime, NULL);
+	}
 	return (error);
 }
 
@@ -2118,9 +2120,10 @@ nfs_mkdir(struct vop_mkdir_args *ap)
 		 * must care about lookup caching hit rate, so...
 		 */
 		if (VFSTONFS(dvp->v_mount)->nm_negnametimeo != 0 &&
-		    (cnp->cn_flags & MAKEENTRY) && dattrflag) {
-			cache_enter_time(dvp, newvp, cnp, &dnfsva.na_mtime);
-		}
+		    (cnp->cn_flags & MAKEENTRY) &&
+		    attrflag != 0 && dattrflag != 0)
+			cache_enter_time(dvp, newvp, cnp, &nfsva.na_ctime,
+			    &dnfsva.na_ctime);
 		*ap->a_vpp = newvp;
 	}
 	return (error);

Modified: head/sys/kern/vfs_cache.c
==============================================================================
--- head/sys/kern/vfs_cache.c	Sat Mar  3 01:00:18 2012	(r232419)
+++ head/sys/kern/vfs_cache.c	Sat Mar  3 01:06:54 2012	(r232420)
@@ -105,6 +105,10 @@ struct	namecache {
 /*
  * struct namecache_ts repeats struct namecache layout up to the
  * nc_nlen member.
+ * struct namecache_ts is used in place of struct namecache when time(s) need
+ * to be stored.  The nc_dotdottime field is used when a cache entry is mapping
+ * both a non-dotdot directory name plus dotdot for the directory's
+ * parent.
  */
 struct	namecache_ts {
 	LIST_ENTRY(namecache) nc_hash;	/* hash chain */
@@ -115,6 +119,7 @@ struct	namecache_ts {
 	u_char	nc_flag;		/* flag bits */
 	u_char	nc_nlen;		/* length of name */
 	struct	timespec nc_time;	/* timespec provided by fs */
+	struct	timespec nc_dotdottime;	/* dotdot timespec provided by fs */
 	int	nc_ticks;		/* ticks value when entry was added */
 	char	nc_name[0];		/* segment name + nul */
 };
@@ -125,6 +130,7 @@ struct	namecache_ts {
 #define NCF_WHITE	0x01
 #define NCF_ISDOTDOT	0x02
 #define	NCF_TS		0x04
+#define	NCF_DTS		0x08
 
 /*
  * Name caching works as follows:
@@ -190,6 +196,7 @@ RW_SYSINIT(vfscache, &cache_lock, "Name 
 static uma_zone_t cache_zone_small;
 static uma_zone_t cache_zone_small_ts;
 static uma_zone_t cache_zone_large;
+static uma_zone_t cache_zone_large_ts;
 
 #define	CACHE_PATH_CUTOFF	35
 
@@ -197,8 +204,12 @@ static struct namecache *
 cache_alloc(int len, int ts)
 {
 
-	if (len > CACHE_PATH_CUTOFF)
-		return (uma_zalloc(cache_zone_large, M_WAITOK));
+	if (len > CACHE_PATH_CUTOFF) {
+		if (ts)
+			return (uma_zalloc(cache_zone_large_ts, M_WAITOK));
+		else
+			return (uma_zalloc(cache_zone_large, M_WAITOK));
+	}
 	if (ts)
 		return (uma_zalloc(cache_zone_small_ts, M_WAITOK));
 	else
@@ -218,7 +229,9 @@ cache_free(struct namecache *ncp)
 			uma_zfree(cache_zone_small_ts, ncp);
 		else
 			uma_zfree(cache_zone_small, ncp);
-	} else
+	} else if (ts)
+		uma_zfree(cache_zone_large_ts, ncp);
+	else
 		uma_zfree(cache_zone_large, ncp);
 }
 
@@ -521,6 +534,10 @@ retry_wlocked:
 			SDT_PROBE(vfs, namecache, lookup, hit, dvp, "..",
 			    *vpp, 0, 0);
 			cache_out_ts(ncp, tsp, ticksp);
+			if ((ncp->nc_flag & (NCF_ISDOTDOT | NCF_DTS)) ==
+			    NCF_DTS && tsp != NULL)
+				*tsp = ((struct namecache_ts *)ncp)->
+				    nc_dotdottime;
 			goto success;
 		}
 	}
@@ -686,11 +703,12 @@ unlock:
  * Add an entry to the cache.
  */
 void
-cache_enter_time(dvp, vp, cnp, tsp)
+cache_enter_time(dvp, vp, cnp, tsp, dtsp)
 	struct vnode *dvp;
 	struct vnode *vp;
 	struct componentname *cnp;
 	struct timespec *tsp;
+	struct timespec *dtsp;
 {
 	struct namecache *ncp, *n2;
 	struct namecache_ts *n3;
@@ -769,6 +787,10 @@ cache_enter_time(dvp, vp, cnp, tsp)
 		n3->nc_time = *tsp;
 		n3->nc_ticks = ticks;
 		n3->nc_flag |= NCF_TS;
+		if (dtsp != NULL) {
+			n3->nc_dotdottime = *dtsp;
+			n3->nc_flag |= NCF_DTS;
+		}
 	}
 	len = ncp->nc_nlen = cnp->cn_namelen;
 	hash = fnv_32_buf(cnp->cn_nameptr, len, FNV1_32_INIT);
@@ -794,6 +816,12 @@ cache_enter_time(dvp, vp, cnp, tsp)
 				    ((struct namecache_ts *)ncp)->nc_time;
 				n3->nc_ticks =
 				    ((struct namecache_ts *)ncp)->nc_ticks;
+				if (dtsp != NULL) {
+					n3->nc_dotdottime =
+					    ((struct namecache_ts *)ncp)->
+					    nc_dotdottime;
+					n3->nc_flag |= NCF_DTS;
+				}
 			}
 			CACHE_WUNLOCK();
 			cache_free(ncp);
@@ -823,6 +851,11 @@ cache_enter_time(dvp, vp, cnp, tsp)
 			ncp->nc_flag |= NCF_WHITE;
 	} else if (vp->v_type == VDIR) {
 		if (flag != NCF_ISDOTDOT) {
+			/*
+			 * For this case, the cache entry maps both the
+			 * directory name in it and the name ".." for the
+			 * directory's parent.
+			 */
 			if ((n2 = vp->v_cache_dd) != NULL &&
 			    (n2->nc_flag & NCF_ISDOTDOT) != 0)
 				cache_zap(n2);
@@ -886,6 +919,9 @@ nchinit(void *dummy __unused)
 	    sizeof(struct namecache_ts) + CACHE_PATH_CUTOFF + 1,
 	    NULL, NULL, NULL, NULL, UMA_ALIGN_PTR, UMA_ZONE_ZINIT);
 	cache_zone_large = uma_zcreate("L VFS Cache",
+	    sizeof(struct namecache) + NAME_MAX + 1,
+	    NULL, NULL, NULL, NULL, UMA_ALIGN_PTR, UMA_ZONE_ZINIT);
+	cache_zone_large_ts = uma_zcreate("LTS VFS Cache",
 	    sizeof(struct namecache_ts) + NAME_MAX + 1,
 	    NULL, NULL, NULL, NULL, UMA_ALIGN_PTR, UMA_ZONE_ZINIT);
 
@@ -1382,7 +1418,7 @@ void
 cache_enter(struct vnode *dvp, struct vnode *vp, struct componentname *cnp)
 {
 
-	cache_enter_time(dvp, vp, cnp, NULL);
+	cache_enter_time(dvp, vp, cnp, NULL, NULL);
 }
 
 /*

Modified: head/sys/nfsclient/nfs_vnops.c
==============================================================================
--- head/sys/nfsclient/nfs_vnops.c	Sat Mar  3 01:00:18 2012	(r232419)
+++ head/sys/nfsclient/nfs_vnops.c	Sat Mar  3 01:06:54 2012	(r232420)
@@ -912,7 +912,7 @@ nfs_lookup(struct vop_lookup_args *ap)
 	struct vnode *dvp = ap->a_dvp;
 	struct vnode **vpp = ap->a_vpp;
 	struct mount *mp = dvp->v_mount;
-	struct vattr vattr;
+	struct vattr dvattr, vattr;
 	struct timespec nctime;
 	int flags = cnp->cn_flags;
 	struct vnode *newvp;
@@ -1129,7 +1129,7 @@ nfs_lookup(struct vop_lookup_args *ap)
 	}
 	if (v3) {
 		nfsm_postop_attr_va(newvp, attrflag, &vattr);
-		nfsm_postop_attr(dvp, dattrflag);
+		nfsm_postop_attr_va(dvp, dattrflag, &dvattr);
 	} else {
 		nfsm_loadattr(newvp, &vattr);
 		attrflag = 1;
@@ -1137,9 +1137,10 @@ nfs_lookup(struct vop_lookup_args *ap)
 	if (cnp->cn_nameiop != LOOKUP && (flags & ISLASTCN))
 		cnp->cn_flags |= SAVENAME;
 	if ((cnp->cn_flags & MAKEENTRY) &&
-	    (cnp->cn_nameiop != DELETE || !(flags & ISLASTCN)) && attrflag) {
-		cache_enter_time(dvp, newvp, cnp, &vattr.va_ctime);
-	}
+	    (cnp->cn_nameiop != DELETE || !(flags & ISLASTCN)) &&
+	    attrflag != 0 && (newvp->v_type != VDIR || dattrflag != 0))
+		cache_enter_time(dvp, newvp, cnp, &vattr.va_ctime,
+		    newvp->v_type != VDIR ? NULL : &dvattr.va_ctime);
 	*vpp = newvp;
 	m_freem(mrep);
 nfsmout:
@@ -1181,7 +1182,7 @@ nfsmout:
 			    &vattr.va_mtime, ==)) {
 				mtx_unlock(&np->n_mtx);
 				cache_enter_time(dvp, NULL, cnp,
-				    &vattr.va_mtime);
+				    &vattr.va_mtime, NULL);
 			} else
 				mtx_unlock(&np->n_mtx);
 		}
@@ -2463,11 +2464,11 @@ nfs_readdirplusrpc(struct vnode *vp, str
 	nfsuint64 cookie;
 	struct nfsmount *nmp = VFSTONFS(vp->v_mount);
 	struct nfsnode *dnp = VTONFS(vp), *np;
-	struct vattr vattr;
+	struct vattr vattr, dvattr;
 	nfsfh_t *fhp;
 	u_quad_t fileno;
 	int error = 0, tlen, more_dirs = 1, blksiz = 0, doit, bigenough = 1, i;
-	int attrflag, fhsize;
+	int attrflag, dattrflag, fhsize;
 
 #ifndef nolint
 	dp = NULL;
@@ -2513,7 +2514,7 @@ nfs_readdirplusrpc(struct vnode *vp, str
 		*tl++ = txdr_unsigned(nmp->nm_readdirsize);
 		*tl = txdr_unsigned(nmp->nm_rsize);
 		nfsm_request(vp, NFSPROC_READDIRPLUS, uiop->uio_td, cred);
-		nfsm_postop_attr(vp, attrflag);
+		nfsm_postop_attr_va(vp, dattrflag, &dvattr);
 		if (error) {
 			m_freem(mrep);
 			goto nfsmout;
@@ -2649,8 +2650,11 @@ nfs_readdirplusrpc(struct vnode *vp, str
 				md = mdsav2;
 				dp->d_type = IFTODT(VTTOIF(vattr.va_type));
 				ndp->ni_vp = newvp;
-			        cache_enter_time(ndp->ni_dvp, ndp->ni_vp, cnp,
-				    &vattr.va_ctime);
+				if (newvp->v_type != VDIR || dattrflag != 0)
+				    cache_enter_time(ndp->ni_dvp, ndp->ni_vp,
+					cnp, &vattr.va_ctime,
+					newvp->v_type != VDIR ? NULL :
+					&dvattr.va_ctime);
 			    }
 			} else {
 			    /* Just skip over the file handle */

Modified: head/sys/sys/vnode.h
==============================================================================
--- head/sys/sys/vnode.h	Sat Mar  3 01:00:18 2012	(r232419)
+++ head/sys/sys/vnode.h	Sat Mar  3 01:06:54 2012	(r232420)
@@ -583,9 +583,10 @@ struct vnode;
 
 /* cache_* may belong in namei.h. */
 #define	cache_enter(dvp, vp, cnp)					\
-	cache_enter_time(dvp, vp, cnp, NULL)
+	cache_enter_time(dvp, vp, cnp, NULL, NULL)
 void	cache_enter_time(struct vnode *dvp, struct vnode *vp,
-	    struct componentname *cnp, struct timespec *tsp);
+	    struct componentname *cnp, struct timespec *tsp,
+	    struct timespec *dtsp);
 int	cache_lookup(struct vnode *dvp, struct vnode **vpp,
 	    struct componentname *cnp, struct timespec *tsp, int *ticksp);
 void	cache_purge(struct vnode *vp);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201203030106.q2316t1h055748>