Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 21 Jan 2016 01:04:03 +0000 (UTC)
From:      Mateusz Guzik <mjg@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r294475 - head/sys/kern
Message-ID:  <201601210104.u0L143tP098546@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: mjg
Date: Thu Jan 21 01:04:03 2016
New Revision: 294475
URL: https://svnweb.freebsd.org/changeset/base/294475

Log:
  cache: use counter(9) API to maintain statistics
  
  Previously the code would just increment statistics while only holding a
  shared lock, in effect losing updates.
  
  Separate tracking for nchstats is removed as values can be obtained from
  existing counters. Note that some fields are updated by external
  consumers and are left unfixed. This should not be a serious issue as
  this structure looks quite obsolete.
  
  No strong objections: kib

Modified:
  head/sys/kern/vfs_cache.c

Modified: head/sys/kern/vfs_cache.c
==============================================================================
--- head/sys/kern/vfs_cache.c	Thu Jan 21 00:42:48 2016	(r294474)
+++ head/sys/kern/vfs_cache.c	Thu Jan 21 01:04:03 2016	(r294475)
@@ -39,6 +39,7 @@ __FBSDID("$FreeBSD$");
 
 #include <sys/param.h>
 #include <sys/systm.h>
+#include <sys/counter.h>
 #include <sys/filedesc.h>
 #include <sys/fnv_hash.h>
 #include <sys/kernel.h>
