svn commit: r364857 - head/sys/kern

Mateusz Guzik mjg at FreeBSD.org
Thu Aug 27 06:31:28 UTC 2020


Author: mjg
Date: Thu Aug 27 06:31:27 2020
New Revision: 364857
URL: https://svnweb.freebsd.org/changeset/base/364857

Log:
  cache: assorted clean ups
  
  In particular remove spurious comments, duplicate assertions and the
  inconsistently done KTR support.

Modified:
  head/sys/kern/vfs_cache.c

Modified: head/sys/kern/vfs_cache.c
==============================================================================
--- head/sys/kern/vfs_cache.c	Thu Aug 27 06:30:40 2020	(r364856)
+++ head/sys/kern/vfs_cache.c	Thu Aug 27 06:31:27 2020	(r364857)
@@ -999,9 +999,6 @@ cache_zap_locked(struct namecache *ncp)
 	cache_assert_vnode_locked(ncp->nc_dvp);
 	cache_assert_bucket_locked(ncp);
 
-	CTR2(KTR_VFS, "cache_zap(%p) vp %p", ncp,
-	    (ncp->nc_flag & NCF_NEGATIVE) ? NULL : ncp->nc_vp);
-
 	cache_ncp_invalidate(ncp);
 
 	ncpp = NCP2BUCKET(ncp);
@@ -1260,6 +1257,83 @@ cache_zap_locked_bucket_kl(struct namecache *ncp, stru
 	return (EAGAIN);
 }
 
+static __noinline int
+cache_remove_cnp(struct vnode *dvp, struct componentname *cnp)
+{
+	struct namecache *ncp;
+	struct mtx *blp;
+	struct mtx *dvlp, *dvlp2;
+	uint32_t hash;
+	int error;
+
+	if (cnp->cn_namelen == 2 &&
+	    cnp->cn_nameptr[0] == '.' && cnp->cn_nameptr[1] == '.') {
+		dvlp = VP2VNODELOCK(dvp);
+		dvlp2 = NULL;
+		mtx_lock(dvlp);
+retry_dotdot:
+		ncp = dvp->v_cache_dd;
+		if (ncp == NULL) {
+			mtx_unlock(dvlp);
+			if (dvlp2 != NULL)
+				mtx_unlock(dvlp2);
+			SDT_PROBE2(vfs, namecache, removecnp, miss, dvp, cnp);
+			return (0);
+		}
+		if ((ncp->nc_flag & NCF_ISDOTDOT) != 0) {
+			if (!cache_zap_locked_vnode_kl2(ncp, dvp, &dvlp2))
+				goto retry_dotdot;
+			MPASS(dvp->v_cache_dd == NULL);
+			mtx_unlock(dvlp);
+			if (dvlp2 != NULL)
+				mtx_unlock(dvlp2);
+			cache_free(ncp);
+		} else {
+			vn_seqc_write_begin(dvp);
+			dvp->v_cache_dd = NULL;
+			vn_seqc_write_end(dvp);
+			mtx_unlock(dvlp);
+			if (dvlp2 != NULL)
+				mtx_unlock(dvlp2);
+		}
+		SDT_PROBE2(vfs, namecache, removecnp, hit, dvp, cnp);
+		return (1);
+	}
+
+	hash = cache_get_hash(cnp->cn_nameptr, cnp->cn_namelen, dvp);
+	blp = HASH2BUCKETLOCK(hash);
+retry:
+	if (CK_SLIST_EMPTY(NCHHASH(hash)))
+		goto out_no_entry;
+
+	mtx_lock(blp);
+
+	CK_SLIST_FOREACH(ncp, (NCHHASH(hash)), nc_hash) {
+		if (ncp->nc_dvp == dvp && ncp->nc_nlen == cnp->cn_namelen &&
+		    !bcmp(ncp->nc_name, cnp->cn_nameptr, ncp->nc_nlen))
+			break;
+	}
+
+	if (ncp == NULL) {
+		mtx_unlock(blp);
+		goto out_no_entry;
+	}
+
+	error = cache_zap_locked_bucket(ncp, cnp, hash, blp);
+	if (__predict_false(error != 0)) {
+		zap_and_exit_bucket_fail++;
+		goto retry;
+	}
+	counter_u64_add(numposzaps, 1);
+	SDT_PROBE2(vfs, namecache, removecnp, hit, dvp, cnp);
+	cache_free(ncp);
+	return (1);
+out_no_entry:
+	counter_u64_add(nummisszap, 1);
+	SDT_PROBE2(vfs, namecache, removecnp, miss, dvp, cnp);
+	return (0);
+}
+
 static int __noinline
 cache_lookup_dot(struct vnode *dvp, struct vnode **vpp, struct componentname *cnp,
     struct timespec *tsp, int *ticksp)
