Re: acl_permission_check: disgusting performance

[Linus Torvalds <torvalds@linux-foundation.org>]

On Thu, May 12, 2011 at 9:02 PM, Serge E. Hallyn <serge@hallyn.com> wrote:
>
> I wonder how much this would help:  (only compile-tested)

So it should help a lot, but it breaks when CONFIG_USER_NS isn't even
set (the case that Eric fixed.

So instead, do this:

 (a) get rid of the "_current_user_ns()" thing. There is no reason to
have it, if it's directly off "current->cred", then it's cheaper to
inline it than have a function just for two pointer indirections.

 (b) do

  #ifdef CONFIG_USER_NS
    #define current_user_ns() (&init_user_ns)
  #else
    #define current_user_ns() (current_cred_xxx(user_ns))
  #endif

 (c) and then the rest of your patch (to actually initialize and set
"cred->user_ns") should be fine. But don't touch
acl_permission_check() at all - once you've fixed current_user_ns() to
DTRT, acl_permission_check will be ok.

Ok?

Then the compiler will do the right thing: in acl_permission_check()
it will see that current_user_ns() and "current_fsuid()" share 90% of
the code, and will CSE the "current_cred()" part (or, if
CONFIG_USER_NS isn't set, it will see a constant comparison of
&init_user_ns with itself, and generate no code for hat case). There's
no reason for you to do it for it, and when  you do it by hand you get
the !CONFIG_USER_NS case wrong.

Hmm? That should fix all the horrible code generation issues in
acl_permission_check. When CONFIG_USER_NS isn't set, it will generate
no code at all, and when it _is_ set, it will just generate two loads
from current->cred (one for user_ns, one for fsuid), which is about as
good as it gets.

                              Linus