Re: [PATCH v2 0/3] symlink length caching

[Mateusz Guzik <mjguzik@gmail.com>]

On Wed, Nov 20, 2024 at 11:33 AM Christian Brauner <brauner@kernel.org> wrote:
>
> On Tue, Nov 19, 2024 at 10:45:52AM +0100, Mateusz Guzik wrote:
> > quote:
> >     When utilized it dodges strlen() in vfs_readlink(), giving about 1.5%
> >     speed up when issuing readlink on /initrd.img on ext4.
> >
> > Benchmark code at the bottom.
> >
> > ext4 and tmpfs are patched, other filesystems can also get there with
> > some more work.
> >
> > Arguably the current get_link API should be patched to let the fs return
> > the size, but that's not a churn I'm interested into diving in.
> >
> > On my v1 Jan remarked 1.5% is not a particularly high win questioning
> > whether doing this makes sense. I noted the value is only this small
> > because of other slowdowns.
>
> The thing is that you're stealing one of the holes I just put into struct
> inode a cycle ago or so. The general idea has been to shrink struct
> inode if we can and I'm not sure that caching the link length is
> actually worth losing that hole. Otherwise I wouldn't object.
>

Per the patch description this can be a union with something not used
for symlinks. I'll find a nice field.

> > All that aside there is also quite a bit of branching and func calling
> > which does not need to be there (example: make vfsuid/vfsgid, could be
> > combined into one routine etc.).
>
> They should probably also be made inline functions and likely/unlikely
> sprinkled in there.

someone(tm) should at least do a sweep through in-vfs code. for
example LOOKUP_IS_SCOPED is sometimes marked as unlikely and other
times has no annotations whatsoever, even though ultimately it all
executes in the same setting

Interestingly even __read_seqcount_begin (used *twice* in path_init())
is missing one. I sent a patch to fix it long time ago but the
recipient did not respond, maybe I should resend with more people in
cc (who though?), see:
https://lore.kernel.org/all/20230727180355.813995-1-mjguzik@gmail.com/

-- 
Mateusz Guzik <mjguzik gmail.com>