linux-kernel - Re: [PATCH] enforce function inlining for hot functions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150424231056.GA6321@virgo.local>
Date:	Sat, 25 Apr 2015 01:10:56 +0200
From:	Hagen Paul Pfeifer <hagen@...u.net>
To:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	"David S. Miller" <davem@...emloft.net>, x86@...nel.org
Subject: Re: [PATCH] enforce function inlining for hot functions

* Paul E. McKenney | 2015-04-24 13:13:40 [-0700]:

>Hmmm...  allyesconfig would have PROVE_RCU=y, which would mean that the
>above two would contain lockdep calls that might in some cases defeat
>inlining.  With the more typical production choice of PROVE_RCU=n, I would
>expect these to just be a call instruction, which should get inlined.


Ok, here are the results:

with PROVE_RCU=y:
    rcu_read_lock: 383 duplicates
with PROVE_RCU=n:
    rcu_read_lock: 114 duplicates


If you look at the function anatomy of rcu_read_lock you often see the
following definitions:

<rcu_read_lock>:
 55                        push   %rbp
 48 89 e5                  mov    %rsp,%rbp
 48 c7 c7 50 64 e7 85      mov    $0xffffffff85e76450,%rdi
 e8 ce ff ff ff            callq  ffffffff816af206 <rcu_lock_acquire>
 5d                        pop    %rbp
 c3                        retq

but sometimes rcu_read_lock looks:

<rcu_read_lock>:
 55                        push   %rbp
 48 89 e5                  mov    %rsp,%rbp
 50                        push   %rax
 68 83 1e 1c 81            pushq  $0xffffffff811c1e83
 b9 02 00 00 00            mov    $0x2,%ecx
 31 d2                     xor    %edx,%edx
 45 31 c9                  xor    %r9d,%r9d
 45 31 c0                  xor    %r8d,%r8d
 31 f6                     xor    %esi,%esi
 48 c7 c7 50 64 e7 85      mov    $0xffffffff85e76450,%rdi
 e8 86 4c f9 ff            callq  ffffffff81156b2e <lock_acquire>
 5a                        pop    %rdx
 59                        pop    %rcx
 c9                        leaveq   
 c3                        retq


Means rcu_lock_acquire() is inlined here - but not in every compilation unit.
Don't know exactly what forces gcc to inline not everywhere. Maybe register
pressure in the function unit, or at least gcc is think that. I don't know.

At the end you may notice that gcc inlining decisions are not always perfect
and a little bit fuzzy (sure, they have their metric/scoring system). And
sometimes the inlining should be enforced - as this patch do for some important
functions. But as I said we should not enforce it everywhere, rather we should
pray for better heuristics and let the compiler choose the best strategy (and
incorporate -Os/-O2 decisions too). I think this is the best compromise here.

Cheers, Hagen
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/