@@ -272,37 +273,35 @@ SYSCTL_INT(_debug_sizeof, OID_AUTO, name
  */
 static SYSCTL_NODE(_vfs, OID_AUTO, cache, CTLFLAG_RW, 0,
     "Name cache statistics");
-#define STATNODE(mode, name, var, descr) \
-	SYSCTL_ULONG(_vfs_cache, OID_AUTO, name, mode, var, 0, descr);
-STATNODE(CTLFLAG_RD, numneg, &numneg, "Number of negative cache entries");
-STATNODE(CTLFLAG_RD, numcache, &numcache, "Number of cache entries");
-static u_long numcalls; STATNODE(CTLFLAG_RD, numcalls, &numcalls,
-    "Number of cache lookups");
-static u_long dothits; STATNODE(CTLFLAG_RD, dothits, &dothits,
-    "Number of '.' hits");
-static u_long dotdothits; STATNODE(CTLFLAG_RD, dotdothits, &dotdothits,
-    "Number of '..' hits");
-static u_long numchecks; STATNODE(CTLFLAG_RD, numchecks, &numchecks,
-    "Number of checks in lookup");
-static u_long nummiss; STATNODE(CTLFLAG_RD, nummiss, &nummiss,
-    "Number of cache misses");
-static u_long nummisszap; STATNODE(CTLFLAG_RD, nummisszap, &nummisszap,
-    "Number of cache misses we do not want to cache");
-static u_long numposzaps; STATNODE(CTLFLAG_RD, numposzaps, &numposzaps,
+#define STATNODE_ULONG(name, descr)	\
+	SYSCTL_ULONG(_vfs_cache, OID_AUTO, name, CTLFLAG_RD, &name, 0, descr);
+#define STATNODE_COUNTER(name, descr)	\
+	static counter_u64_t name;	\
+	SYSCTL_COUNTER_U64(_vfs_cache, OID_AUTO, name, CTLFLAG_RD, &name, descr);
+STATNODE_ULONG(numneg, "Number of negative cache entries");
+STATNODE_ULONG(numcache, "Number of cache entries");
+STATNODE_COUNTER(numcalls, "Number of cache lookups");
+STATNODE_COUNTER(dothits, "Number of '.' hits");
+STATNODE_COUNTER(dotdothits, "Number of '..' hits");
+STATNODE_COUNTER(numchecks, "Number of checks in lookup");
+STATNODE_COUNTER(nummiss, "Number of cache misses");
+STATNODE_COUNTER(nummisszap, "Number of cache misses we do not want to cache");
+STATNODE_COUNTER(numposzaps,
     "Number of cache hits (positive) we do not want to cache");
-static u_long numposhits; STATNODE(CTLFLAG_RD, numposhits, &numposhits,
-    "Number of cache hits (positive)");
-static u_long numnegzaps; STATNODE(CTLFLAG_RD, numnegzaps, &numnegzaps,
+STATNODE_COUNTER(numposhits, "Number of cache hits (positive)");
+STATNODE_COUNTER(numnegzaps,
     "Number of cache hits (negative) we do not want to cache");
-static u_long numneghits; STATNODE(CTLFLAG_RD, numneghits, &numneghits,
-    "Number of cache hits (negative)");
-static u_long numupgrades; STATNODE(CTLFLAG_RD, numupgrades, &numupgrades,
+STATNODE_COUNTER(numneghits, "Number of cache hits (negative)");
+/* These count for kern___getcwd(), too. */
+STATNODE_COUNTER(numfullpathcalls, "Number of fullpath search calls");
+STATNODE_COUNTER(numfullpathfail1, "Number of fullpath search errors (ENOTDIR)");
+STATNODE_COUNTER(numfullpathfail2,
+    "Number of fullpath search errors (VOP_VPTOCNP failures)");
+STATNODE_COUNTER(numfullpathfail4, "Number of fullpath search errors (ENOMEM)");
+STATNODE_COUNTER(numfullpathfound, "Number of successful fullpath calls");
+static long numupgrades; STATNODE_ULONG(numupgrades,
     "Number of updates of the cache after lookup (write lock + retry)");
 
-SYSCTL_OPAQUE(_vfs_cache, OID_AUTO, nchstats, CTLFLAG_RD | CTLFLAG_MPSAFE,
-    &nchstats, sizeof(nchstats), "LU",
-    "VFS cache effectiveness statistics");
-
 static void cache_zap(struct namecache *ncp);
 static int vn_vptocnp_locked(struct vnode **vp, struct ucred *cred, char *buf,
     u_int *buflen);
@@ -311,6 +310,28 @@ static int vn_fullpath1(struct thread *t
 
 static MALLOC_DEFINE(M_VFSCACHE, "vfscache", "VFS name cache entries");
 
+static int
+sysctl_nchstats(SYSCTL_HANDLER_ARGS)
+{
+	struct nchstats snap;
+
+	if (req->oldptr == NULL)
+		return (SYSCTL_OUT(req, 0, sizeof(snap)));
+
+	snap = nchstats;
+	snap.ncs_goodhits = counter_u64_fetch(numposhits);
+	snap.ncs_neghits = counter_u64_fetch(numneghits);
+	snap.ncs_badhits = counter_u64_fetch(numposzaps) +
+	    counter_u64_fetch(numnegzaps);
+	snap.ncs_miss = counter_u64_fetch(nummisszap) +
+	    counter_u64_fetch(nummiss);
+
+	return (SYSCTL_OUT(req, &snap, sizeof(snap)));
+}
+SYSCTL_PROC(_vfs_cache, OID_AUTO, nchstats, CTLTYPE_OPAQUE | CTLFLAG_RD |
+    CTLFLAG_MPSAFE, 0, 0, sysctl_nchstats, "LU",
+    "VFS cache effectiveness statistics");
+
 #ifdef DIAGNOSTIC
 /*
  * Grab an atomic snapshot of the name cache hash chain lengths
@@ -479,7 +500,7 @@ cache_lookup(struct vnode *dvp, struct v
 retry:
 	CACHE_RLOCK();
 	wlocked = 0;
-	numcalls++;
+	counter_u64_add(numcalls, 1);
 	error = 0;
 
 retry_wlocked:
@@ -488,7 +509,7 @@ retry_wlocked:
 			*vpp = dvp;
 			CTR2(KTR_VFS, "cache_lookup(%p, %s) found via .",
 			    dvp, cnp->cn_nameptr);
-			dothits++;
+			counter_u64_add(dothits, 1);
 			SDT_PROBE3(vfs, namecache, lookup, hit, dvp, ".", *vpp);
 			if (tsp != NULL)
 				timespecclear(tsp);
@@ -497,7 +518,7 @@ retry_wlocked:
 			goto success;
 		}
 		if (cnp->cn_namelen == 2 && cnp->cn_nameptr[1] == '.') {
-			dotdothits++;
+			counter_u64_add(dotdothits, 1);
 			if (dvp->v_cache_dd == NULL) {
 				SDT_PROBE3(vfs, namecache, lookup, miss, dvp,
 				    "..", NULL);
@@ -536,7 +557,7 @@ retry_wlocked:
 	hash = fnv_32_buf(cnp->cn_nameptr, cnp->cn_namelen, FNV1_32_INIT);
 	hash = fnv_32_buf(&dvp, sizeof(dvp), hash);
 	LIST_FOREACH(ncp, (NCHHASH(hash)), nc_hash) {
-		numchecks++;
+		counter_u64_add(numchecks, 1);
 		if (ncp->nc_dvp == dvp && ncp->nc_nlen == cnp->cn_namelen &&
 		    !bcmp(nc_get_name(ncp), cnp->cn_nameptr, ncp->nc_nlen))
 			break;
@@ -547,18 +568,16 @@ retry_wlocked:
 		SDT_PROBE3(vfs, namecache, lookup, miss, dvp, cnp->cn_nameptr,
 		    NULL);
 		if ((cnp->cn_flags & MAKEENTRY) == 0) {
-			nummisszap++;
+			counter_u64_add(nummisszap, 1);
 		} else {
-			nummiss++;
+			counter_u64_add(nummiss, 1);
 		}
-		nchstats.ncs_miss++;
 		goto unlock;
 	}
 
 	/* We don't want to have an entry, so dump it */
 	if ((cnp->cn_flags & MAKEENTRY) == 0) {
-		numposzaps++;
-		nchstats.ncs_badhits++;
+		counter_u64_add(numposzaps, 1);
 		if (!wlocked && !CACHE_UPGRADE_LOCK())
 			goto wlock;
 		cache_zap(ncp);
@@ -568,8 +587,7 @@ retry_wlocked:
 
 	/* We found a "positive" match, return the vnode */
 	if (ncp->nc_vp) {
-		numposhits++;
-		nchstats.ncs_goodhits++;
+		counter_u64_add(numposhits, 1);
 		*vpp = ncp->nc_vp;
 		CTR4(KTR_VFS, "cache_lookup(%p, %s) found %p via ncp %p",
 		    dvp, cnp->cn_nameptr, *vpp, ncp);
@@ -582,8 +600,7 @@ retry_wlocked:
 negative_success:
 	/* We found a negative match, and want to create it, so purge */
 	if (cnp->cn_nameiop == CREATE) {
-		numnegzaps++;
-		nchstats.ncs_badhits++;
+		counter_u64_add(numnegzaps, 1);
 		if (!wlocked && !CACHE_UPGRADE_LOCK())
 			goto wlock;
 		cache_zap(ncp);
@@ -593,7 +610,7 @@ negative_success:
 
 	if (!wlocked && !CACHE_UPGRADE_LOCK())
 		goto wlock;
-	numneghits++;
+	counter_u64_add(numneghits, 1);
 	/*
 	 * We found a "negative" match, so we shift it to the end of
 	 * the "negative" cache entries queue to satisfy LRU.  Also,
@@ -602,7 +619,6 @@ negative_success:
 	 */
 	TAILQ_REMOVE(&ncneg, ncp, nc_dst);
 	TAILQ_INSERT_TAIL(&ncneg, ncp, nc_dst);
-	nchstats.ncs_neghits++;
 	if (ncp->nc_flag & NCF_WHITE)
 		cnp->cn_flags |= ISWHITEOUT;
 	SDT_PROBE2(vfs, namecache, lookup, hit__negative, dvp,
@@ -918,6 +934,22 @@ nchinit(void *dummy __unused)
 	    NULL, NULL, NULL, NULL, UMA_ALIGN_PTR, UMA_ZONE_ZINIT);
 
 	nchashtbl = hashinit(desiredvnodes * 2, M_VFSCACHE, &nchash);
+
+	numcalls = counter_u64_alloc(M_WAITOK);
+	dothits = counter_u64_alloc(M_WAITOK);
+	dotdothits = counter_u64_alloc(M_WAITOK);
+	numchecks = counter_u64_alloc(M_WAITOK);
+	nummiss = counter_u64_alloc(M_WAITOK);
+	nummisszap = counter_u64_alloc(M_WAITOK);
+	numposzaps = counter_u64_alloc(M_WAITOK);
+	numposhits = counter_u64_alloc(M_WAITOK);
+	numnegzaps = counter_u64_alloc(M_WAITOK);
+	numneghits = counter_u64_alloc(M_WAITOK);
+	numfullpathcalls = counter_u64_alloc(M_WAITOK);
+	numfullpathfail1 = counter_u64_alloc(M_WAITOK);
+	numfullpathfail2 = counter_u64_alloc(M_WAITOK);
+	numfullpathfail4 = counter_u64_alloc(M_WAITOK);
+	numfullpathfound = counter_u64_alloc(M_WAITOK);
 }
 SYSINIT(vfs, SI_SUB_VFS, SI_ORDER_SECOND, nchinit, NULL);
 
@@ -1122,23 +1154,10 @@ kern___getcwd(struct thread *td, char *b
  * Thus begins the fullpath magic.
  */
 
-#undef STATNODE
-#define STATNODE(name, descr)						\
-	static u_int name;						\
-	SYSCTL_UINT(_vfs_cache, OID_AUTO, name, CTLFLAG_RD, &name, 0, descr)
-
 static int disablefullpath;
 SYSCTL_INT(_debug, OID_AUTO, disablefullpath, CTLFLAG_RW, &disablefullpath, 0,
     "Disable the vn_fullpath function");
 
-/* These count for kern___getcwd(), too. */
-STATNODE(numfullpathcalls, "Number of fullpath search calls");
-STATNODE(numfullpathfail1, "Number of fullpath search errors (ENOTDIR)");
-STATNODE(numfullpathfail2,
-    "Number of fullpath search errors (VOP_VPTOCNP failures)");
-STATNODE(numfullpathfail4, "Number of fullpath search errors (ENOMEM)");
-STATNODE(numfullpathfound, "Number of successful fullpath calls");
-
 /*
  * Retrieve the full filesystem path that correspond to a vnode from the name
  * cache (if available)
@@ -1226,7 +1245,7 @@ vn_vptocnp_locked(struct vnode **vp, str
 		if (*buflen < ncp->nc_nlen) {
 			CACHE_RUNLOCK();
 			vrele(*vp);
-			numfullpathfail4++;
+			counter_u64_add(numfullpathfail4, 1);
 			error = ENOMEM;
 			SDT_PROBE3(vfs, namecache, fullpath, return, error,
 			    vp, NULL);
@@ -1251,7 +1270,7 @@ vn_vptocnp_locked(struct vnode **vp, str
 	error = VOP_VPTOCNP(*vp, &dvp, cred, buf, buflen);
 	vput(*vp);
 	if (error) {
-		numfullpathfail2++;
+		counter_u64_add(numfullpathfail2, 1);
 		SDT_PROBE3(vfs, namecache, fullpath, return,  error, vp, NULL);
 		return (error);
 	}
@@ -1292,7 +1311,7 @@ vn_fullpath1(struct thread *td, struct v
 	slash_prefixed = 0;
 
 	SDT_PROBE1(vfs, namecache, fullpath, entry, vp);
-	numfullpathcalls++;
+	counter_u64_add(numfullpathcalls, 1);
 	vref(vp);
 	CACHE_RLOCK();
 	if (vp->v_type != VDIR) {
@@ -1328,7 +1347,7 @@ vn_fullpath1(struct thread *td, struct v
 		if (vp->v_type != VDIR) {
 			CACHE_RUNLOCK();
 			vrele(vp);
-			numfullpathfail1++;
+			counter_u64_add(numfullpathfail1, 1);
 			error = ENOTDIR;
 			SDT_PROBE3(vfs, namecache, fullpath, return,
 			    error, vp, NULL);
@@ -1354,14 +1373,14 @@ vn_fullpath1(struct thread *td, struct v
 		if (buflen == 0) {
 			CACHE_RUNLOCK();
 			vrele(vp);
-			numfullpathfail4++;
+			counter_u64_add(numfullpathfail4, 1);
 			SDT_PROBE3(vfs, namecache, fullpath, return, ENOMEM,
 			    startvp, NULL);
 			return (ENOMEM);
 		}
 		buf[--buflen] = '/';
 	}
-	numfullpathfound++;
+	counter_u64_add(numfullpathfound, 1);
 	CACHE_RUNLOCK();
 	vrele(vp);
 



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