Date: Thu, 27 Aug 2020 06:31:27 +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: r364857 - head/sys/kern Message-ID: <202008270631.07R6VRsE050406@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
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;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202008270631.07R6VRsE050406>