Date: Fri, 1 Jan 2021 00:11:53 GMT From: Mateusz Guzik <mjg@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 67297766b5f1 - main - cache: hoist trailing slash and degenerate path handling out of the loop Message-ID: <202101010011.1010Brrq005874@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by mjg: URL: https://cgit.FreeBSD.org/src/commit/?id=67297766b5f1bb875090dd50a0da4e006c9bf85b commit 67297766b5f1bb875090dd50a0da4e006c9bf85b Author: Mateusz Guzik <mjguzik@gmail.com> AuthorDate: 2020-12-28 04:24:15 +0000 Commit: Mateusz Guzik <mjg@FreeBSD.org> CommitDate: 2021-01-01 00:10:42 +0000 cache: hoist trailing slash and degenerate path handling out of the loop Tested by: pho --- sys/kern/vfs_cache.c | 91 +++++++++++++++++++++++++++++++++++----------------- 1 file changed, 61 insertions(+), 30 deletions(-) diff --git a/sys/kern/vfs_cache.c b/sys/kern/vfs_cache.c index 94d622867425..7f9339d2ed56 100644 --- a/sys/kern/vfs_cache.c +++ b/sys/kern/vfs_cache.c @@ -4685,6 +4685,51 @@ 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. */ +static int +cache_fplookup_preparse(struct cache_fpl *fpl) +{ + struct nameidata *ndp; + struct componentname *cnp; + + ndp = fpl->ndp; + cnp = fpl->cnp; + + /* + * TODO + * Original comment: + * Check for degenerate name (e.g. / or "") + * which is a way of talking about a directory, + * e.g. like "/." or ".". + */ + if (__predict_false(cnp->cn_nameptr[0] == '\0')) { + return (cache_fpl_aborted(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] == '/')) { + /* + * TODO + * Regular lookup performs the following: + * *ndp->ni_next = '\0'; + * cnp->cn_flags |= TRAILINGSLASH; + * + * Which is problematic since it modifies data read + * from userspace. Then if fast path lookup was to + * abort we would have to either restore it or convey + * the flag. Since this is a corner case just ignore + * it for simplicity. + */ + return (cache_fpl_aborted(fpl)); + } + return (0); +} + static int cache_fplookup_parse(struct cache_fpl *fpl) { @@ -4715,45 +4760,26 @@ cache_fplookup_parse(struct cache_fpl *fpl) ("%s: ni_pathlen underflow to %zd\n", __func__, ndp->ni_pathlen)); ndp->ni_next = cp; +#ifdef INVARIANTS /* - * Replace multiple slashes by a single slash and trailing slashes - * by a null. This must be done before VOP_LOOKUP() because some - * fs's don't know about trailing slashes. Remember if there were - * trailing slashes to handle symlinks, existing non-directories - * and non-existing files that won't be directories specially later. + * Code below is only here to assure compatibility with regular lookup. + * It covers handling of trailing slashles and names like "/", both of + * which of can be taken care of upfront which lockless lookup does + * in cache_fplookup_preparse. Regular lookup performs these for each + * path component. */ while (*cp == '/' && (cp[1] == '/' || cp[1] == '\0')) { cp++; - ndp->ni_pathlen--; if (*cp == '\0') { - /* - * TODO - * Regular lookup performs the following: - * *ndp->ni_next = '\0'; - * cnp->cn_flags |= TRAILINGSLASH; - * - * Which is problematic since it modifies data read - * from userspace. Then if fast path lookup was to - * abort we would have to either restore it or convey - * the flag. Since this is a corner case just ignore - * it for simplicity. - */ - return (cache_fpl_partial(fpl)); + panic("%s: ran into TRAILINGSLASH handling from [%s]\n", + __func__, cnp->cn_pnbuf); } } - ndp->ni_next = cp; - /* - * Check for degenerate name (e.g. / or "") - * which is a way of talking about a directory, - * e.g. like "/." or ".". - * - * TODO - * Another corner case handled by the regular lookup - */ - if (__predict_false(cnp->cn_nameptr[0] == '\0')) { - return (cache_fpl_partial(fpl)); + if (cnp->cn_nameptr[0] == '\0') { + panic("%s: ran into degenerate name from [%s]\n", __func__, cnp->cn_pnbuf); } +#endif return (0); } @@ -4877,6 +4903,11 @@ cache_fplookup_impl(struct vnode *dvp, struct cache_fpl *fpl) VNPASS(cache_fplookup_vnode_supported(fpl->dvp), fpl->dvp); + error = cache_fplookup_preparse(fpl); + if (__predict_false(error != 0)) { + goto out; + } + for (;;) { error = cache_fplookup_parse(fpl); if (__predict_false(error != 0)) {
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202101010011.1010Brrq005874>