Re: [PATCH] enforce function inlining for hot functions

["Paul E. McKenney" <paulmck@linux.vnet.ibm.com>]

On Sat, Apr 25, 2015 at 01:10:56AM +0200, Hagen Paul Pfeifer wrote:
> * 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

OK, so you have PROVE_RCU=n and CONFIG_DEBUG_LOCK_ALLOC=y in this
case?  That would get rid of the rcu_lockdep_assert(), but keep the
rcu_lock_acquire().

> 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.

I am not arguing either way on the wisdom or lack thereof of gcc's
inlining decisions.  But PROVE_RCU=n and CONFIG_DEBUG_LOCK_ALLOC=n should
make rcu_read_lock() and rcu_read_unlock() both be empty functions in
a CONFIG_PREEMPT=n, which should hopefully trivialize gcc's inlining
decisions in that particular case.

Apologies for not identifying CONFIG_DEBUG_LOCK_ALLOC=n to begin with.

							Thanx, Paul