On Wed, Nov 20, 2024 at 11:33 AM Christian Brauner <brauner@xxxxxxxxxx> 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@xxxxxxxxx/ -- Mateusz Guzik <mjguzik gmail.com>
