Message-ID: <CAGudoHEPAGu4T9WvuA_zG_ALyMAbnbaFDqv54y4-G9FgbqEqrg@mail.gmail.com>
Date: Wed, 19 Nov 2025 21:40:31 +0100
From: Mateusz Guzik <mjguzik@...il.com>
To: Al Viro <viro@...iv.linux.org.uk>
Cc: brauner@...nel.org, jack@...e.cz, linux-kernel@...r.kernel.org, 
	linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH] fs: inline step_into() and walk_component()

So I think this is best split into 2 parts:
1. the tidy ups around step_into as present right now
2. implementation of the fast path + inlining of complete_walk

I did not previously mentally register that pick_link() is not
tail-callable because the address of struct path is passed as one of
the arguments. I'm going to massage it to pass dentry, mnt as 2 args
instead.

As for the fast path, I slightly tweaked your version:
static __always_inline const char *step_into(struct nameidata *nd, int flags,
                    struct dentry *dentry)
{
       if (likely((nd->flags & LOOKUP_RCU) &&
           !d_managed(dentry) && !d_is_symlink(dentry))) {
               /*
                * Simple case of not having to worry about mount
points or symlink traversal.
                */
               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);
}

There is a comment about path.mnt and use of d_managed instead of
open-coding it, the other comment is a little different, but again
that's cosmetics.

Since this is de facto your code, modulo small massaging by me (+ two
instances of __always_inline), I don't think I should be slapping my
authorship on it and maybe you prefer your version exactly as you sent
it. Alternatively, since this follows step_into() as currently
present, just lifted out and slightly massaged, I can slap my name on
this and state this is what happened.

I'll be a perfectly happy camper if you take authorship here and write
your own commit message or take mine in whatever capacity (presumably
you would want the bench + profiling info).

Just state how you want this sorted out on my end. My goal here is to
get this crapper out of the way, I don't care for some meta data on a
commit.

On Wed, Nov 19, 2025 at 9:24 PM Al Viro <viro@...iv.linux.org.uk> wrote:
>
> On Wed, Nov 19, 2025 at 08:49:36PM +0100, Mateusz Guzik wrote:
> > btw, is there a way to combine DCACHE_MANAGED_DENTRY + symlink check
> > into one branch? The compiler refuses at the moment due to type being
> > a bitfield. Given how few free flags are remaining this is quite a
> > bummer.
>
> Is that really worth bothering with?  Condition is "bits 15..17 are 000
> and bits 19..21 are _not_ 110" and I don't see any clean way to change
> the encoding so that it could be done in one test.  In theory we could
> do something like swapping the "type" and "managed" bits and use the
> fact that 6 (symlink) is the maximal type we might run into, turning
> that into flags & ((7<<15)|(7<<19))) < (6<<15), but...  *ewww*
>
> If nothing else, it would
>         * require us to remember that "managed" bits must be higher
> than "type" ones
>         * require us to remember that 7 is never to be used as type
>         * require every sodding human reader to figure out what the
> bleeding fuck is that test about
> and I really don't believe that shaving a couple of cycles is worth
> that.

Powered by blists - more mailing lists