The branch main has been updated by mjg: URL: https://cgit.FreeBSD.org/src/commit/?id=0f1fc3a31fd309ee6d8f9269fa2ab2fdde9c50c8
commit 0f1fc3a31fd309ee6d8f9269fa2ab2fdde9c50c8 Author: Mateusz Guzik <[email protected]> AuthorDate: 2021-01-01 09:29:57 +0000 Commit: Mateusz Guzik <[email protected]> CommitDate: 2021-01-07 23:26:53 +0000 cache: stop manipulating pathlen It is a copy-pasto from regular lookup. Add debug to ensure the result is the same. --- sys/kern/vfs_cache.c | 120 ++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 100 insertions(+), 20 deletions(-) diff --git a/sys/kern/vfs_cache.c b/sys/kern/vfs_cache.c index 43c44aa60afc..b6f97e55746b 100644 --- a/sys/kern/vfs_cache.c +++ b/sys/kern/vfs_cache.c @@ -3616,13 +3616,22 @@ SYSCTL_PROC(_vfs, OID_AUTO, cache_fast_lookup, CTLTYPE_INT|CTLFLAG_RW|CTLFLAG_MP */ struct nameidata_saved { char *cn_nameptr; +#ifdef INVARIANTS size_t ni_pathlen; +#endif int cn_flags; }; +#ifdef INVARIANTS +struct cache_fpl_debug { + size_t ni_pathlen; +}; +#endif + struct cache_fpl { struct nameidata *ndp; struct componentname *cnp; + char *nulchar; struct pwd **pwd; struct vnode *dvp; struct vnode *tvp; @@ -3635,12 +3644,18 @@ struct cache_fpl { bool in_smr; bool fsearch; bool savename; +#ifdef INVARIANTS + struct cache_fpl_debug debug; +#endif }; static bool cache_fplookup_is_mp(struct cache_fpl *fpl); static int cache_fplookup_cross_mount(struct cache_fpl *fpl); static int cache_fplookup_partial_setup(struct cache_fpl *fpl); static int cache_fplookup_skip_slashes(struct cache_fpl *fpl); +static void cache_fpl_pathlen_dec(struct cache_fpl *fpl); +static void cache_fpl_pathlen_inc(struct cache_fpl *fpl); +static void cache_fpl_pathlen_sub(struct cache_fpl *fpl, size_t n); static void cache_fpl_cleanup_cnp(struct componentname *cnp) @@ -3664,12 +3679,12 @@ cache_fpl_handle_root(struct cache_fpl *fpl) MPASS(*(cnp->cn_nameptr) == '/'); cnp->cn_nameptr++; - ndp->ni_pathlen--; + cache_fpl_pathlen_dec(fpl); if (__predict_false(*(cnp->cn_nameptr) == '/')) { do { cnp->cn_nameptr++; - ndp->ni_pathlen--; + cache_fpl_pathlen_dec(fpl); } while (*(cnp->cn_nameptr) == '/'); } @@ -3682,7 +3697,9 @@ cache_fpl_checkpoint(struct cache_fpl *fpl, struct nameidata_saved *snd) snd->cn_flags = fpl->ndp->ni_cnd.cn_flags; snd->cn_nameptr = fpl->ndp->ni_cnd.cn_nameptr; - snd->ni_pathlen = fpl->ndp->ni_pathlen; +#ifdef INVARIANTS + snd->ni_pathlen = fpl->debug.ni_pathlen; +#endif } static void @@ -3691,7 +3708,9 @@ cache_fpl_restore_partial(struct cache_fpl *fpl, struct nameidata_saved *snd) fpl->ndp->ni_cnd.cn_flags = snd->cn_flags; fpl->ndp->ni_cnd.cn_nameptr = snd->cn_nameptr; - fpl->ndp->ni_pathlen = snd->ni_pathlen; +#ifdef INVARIANTS + fpl->debug.ni_pathlen = snd->ni_pathlen; +#endif } static void @@ -4014,10 +4033,18 @@ cache_fplookup_partial_setup(struct cache_fpl *fpl) if (__predict_false(*(cnp->cn_nameptr) == '/')) { do { cnp->cn_nameptr++; - ndp->ni_pathlen--; + cache_fpl_pathlen_dec(fpl); } while (*(cnp->cn_nameptr) == '/'); } + ndp->ni_pathlen = fpl->nulchar - cnp->cn_nameptr + 1; +#ifdef INVARIANTS + if (ndp->ni_pathlen != fpl->debug.ni_pathlen) { + panic("%s: mismatch (%zu != %zu) nulchar %p nameptr %p [%s] ; full string [%s]\n", + __func__, ndp->ni_pathlen, fpl->debug.ni_pathlen, fpl->nulchar, + cnp->cn_nameptr, cnp->cn_nameptr, cnp->cn_pnbuf); + } +#endif return (0); } @@ -4876,6 +4903,50 @@ cache_fplookup_is_mp(struct cache_fpl *fpl) * must take into account that in case off fallback the resulting * nameidata state has to be compatible with the original. */ + +/* + * Debug ni_pathlen tracking. + */ +#ifdef INVARIANTS +static void +cache_fpl_pathlen_dec(struct cache_fpl *fpl) +{ + + cache_fpl_pathlen_sub(fpl, 1); +} + +static void +cache_fpl_pathlen_inc(struct cache_fpl *fpl) +{ + + fpl->debug.ni_pathlen++; +} + +static void +cache_fpl_pathlen_sub(struct cache_fpl *fpl, size_t n) +{ + + fpl->debug.ni_pathlen -= n; + KASSERT(fpl->debug.ni_pathlen <= PATH_MAX, + ("%s: pathlen underflow to %zd\n", __func__, fpl->debug.ni_pathlen)); +} +#else +static void __always_inline +cache_fpl_pathlen_dec(struct cache_fpl *fpl) +{ +} + +static void __always_inline +cache_fpl_pathlen_inc(struct cache_fpl *fpl) +{ +} + +static void +cache_fpl_pathlen_sub(struct cache_fpl *fpl, size_t n) +{ +} +#endif + static int cache_fplookup_preparse(struct cache_fpl *fpl) { @@ -4893,10 +4964,13 @@ cache_fplookup_preparse(struct cache_fpl *fpl) * By this point the shortest possible pathname is one character + nul * terminator, hence 2. */ - KASSERT(ndp->ni_pathlen >= 2, ("%s: ni_pathlen %zu\n", __func__, - ndp->ni_pathlen)); - - if (__predict_false(cnp->cn_nameptr[ndp->ni_pathlen - 2] == '/')) { + KASSERT(fpl->debug.ni_pathlen >= 2, ("%s: pathlen %zu\n", __func__, + fpl->debug.ni_pathlen)); + KASSERT(&cnp->cn_nameptr[fpl->debug.ni_pathlen - 2] == fpl->nulchar - 1, + ("%s: mismatch on string (%p != %p) [%s]\n", __func__, + &cnp->cn_nameptr[fpl->debug.ni_pathlen - 2], fpl->nulchar - 1, + cnp->cn_pnbuf)); + if (__predict_false(*(fpl->nulchar - 1) == '/')) { /* * TODO * Regular lookup performs the following: @@ -4931,22 +5005,24 @@ cache_fplookup_parse(struct cache_fpl *fpl) * to test for. Pathnames tend to be short so this should not be * resulting in cache misses. */ - KASSERT(cnp->cn_nameptr[ndp->ni_pathlen - 1] == '\0', - ("%s: expected nul at %p + %zu; string [%s]\n", __func__, - cnp->cn_nameptr, ndp->ni_pathlen - 1, cnp->cn_nameptr)); - cnp->cn_nameptr[ndp->ni_pathlen - 1] = '/'; + KASSERT(&cnp->cn_nameptr[fpl->debug.ni_pathlen - 1] == fpl->nulchar, + ("%s: mismatch between pathlen (%zu) and nulchar (%p != %p), string [%s]\n", + __func__, fpl->debug.ni_pathlen, &cnp->cn_nameptr[fpl->debug.ni_pathlen - 1], + fpl->nulchar, cnp->cn_pnbuf)); + KASSERT(*fpl->nulchar == '\0', + ("%s: expected nul at %p; string [%s]\n", __func__, fpl->nulchar, + cnp->cn_pnbuf)); + *fpl->nulchar = '/'; for (cp = cnp->cn_nameptr; *cp != '/'; cp++) { KASSERT(*cp != '\0', ("%s: encountered unexpected nul; string [%s]\n", __func__, cnp->cn_nameptr)); continue; } - cnp->cn_nameptr[ndp->ni_pathlen - 1] = '\0'; + *fpl->nulchar = '\0'; cnp->cn_namelen = cp - cnp->cn_nameptr; - ndp->ni_pathlen -= cnp->cn_namelen; - KASSERT(ndp->ni_pathlen <= PATH_MAX, - ("%s: ni_pathlen underflow to %zd\n", __func__, ndp->ni_pathlen)); + cache_fpl_pathlen_sub(fpl, cnp->cn_namelen); /* * Hack: we have to check if the found path component's length exceeds * NAME_MAX. However, the condition is very rarely true and check can @@ -4992,7 +5068,7 @@ cache_fplookup_parse_advance(struct cache_fpl *fpl) ("%s: should have seen slash at %p ; buf %p [%s]\n", __func__, cnp->cn_nameptr, cnp->cn_pnbuf, cnp->cn_pnbuf)); cnp->cn_nameptr++; - ndp->ni_pathlen--; + cache_fpl_pathlen_dec(fpl); } /* @@ -5016,7 +5092,7 @@ cache_fplookup_skip_slashes(struct cache_fpl *fpl) MPASS(*(cnp->cn_nameptr) == '/'); do { cnp->cn_nameptr++; - ndp->ni_pathlen--; + cache_fpl_pathlen_dec(fpl); } while (*(cnp->cn_nameptr) == '/'); /* @@ -5024,7 +5100,7 @@ cache_fplookup_skip_slashes(struct cache_fpl *fpl) * something to skip. */ cnp->cn_nameptr--; - ndp->ni_pathlen++; + cache_fpl_pathlen_inc(fpl); /* * cache_fplookup_parse_advance starts from ndp->ni_next @@ -5309,6 +5385,10 @@ cache_fplookup(struct nameidata *ndp, enum cache_fpl_status *status, cache_fpl_checkpoint(&fpl, &fpl.snd_orig); cache_fpl_smr_enter_initial(&fpl); +#ifdef INVARIANTS + fpl.debug.ni_pathlen = ndp->ni_pathlen; +#endif + fpl.nulchar = &cnp->cn_nameptr[ndp->ni_pathlen - 1]; fpl.fsearch = false; fpl.savename = (cnp->cn_flags & SAVENAME) != 0; fpl.pwd = pwdp; _______________________________________________ [email protected] mailing list https://lists.freebsd.org/mailman/listinfo/dev-commits-src-all To unsubscribe, send any mail to "[email protected]"
