Messages in this thread | | | From | Linus Torvalds <> | Date | Fri, 5 Jun 2020 13:18:21 -0700 | Subject | Re: [PATCH resend] fs/namei.c: micro-optimize acl_permission_check |
| |
On Fri, Jun 5, 2020 at 7:23 AM Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote: > > + /* > + * If the "group" and "other" permissions are the same, > + * there's no point calling in_group_p() to decide which > + * set to use. > + */ > + if ((((mode >> 3) ^ mode) & 7) && in_group_p(inode->i_gid)) > mode >>= 3;
Ugh. Not only is this ugly, but it's not even the best optimization.
We don't care that group and other match exactly. We only care that they match in the low 3 bits of the "mask" bits.
So if we want this optimization - and it sounds worth it - I think we should do it right. But I also think it should be written more legibly.
And the "& 7" is the same "& (MAY_READ | MAY_WRITE | MAY_EXEC)" we do later.
In other words, if we do this, I'd like it to be done even more aggressively, but I'd also like the end result to be a lot more readable and have more comments about why we do that odd thing.
Something like this *UNTESTED* patch, perhaps?
I might have gotten something wrong, so this would need double-checking, but if it's right, I find it a _lot_ more easy to understand than making one expression that is pretty complicated and opaque.
Hmm?
Linus [unhandled content-type:application/octet-stream] | |