On Sat 12-04-25 13:09:35, Mateusz Guzik wrote:
> Looking at the asm produced by gcc 13.3 for x86-64:
> 1. may_lookup() usage was not optimized for succeeding, despite the
> routine being inlined and rightfully starting with likely(!err)
> 2. the compiler assumed the path will have an indefinite amount of
> slashes to skip, after which the result will be an empty name
>
> As such:
> 1. predict may_lookup() succeeding
> 2. check for one slash, no explicit predicts. do roll forward with
> skipping more slashes while predicting there is only one
> 3. predict the path to find was not a mere slash
>
> This also has a side effect of shrinking the file:
> add/remove: 1/1 grow/shrink: 0/3 up/down: 934/-1012 (-78)
> Function old new delta
> link_path_walk - 934 +934
> path_parentat 138 112 -26
> path_openat 4864 4823 -41
> path_lookupat 418 374 -44
> link_path_walk.part.constprop 901 - -901
> Total: Before=46639, After=46561, chg -0.17%
>
> Signed-off-by: Mateusz Guzik <mjguzik@xxxxxxxxx>
Looks sensible. Feel free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
> ---
>
> I'm looking at skipping perm checks with an "everybody can MAY_EXEC and
> there are no acls" bit for opflags. This crapper is a side effect of
> straighetning out the code before I get there.
>
> fs/namei.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index 360a86ca1f02..40a636bbfa0c 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -2424,9 +2424,12 @@ static int link_path_walk(const char *name, struct nameidata *nd)
> nd->flags |= LOOKUP_PARENT;
> if (IS_ERR(name))
> return PTR_ERR(name);
> - while (*name=='/')
> - name++;
> - if (!*name) {
> + if (*name == '/') {
> + do {
> + name++;
> + } while (unlikely(*name == '/'));
> + }
> + if (unlikely(!*name)) {
> nd->dir_mode = 0; // short-circuit the 'hardening' idiocy
> return 0;
> }
> @@ -2439,7 +2442,7 @@ static int link_path_walk(const char *name, struct nameidata *nd)
>
> idmap = mnt_idmap(nd->path.mnt);
> err = may_lookup(idmap, nd);
> - if (err)
> + if (unlikely(err))
> return err;
>
> nd->last.name = name;
> --
> 2.43.0
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
