Date: Sat, 3 May 2025 00:04:49 GMT From: Mark Johnston <markj@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: cc25864d4568 - main - vfs cache: Move hash row lookup loops into a subroutine Message-ID: <202505030004.54304n2p035254@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=cc25864d4568079cadef46291ddf7d501c81d60a commit cc25864d4568079cadef46291ddf7d501c81d60a Author: Mark Johnston <markj@FreeBSD.org> AuthorDate: 2025-05-02 21:35:04 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2025-05-03 00:04:32 +0000 vfs cache: Move hash row lookup loops into a subroutine No functional change intended. Reviewed by: olce, kib MFC after: 2 weeks Sponsored by: Klara, Inc. Differential Revision: https://reviews.freebsd.org/D50106 --- sys/kern/vfs_cache.c | 129 +++++++++++++++++++++++++-------------------------- sys/sys/vnode.h | 1 + 2 files changed, 65 insertions(+), 65 deletions(-) diff --git a/sys/kern/vfs_cache.c b/sys/kern/vfs_cache.c index 670ac66ae6d7..86e5c65ba3da 100644 --- a/sys/kern/vfs_cache.c +++ b/sys/kern/vfs_cache.c @@ -585,6 +585,24 @@ VP2VNODELOCK(struct vnode *vp) return (&vnodelocks[(((uintptr_t)(vp) >> 8) & ncvnodehash)]); } +/* + * Search the hash table for a namecache entry. Either the corresponding bucket + * must be locked, or the caller must be in an SMR read section. + */ +static struct namecache * +cache_ncp_find(struct vnode *dvp, struct componentname *cnp, uint32_t hash) +{ + struct namecache *ncp; + + KASSERT(mtx_owned(HASH2BUCKETLOCK(hash)) || VFS_SMR_ENTERED(), + ("%s: hash %u not locked", __func__, hash)); + CK_SLIST_FOREACH(ncp, NCHHASH(hash), nc_hash) { + if (cache_ncp_match(ncp, dvp, cnp)) + break; + } + return (ncp); +} + static void cache_out_ts(struct namecache *ncp, struct timespec *tsp, int *ticksp) { @@ -1877,12 +1895,7 @@ retry: goto out_no_entry; mtx_lock(blp); - - CK_SLIST_FOREACH(ncp, (NCHHASH(hash)), nc_hash) { - if (cache_ncp_match(ncp, dvp, cnp)) - break; - } - + ncp = cache_ncp_find(dvp, cnp, hash); if (ncp == NULL) { mtx_unlock(blp); goto out_no_entry; @@ -2078,11 +2091,7 @@ retry: blp = HASH2BUCKETLOCK(hash); mtx_lock(blp); - CK_SLIST_FOREACH(ncp, (NCHHASH(hash)), nc_hash) { - if (cache_ncp_match(ncp, dvp, cnp)) - break; - } - + ncp = cache_ncp_find(dvp, cnp, hash); if (__predict_false(ncp == NULL)) { mtx_unlock(blp); SDT_PROBE2(vfs, namecache, lookup, miss, dvp, cnp->cn_nameptr); @@ -2172,11 +2181,7 @@ cache_lookup(struct vnode *dvp, struct vnode **vpp, struct componentname *cnp, hash = cache_get_hash(cnp->cn_nameptr, cnp->cn_namelen, dvp); vfs_smr_enter(); - CK_SLIST_FOREACH(ncp, (NCHHASH(hash)), nc_hash) { - if (cache_ncp_match(ncp, dvp, cnp)) - break; - } - + ncp = cache_ncp_find(dvp, cnp, hash); if (__predict_false(ncp == NULL)) { vfs_smr_exit(); SDT_PROBE2(vfs, namecache, lookup, miss, dvp, cnp->cn_nameptr); @@ -2494,7 +2499,6 @@ cache_enter_time(struct vnode *dvp, struct vnode *vp, struct componentname *cnp, struct celockstate cel; struct namecache *ncp, *n2, *ndd; struct namecache_ts *ncp_ts; - struct nchashhead *ncpp; uint32_t hash; int flag; int len; @@ -2571,45 +2575,46 @@ cache_enter_time(struct vnode *dvp, struct vnode *vp, struct componentname *cnp, * with this name. This can happen with concurrent lookups of * the same path name. */ - ncpp = NCHHASH(hash); - CK_SLIST_FOREACH(n2, ncpp, nc_hash) { - if (cache_ncp_match(n2, dvp, cnp)) { - MPASS(cache_ncp_canuse(n2)); - if ((n2->nc_flag & NCF_NEGATIVE) != 0) - KASSERT(vp == NULL, - ("%s: found entry pointing to a different vnode (%p != %p) ; name [%s]", - __func__, NULL, vp, cnp->cn_nameptr)); - else - KASSERT(n2->nc_vp == vp, - ("%s: found entry pointing to a different vnode (%p != %p) ; name [%s]", - __func__, n2->nc_vp, vp, cnp->cn_nameptr)); - /* - * Entries are supposed to be immutable unless in the - * process of getting destroyed. Accommodating for - * changing timestamps is possible but not worth it. - * This should be harmless in terms of correctness, in - * the worst case resulting in an earlier expiration. - * Alternatively, the found entry can be replaced - * altogether. - */ - MPASS((n2->nc_flag & (NCF_TS | NCF_DTS)) == (ncp->nc_flag & (NCF_TS | NCF_DTS))); + n2 = cache_ncp_find(dvp, cnp, hash); + if (n2 != NULL) { + MPASS(cache_ncp_canuse(n2)); + if ((n2->nc_flag & NCF_NEGATIVE) != 0) + KASSERT(vp == NULL, + ("%s: found entry pointing to a different vnode " + "(%p != %p); name [%s]", + __func__, NULL, vp, cnp->cn_nameptr)); + else + KASSERT(n2->nc_vp == vp, + ("%s: found entry pointing to a different vnode " + "(%p != %p); name [%s]", + __func__, n2->nc_vp, vp, cnp->cn_nameptr)); + /* + * Entries are supposed to be immutable unless in the + * process of getting destroyed. Accommodating for + * changing timestamps is possible but not worth it. + * This should be harmless in terms of correctness, in + * the worst case resulting in an earlier expiration. + * Alternatively, the found entry can be replaced + * altogether. + */ + MPASS((n2->nc_flag & (NCF_TS | NCF_DTS)) == + (ncp->nc_flag & (NCF_TS | NCF_DTS))); #if 0 - if (tsp != NULL) { - KASSERT((n2->nc_flag & NCF_TS) != 0, - ("no NCF_TS")); - n2_ts = __containerof(n2, struct namecache_ts, nc_nc); - n2_ts->nc_time = ncp_ts->nc_time; - n2_ts->nc_ticks = ncp_ts->nc_ticks; - if (dtsp != NULL) { - n2_ts->nc_dotdottime = ncp_ts->nc_dotdottime; - n2_ts->nc_nc.nc_flag |= NCF_DTS; - } + if (tsp != NULL) { + KASSERT((n2->nc_flag & NCF_TS) != 0, + ("no NCF_TS")); + n2_ts = __containerof(n2, struct namecache_ts, nc_nc); + n2_ts->nc_time = ncp_ts->nc_time; + n2_ts->nc_ticks = ncp_ts->nc_ticks; + if (dtsp != NULL) { + n2_ts->nc_dotdottime = ncp_ts->nc_dotdottime; + n2_ts->nc_nc.nc_flag |= NCF_DTS; } -#endif - SDT_PROBE3(vfs, namecache, enter, duplicate, dvp, ncp->nc_name, - vp); - goto out_unlock_free; } +#endif + SDT_PROBE3(vfs, namecache, enter, duplicate, dvp, ncp->nc_name, + vp); + goto out_unlock_free; } if (flag == NCF_ISDOTDOT) { @@ -2675,7 +2680,7 @@ cache_enter_time(struct vnode *dvp, struct vnode *vp, struct componentname *cnp, * Insert the new namecache entry into the appropriate chain * within the cache entries table. */ - CK_SLIST_INSERT_HEAD(ncpp, ncp, nc_hash); + CK_SLIST_INSERT_HEAD(NCHHASH(hash), ncp, nc_hash); atomic_thread_fence_rel(); /* @@ -3105,12 +3110,10 @@ cache_validate(struct vnode *dvp, struct vnode *vp, struct componentname *cnp) return; blp = HASH2BUCKETLOCK(hash); mtx_lock(blp); - CK_SLIST_FOREACH(ncp, (NCHHASH(hash)), nc_hash) { - if (cache_ncp_match(ncp, dvp, cnp)) { - if (ncp->nc_vp != vp) - panic("%s: mismatch (%p != %p); ncp %p [%s] dvp %p\n", - __func__, vp, ncp->nc_vp, ncp, ncp->nc_name, ncp->nc_dvp); - } + ncp = cache_ncp_find(dvp, cnp, hash); + if (ncp != NULL && ncp->nc_vp != vp) { + panic("%s: mismatch (%p != %p); ncp %p [%s] dvp %p\n", + __func__, vp, ncp->nc_vp, ncp, ncp->nc_name, ncp->nc_dvp); } mtx_unlock(blp); } @@ -5554,11 +5557,7 @@ cache_fplookup_next(struct cache_fpl *fpl) MPASS(!cache_fpl_isdotdot(cnp)); - CK_SLIST_FOREACH(ncp, (NCHHASH(hash)), nc_hash) { - if (cache_ncp_match(ncp, dvp, cnp)) - break; - } - + ncp = cache_ncp_find(dvp, cnp, hash); if (__predict_false(ncp == NULL)) { return (cache_fplookup_noentry(fpl)); } diff --git a/sys/sys/vnode.h b/sys/sys/vnode.h index a2706e6e6b88..bed20f607339 100644 --- a/sys/sys/vnode.h +++ b/sys/sys/vnode.h @@ -1195,6 +1195,7 @@ vn_get_state(struct vnode *vp) #define vfs_smr_exit() smr_exit(VFS_SMR()) #define vfs_smr_synchronize() smr_synchronize(VFS_SMR()) #define vfs_smr_entered_load(ptr) smr_entered_load((ptr), VFS_SMR()) +#define VFS_SMR_ENTERED() SMR_ENTERED(VFS_SMR()) #define VFS_SMR_ASSERT_ENTERED() SMR_ASSERT_ENTERED(VFS_SMR()) #define VFS_SMR_ASSERT_NOT_ENTERED() SMR_ASSERT_NOT_ENTERED(VFS_SMR()) #define VFS_SMR_ZONE_SET(zone) uma_zone_set_smr((zone), VFS_SMR())
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202505030004.54304n2p035254>