lkml.org 
[lkml]   [2015]   [Apr]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] enforce function inlining for hot functions
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



\
 
 \ /
  Last update: 2015-04-25 13:01    [W:2.244 / U:0.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site