git: 0f1fc3a31fd3 - main - cache: stop manipulating pathlen
Mateusz Guzik
mjg at FreeBSD.org
Thu Jan 7 23:30:22 UTC 2021
The branch main has been updated by mjg:
URL: https://cgit.FreeBSD.org/src/commit/?id=0f1fc3a31fd309ee6d8f9269fa2ab2fdde9c50c8
commit 0f1fc3a31fd309ee6d8f9269fa2ab2fdde9c50c8
Author: Mateusz Guzik <mjg at FreeBSD.org>
AuthorDate: 2021-01-01 09:29:57 +0000
Commit: Mateusz Guzik <mjg at FreeBSD.org>
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;
More information about the dev-commits-src-all
mailing list