@@ -1267,8 +1341,6 @@ cache_lookup_dot(struct vnode *dvp, struct vnode **vpp
 	int ltype;
 
 	*vpp = dvp;
-	CTR2(KTR_VFS, "cache_lookup(%p, %s) found via .",
-			dvp, cnp->cn_nameptr);
 	counter_u64_add(dothits, 1);
 	SDT_PROBE3(vfs, namecache, lookup, hit, dvp, ".", *vpp);
 	if (tsp != NULL)
@@ -1296,10 +1368,6 @@ cache_lookup_dot(struct vnode *dvp, struct vnode **vpp
 	return (-1);
 }
 
-static __noinline int
-cache_remove_cnp(struct vnode *dvp, struct componentname *cnp);
-
-
 static int __noinline
 cache_lookup_dotdot(struct vnode *dvp, struct vnode **vpp, struct componentname *cnp,
     struct timespec *tsp, int *ticksp)
@@ -1324,8 +1392,7 @@ retry:
 	mtx_lock(dvlp);
 	ncp = dvp->v_cache_dd;
 	if (ncp == NULL) {
-		SDT_PROBE3(vfs, namecache, lookup, miss, dvp,
-		    "..", NULL);
+		SDT_PROBE3(vfs, namecache, lookup, miss, dvp, "..", NULL);
 		mtx_unlock(dvlp);
 		return (0);
 	}
@@ -1336,13 +1403,9 @@ retry:
 			*vpp = ncp->nc_vp;
 	} else
 		*vpp = ncp->nc_dvp;
-	/* Return failure if negative entry was found. */
 	if (*vpp == NULL)
 		goto negative_success;
-	CTR3(KTR_VFS, "cache_lookup(%p, %s) found %p via ..",
-	    dvp, cnp->cn_nameptr, *vpp);
-	SDT_PROBE3(vfs, namecache, lookup, hit, dvp, "..",
-	    *vpp);
+	SDT_PROBE3(vfs, namecache, lookup, hit, dvp, "..", *vpp);
 	cache_out_ts(ncp, tsp, ticksp);
 	if ((ncp->nc_flag & (NCF_ISDOTDOT | NCF_DTS)) ==
 	    NCF_DTS && tsp != NULL) {
@@ -1350,12 +1413,7 @@ retry:
 		*tsp = ncp_ts->nc_dotdottime;
 	}
 
-	/*
-	 * On success we return a locked and ref'd vnode as per the lookup
-	 * protocol.
-	 */
 	MPASS(dvp != *vpp);
-	ltype = 0;	/* silence gcc warning */
 	ltype = VOP_ISLOCKED(dvp);
 	VOP_UNLOCK(dvp);
 	vs = vget_prep(*vpp);
@@ -1372,10 +1430,6 @@ retry:
 		*vpp = NULL;
 		goto retry;
 	}
-	if ((cnp->cn_flags & ISLASTCN) &&
-	    (cnp->cn_lkflags & LK_TYPE_MASK) == LK_EXCLUSIVE) {
-		ASSERT_VOP_ELOCKED(*vpp, "cache_lookup");
-	}
 	return (-1);
 negative_success:
 	if (__predict_false(cnp->cn_nameiop == CREATE)) {
@@ -1402,87 +1456,6 @@ negative_success:
 	return (ENOENT);
 }
 
