| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAGudoHGTUtMKk=2NuSAZj6hZLEX8qPBLYo5qHXEjYMobV8=2BQ@mail.gmail.com>
Date: Wed, 26 Nov 2025 03:32:51 +0100
From: Mateusz Guzik <mjguzik@...il.com>
To: brauner@...nel.org, viro@...iv.linux.org.uk
Cc: jack@...e.cz, linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] fs: inline step_into() and walk_component()
ping? would be a bummer to miss out on this for the merge window
On Thu, Nov 20, 2025 at 1:39 AM Mateusz Guzik <mjguzik@...il.com> wrote:
>
> The primary consumer is link_path_walk(), calling walk_component() every
> time which in turn calls step_into().
>
> Inlining these saves overhead of 2 function calls per path component,
> along with allowing the compiler to do better job optimizing them in place.
>
> step_into() had absolutely atrocious assembly to facilitate the
> slowpath. In order to lessen the burden at the callsite all the hard
> work is moved into step_into_slowpath() and instead an inline-able
> fastpath is implemented for rcu-walk.
>
> The new fastpath is a stripped down step_into() RCU handling with a
> d_managed() check from handle_mounts().
>
> Benchmarked as follows on Sapphire Rapids:
> 1. the "before" was a kernel with not-yet-merged optimizations (notably
> elision of calls to security_inode_permission() and marking ext4
> inodes as not having acls as applicable)
> 2. "after" is the same + the prep patch + this patch
> 3. benchmark consists of issuing 205 calls to access(2) in a loop with
> pathnames lifted out of gcc and the linker building real code, most
> of which have several path components and 118 of which fail with
> -ENOENT.
>
> Result in terms of ops/s:
> before: 21619
> after: 22536 (+4%)
>
> profile before:
> 20.25% [kernel] [k] __d_lookup_rcu
> 10.54% [kernel] [k] link_path_walk
> 10.22% [kernel] [k] entry_SYSCALL_64
> 6.50% libc.so.6 [.] __GI___access
> 6.35% [kernel] [k] strncpy_from_user
> 4.87% [kernel] [k] step_into
> 3.68% [kernel] [k] kmem_cache_alloc_noprof
> 2.88% [kernel] [k] walk_component
> 2.86% [kernel] [k] kmem_cache_free
> 2.14% [kernel] [k] set_root
> 2.08% [kernel] [k] lookup_fast
>
> after:
> 23.38% [kernel] [k] __d_lookup_rcu
> 11.27% [kernel] [k] entry_SYSCALL_64
> 10.89% [kernel] [k] link_path_walk
> 7.00% libc.so.6 [.] __GI___access
> 6.88% [kernel] [k] strncpy_from_user
> 3.50% [kernel] [k] kmem_cache_alloc_noprof
> 2.01% [kernel] [k] kmem_cache_free
> 2.00% [kernel] [k] set_root
> 1.99% [kernel] [k] lookup_fast
> 1.81% [kernel] [k] do_syscall_64
> 1.69% [kernel] [k] entry_SYSCALL_64_safe_stack
>
> While walk_component() and step_into() of course disappear from the
> profile, the link_path_walk() barely gets more overhead despite the
> inlining thanks to the fast path added and while completing more walks
> per second.
>
> I did not investigate why overhead grew a lot on __d_lookup_rcu().
>
> Signed-off-by: Mateusz Guzik <mjguzik@...il.com>
> ---
>
> v2:
> - reimplement without gotos, instead make it look like step_into
> - use d_managed instead of open-coding it
>
> Technically this version was written by Al Viro, but this is just
> step_into() RCU handling with an internal RCU check removed and
> d_managed() check added which re-did anyway to make sure nothing is
> missing (that and some trivial comment changes).
>
> Since Al did not respond yet to a query what he wants done with an
> authorship and I had the cleanup prepared, I decided to send this v2.
>
> I'm more than happy to change the author and drop my name from this
> patch if that's most expedient to get it in.
>
>
> fs/namei.c | 31 ++++++++++++++++++++++++++++---
> 1 file changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index 8777637ef939..2c83f894f276 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -1951,7 +1951,7 @@ static noinline const char *pick_link(struct nameidata *nd, struct path *link,
> int error;
>
> if (nd->flags & LOOKUP_RCU) {
> - /* make sure that d_is_symlink from step_into() matches the inode */
> + /* make sure that d_is_symlink from step_into_slowpath() matches the inode */
> if (read_seqcount_retry(&link->dentry->d_seq, nd->next_seq))
> return ERR_PTR(-ECHILD);
> } else {
> @@ -2033,7 +2033,7 @@ static noinline const char *pick_link(struct nameidata *nd, struct path *link,
> *
> * NOTE: dentry must be what nd->next_seq had been sampled from.
> */
> -static const char *step_into(struct nameidata *nd, int flags,
> +static noinline const char *step_into_slowpath(struct nameidata *nd, int flags,
> struct dentry *dentry)
> {
> struct path path;
> @@ -2066,6 +2066,31 @@ static const char *step_into(struct nameidata *nd, int flags,
> return pick_link(nd, &path, inode, flags);
> }
>
> +static __always_inline const char *step_into(struct nameidata *nd, int flags,
> + struct dentry *dentry)
> +{
> + /*
> + * In the common case we are in rcu-walk and traversing over a non-mounted on
> + * directory (as opposed to e.g., a symlink).
> + *
> + * We can handle that and negative entries with the checks below.
> + */
> + if (likely((nd->flags & LOOKUP_RCU) &&
> + !d_managed(dentry) && !d_is_symlink(dentry))) {
> + struct inode *inode = dentry->d_inode;
> + if (read_seqcount_retry(&dentry->d_seq, nd->next_seq))
> + return ERR_PTR(-ECHILD);
> + if (unlikely(!inode))
> + return ERR_PTR(-ENOENT);
> + nd->path.dentry = dentry;
> + /* nd->path.mnt is retained on purpose */
> + nd->inode = inode;
> + nd->seq = nd->next_seq;
> + return NULL;
> + }
> + return step_into_slowpath(nd, flags, dentry);
> +}
> +
> static struct dentry *follow_dotdot_rcu(struct nameidata *nd)
> {
> struct dentry *parent, *old;
> @@ -2176,7 +2201,7 @@ static const char *handle_dots(struct nameidata *nd, int type)
> return NULL;
> }
>
> -static const char *walk_component(struct nameidata *nd, int flags)
> +static __always_inline const char *walk_component(struct nameidata *nd, int flags)
> {
> struct dentry *dentry;
> /*
> --
> 2.48.1
>
Powered by blists - more mailing lists