From owner-dev-commits-src-main@freebsd.org Fri Jan 1 00:11:53 2021 Return-Path: Delivered-To: dev-commits-src-main@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id A1FD54D5DD7; Fri, 1 Jan 2021 00:11:53 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4D6QS93FcHz4bgp; Fri, 1 Jan 2021 00:11:53 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4E79719913; Fri, 1 Jan 2021 00:11:53 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 1010BraD005875; Fri, 1 Jan 2021 00:11:53 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 1010Brrq005874; Fri, 1 Jan 2021 00:11:53 GMT (envelope-from git) Date: Fri, 1 Jan 2021 00:11:53 GMT Message-Id: <202101010011.1010Brrq005874@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Mateusz Guzik Subject: git: 67297766b5f1 - main - cache: hoist trailing slash and degenerate path handling out of the loop MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: mjg X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 67297766b5f1bb875090dd50a0da4e006c9bf85b Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-main@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for the main branch of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 01 Jan 2021 00:11:53 -0000 The branch main has been updated by mjg: URL: https://cgit.FreeBSD.org/src/commit/?id=67297766b5f1bb875090dd50a0da4e006c9bf85b commit 67297766b5f1bb875090dd50a0da4e006c9bf85b Author: Mateusz Guzik AuthorDate: 2020-12-28 04:24:15 +0000 Commit: Mateusz Guzik 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)) {