-static __noinline int
-cache_remove_cnp(struct vnode *dvp, struct componentname *cnp)
-{
-	struct namecache *ncp;
-	struct mtx *blp;
-	struct mtx *dvlp, *dvlp2;
-	uint32_t hash;
-	int error;
-
-	if (cnp->cn_namelen == 2 &&
-	    cnp->cn_nameptr[0] == '.' && cnp->cn_nameptr[1] == '.') {
-		dvlp = VP2VNODELOCK(dvp);
-		dvlp2 = NULL;
-		mtx_lock(dvlp);
-retry_dotdot:
-		ncp = dvp->v_cache_dd;
-		if (ncp == NULL) {
-			mtx_unlock(dvlp);
-			if (dvlp2 != NULL)
-				mtx_unlock(dvlp2);
-			SDT_PROBE2(vfs, namecache, removecnp, miss, dvp, cnp);
-			return (0);
-		}
-		if ((ncp->nc_flag & NCF_ISDOTDOT) != 0) {
-			if (ncp->nc_dvp != dvp)
-				panic("dvp %p v_cache_dd %p\n", dvp, ncp);
-			if (!cache_zap_locked_vnode_kl2(ncp,
-			    dvp, &dvlp2))
-				goto retry_dotdot;
-			MPASS(dvp->v_cache_dd == NULL);
-			mtx_unlock(dvlp);
-			if (dvlp2 != NULL)
-				mtx_unlock(dvlp2);
-			cache_free(ncp);
-		} else {
-			vn_seqc_write_begin(dvp);
-			dvp->v_cache_dd = NULL;
-			vn_seqc_write_end(dvp);
-			mtx_unlock(dvlp);
-			if (dvlp2 != NULL)
-				mtx_unlock(dvlp2);
-		}
-		SDT_PROBE2(vfs, namecache, removecnp, hit, dvp, cnp);
-		return (1);
-	}
-
-	hash = cache_get_hash(cnp->cn_nameptr, cnp->cn_namelen, dvp);
-	blp = HASH2BUCKETLOCK(hash);
-retry:
-	if (CK_SLIST_EMPTY(NCHHASH(hash)))
-		goto out_no_entry;
-
-	mtx_lock(blp);
-
-	CK_SLIST_FOREACH(ncp, (NCHHASH(hash)), nc_hash) {
-		if (ncp->nc_dvp == dvp && ncp->nc_nlen == cnp->cn_namelen &&
-		    !bcmp(ncp->nc_name, cnp->cn_nameptr, ncp->nc_nlen))
-			break;
-	}
-
-	/* We failed to find an entry */
-	if (ncp == NULL) {
-		mtx_unlock(blp);
-		goto out_no_entry;
-	}
-
-	error = cache_zap_locked_bucket(ncp, cnp, hash, blp);
-	if (__predict_false(error != 0)) {
-		zap_and_exit_bucket_fail++;
-		goto retry;
-	}
-	counter_u64_add(numposzaps, 1);
-	cache_free(ncp);
-	SDT_PROBE2(vfs, namecache, removecnp, hit, dvp, cnp);
-	return (1);
-out_no_entry:
-	SDT_PROBE2(vfs, namecache, removecnp, miss, dvp, cnp);
-	counter_u64_add(nummisszap, 1);
-	return (0);
-}
-
 /**
  * Lookup a name in the name cache
  *
@@ -1504,6 +1477,8 @@ out_no_entry:
  *   		that was current when the cache entry was created, unless cnp
  *   		was ".".
  *
+ * Either both tsp and ticks have to be provided or neither of them.
+ *
  * # Returns
  *
  * - -1:	A positive cache hit.  vpp will contain the desired vnode.
@@ -1543,7 +1518,6 @@ retry:
 			break;
 	}
 
-	/* We failed to find an entry */
 	if (__predict_false(ncp == NULL)) {
 		mtx_unlock(blp);
 		SDT_PROBE3(vfs, namecache, lookup, miss, dvp, cnp->cn_nameptr,
@@ -1555,18 +1529,10 @@ retry:
 	if (ncp->nc_flag & NCF_NEGATIVE)
 		goto negative_success;
 
-	/* We found a "positive" match, return the vnode */
 	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);
-	SDT_PROBE3(vfs, namecache, lookup, hit, dvp, ncp->nc_name,
-	    *vpp);
+	SDT_PROBE3(vfs, namecache, lookup, hit, dvp, ncp->nc_name, *vpp);
 	cache_out_ts(ncp, tsp, ticksp);
-	/*
-	 * On success we return a locked and ref'd vnode as per the lookup
-	 * protocol.
-	 */
 	MPASS(dvp != *vpp);
 	vs = vget_prep(*vpp);
 	mtx_unlock(blp);
@@ -1575,14 +1541,8 @@ retry:
 		*vpp = NULL;
 		goto retry;
 	}
