Messages in this thread | | | Date | Tue, 12 May 2015 11:44:43 +0200 | From | Denys Vlasenko <> | Subject | Re: [PATCH] force inlining of spinlock ops |
| |
On 05/12/2015 12:19 AM, Andrew Morton wrote: > On Mon, 11 May 2015 19:57:22 +0200 Denys Vlasenko <dvlasenk@redhat.com> wrote: > >> With both gcc 4.7.2 and 4.9.2, sometimes gcc mysteriously doesn't inline >> very small functions we expect to be inlined. In particular, >> with this config: http://busybox.net/~vda/kernel_config >> there are more than a thousand copies of tiny spinlock-related functions: >> >> $ nm --size-sort vmlinux | grep -iF ' t ' | uniq -c | grep -v '^ *1 ' | sort -rn | grep ' spin' >> 473 000000000000000b t spin_unlock_irqrestore >> 292 000000000000000b t spin_unlock >> 215 000000000000000b t spin_lock >> 134 000000000000000b t spin_unlock_irq >> 130 000000000000000b t spin_unlock_bh >> 120 000000000000000b t spin_lock_irq >> 106 000000000000000b t spin_lock_bh >> >> Disassembly: >> >> ffffffff81004720 <spin_lock>: >> ffffffff81004720: 55 push %rbp >> ffffffff81004721: 48 89 e5 mov %rsp,%rbp >> ffffffff81004724: e8 f8 4e e2 02 callq <_raw_spin_lock> >> ffffffff81004729: 5d pop %rbp >> ffffffff8100472a: c3 retq >> >> This patch fixes this via s/inline/__always_inline/ in spinlock.h. >> This decreases vmlinux by about 30k: >> >> text data bss dec hex filename >> 82375570 22255544 20627456 125258570 7774b4a vmlinux.before >> 82335059 22255416 20627456 125217931 776ac8b vmlinux > > See also https://lkml.org/lkml/2015/4/23/598 ("enforce function > inlining for hot functions"). > > Presumably Hagen didn't see the issue with spinlock functions. I > wonder why not. > > I suppose we should get both these consolidated into a coherent whole. > > It's a bit irritating to have to do this: presumably gcc will get fixed > and the huge sprinkling of __always_inline will become less and less > relevant over time and people will have trouble distinguishing "real > __always_inline which was put here for a purpose" from "dopey > __always_inline to work around a short-term gcc glitch".
In my patches, I put __always_inline *only* on functions where my measurements show a large size decrease from doing so. *Not* on functions where "I think it may be a good idea".
So far, all such functions were so trivial that inlining decision there is a no-brainer.
> and then use inline_for_broken_gcc everywhere. That way, the reason > for the marker is self-explanatory and we can later hunt all these > things down and remvoe them. > > Also, the inline_for_broken_gcc definition can be made dependent on > particular gcc versions, which will allow us to easily keep an eye on > the behaviour of later gcc versions.
I've seen it on gcc-4.7.2 and gcc-4.9.2, so this behavior is not limited to a narrow range of gcc versions. I'd say by now about half of running kernels can easily be affected.
-- vda
| |