-	if ((cnp->cn_flags & ISLASTCN) &&
-	    (cnp->cn_lkflags & LK_TYPE_MASK) == LK_EXCLUSIVE) {
-		ASSERT_VOP_ELOCKED(*vpp, "cache_lookup");
-	}
 	return (-1);
-
 negative_success:
-	/* We found a negative match, and want to create it, so purge */
 	if (__predict_false(cnp->cn_nameiop == CREATE)) {
 		if (cnp->cn_flags & ISLASTCN) {
 			counter_u64_add(numnegzaps, 1);
@@ -1651,7 +1611,6 @@ cache_lookup(struct vnode *dvp, struct vnode **vpp, st
 			break;
 	}
 
-	/* We failed to find an entry */
 	if (__predict_false(ncp == NULL)) {
 		vfs_smr_exit();
 		SDT_PROBE3(vfs, namecache, lookup, miss, dvp, cnp->cn_nameptr,
@@ -1664,18 +1623,10 @@ cache_lookup(struct vnode *dvp, struct vnode **vpp, st
 	if (nc_flag & NCF_NEGATIVE)
 		goto negative_success;
 
-	/* We found a "positive" match, return the vnode */
 	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);
-	SDT_PROBE3(vfs, namecache, lookup, hit, dvp, ncp->nc_name,
-	    *vpp);
+	SDT_PROBE3(vfs, namecache, lookup, hit, dvp, ncp->nc_name, *vpp);
 	cache_out_ts(ncp, tsp, ticksp);
-	/*
-	 * On success we return a locked and ref'd vnode as per the lookup
-	 * protocol.
-	 */
 	MPASS(dvp != *vpp);
 	if (!cache_ncp_canuse(ncp)) {
 		vfs_smr_exit();
@@ -1693,12 +1644,7 @@ cache_lookup(struct vnode *dvp, struct vnode **vpp, st
 		*vpp = NULL;
 		goto out_fallback;
 	}
-	if ((cnp->cn_flags & ISLASTCN) &&
-	    (cnp->cn_lkflags & LK_TYPE_MASK) == LK_EXCLUSIVE) {
-		ASSERT_VOP_ELOCKED(*vpp, "cache_lookup");
-	}
 	return (-1);
-
 negative_success:
 	if (__predict_false(cnp->cn_nameiop == CREATE)) {
 		if (cnp->cn_flags & ISLASTCN) {
@@ -1712,7 +1658,8 @@ negative_success:
 	counter_u64_add(numneghits, 1);
 	whiteout = (ncp->nc_flag & NCF_WHITE);
 	/*
-	 * We need to take locks to promote an entry.
+	 * TODO: We need to take locks to promote an entry. Code doing it
+	 * in SMR lookup can be modified to be shared.
 	 */
 	negstate = NCP2NEGSTATE(ncp);
 	if ((negstate->neg_flag & NEG_HOT) == 0 ||
@@ -1850,8 +1797,7 @@ cache_unlock_buckets_cel(struct celockstate *cel)
  *
  * This means vnodelocks for dvp, vp and the relevant bucketlock.
  * However, insertion can result in removal of an old entry. In this
- * case we have an additional vnode and bucketlock pair to lock. If the
- * entry is negative, ncelock is locked instead of the vnode.
+ * case we have an additional vnode and bucketlock pair to lock.
  *
  * That is, in the worst case we have to lock 3 vnodes and 2 bucketlocks, while
  * preserving the locking order (smaller address first).
@@ -1986,7 +1932,6 @@ cache_enter_time(struct vnode *dvp, struct vnode *vp, 
 	int len;
 	u_long lnumcache;
 
-	CTR3(KTR_VFS, "cache_enter(%p, %p, %s)", dvp, vp, cnp->cn_nameptr);
 	VNPASS(!VN_IS_DOOMED(dvp), dvp);
 	VNPASS(dvp->v_type != VNON, dvp);
 	if (vp != NULL) {
@@ -2416,7 +2361,6 @@ cache_purge_negative(struct vnode *vp)
 	struct namecache *ncp, *nnp;
 	struct mtx *vlp;
 
-	CTR1(KTR_VFS, "cache_purge_negative(%p)", vp);
 	SDT_PROBE1(vfs, namecache, purge_negative, done, vp);
 	if (LIST_EMPTY(&vp->v_cache_src))
 		return;


More information about the svn-src-head mailing list