From: Hagen Paul Pfeifer <hagen@jauu.net>
To: Ingo Molnar <mingo@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
Borislav Petkov <bp@alien8.de>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
Peter Zijlstra <peterz@infradead.org>,
Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH RFC] x86: enforce inlining for atomics
Date: Wed, 22 Apr 2015 00:57:23 +0200 [thread overview]
Message-ID: <20150421225723.GA5927@virgo.local> (raw)
In-Reply-To: <20150421074212.GA25081@gmail.com>
* Ingo Molnar | 2015-04-21 09:42:12 [+0200]:
Hey Ingo,
>So the thing is that allyesconfig turns on -Os:
>
> CONFIG_CC_OPTIMIZE_FOR_SIZE=y
CONFIG_CC_OPTIMIZE_FOR_SIZE seems to have no effect, The only option which
makes a difference is CONFIG_OPTIMIZE_INLINING! But this is not a big surprise:
*disabling* CONFIG_OPTIMIZE_INLINING substitudes _all_ inlines with
__attribute__((always_inline)).
"If unsure, say N." -> results in configurations with always_inline.
So I tested again, one time with unset CONFIG_OPTIMIZE_INLINING the result
seems fine:
show_temp: 59 duplicates
char2uni: 52 duplicates
uni2char: 52 duplicates
sd_probe: 49 duplicates
sd_driver_init: 48 duplicates
sd_driver_exit: 48 duplicates
usb_serial_module_exit: 47 duplicates
[...]
We see ordinary "template" reuse of common driver code without renaming the
copied static's. But compiled with CONFIG_OPTIMIZE_INLINING=y the inlining is
not respected by gcc:
atomic_inc: 544 duplicates
rcu_read_unlock: 453 duplicates
rcu_read_lock: 383 duplicates
get_dma_ops: 271 duplicates
arch_local_irq_restore: 258 duplicates
atomic_dec: 215 duplicates
kzalloc: 185 duplicates
test_and_set_bit: 156 duplicates
cpumask_check: 148 duplicates
cpumask_next: 146 duplicates
list_del: 131 duplicates
kref_get: 126 duplicates
test_and_clear_bit: 122 duplicates
brelse: 122 duplicates
schedule_work: 122 duplicates
netif_tx_stop_queue: 115 duplicates
atomic_dec_and_test: 107 duplicates
dma_mapping_error: 105 duplicates
list_del_init: 101 duplicates
netif_stop_queue: 100 duplicates
arch_local_save_flags: 98 duplicates
tasklet_schedule: 76 duplicates
clk_prepare_enable: 71 duplicates
init_completion: 69 duplicates
pskb_may_pull: 67 duplicates
[...]
Again, the used gcc version is "gcc (Debian 4.9.2-10) 4.9.2". So it is not
outdated nor a legacy one. The inline heuristic seems really broken for some
parts. Is it possible that gcc is bedeviled because of inline assembler
parts which brings confuse the internal scoring system?
I suggest the following: I prepare a patch series for the most obvious
candidates and substituting inline with __always_inline (probably ~50
functions). Each subsystem maintainer can check and ACK the patch. This has the
benefit that for all other locations gcc is still responsible for inlining
decision. Enforcing inlining via __always_inline for all inline marked function
is probably too hard!? In 2015 gcc is still not able to inline single line
statements - that's strange.
Linus, ack?
Hagen
next prev parent reply other threads:[~2015-04-21 22:57 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-20 21:27 [PATCH RFC] x86: enforce inlining for atomics Hagen Paul Pfeifer
2015-04-20 21:56 ` Borislav Petkov
2015-04-20 22:08 ` Hagen Paul Pfeifer
2015-04-21 7:42 ` Ingo Molnar
2015-04-21 10:56 ` Hagen Paul Pfeifer
2015-04-21 22:57 ` Hagen Paul Pfeifer [this message]
2015-04-22 0:57 ` Linus Torvalds
2015-04-22 5:24 ` Markus Trippelsdorf
2015-04-22 5:58 ` Hagen Paul Pfeifer
2015-04-22 9:20 ` Markus Trippelsdorf
2015-04-22 9:28 ` Ingo Molnar
2015-04-22 9:31 ` Hagen Paul Pfeifer
2015-04-30 1:52 ` Henrique de Moraes Holschuh
2015-07-13 18:27 ` Denys Vlasenko
2015-07-13 19:20 ` Hagen Paul Pfeifer
2015-07-13 21:06 ` Denys Vlasenko
2015-07-13 21:12 ` Hagen Paul Pfeifer
2015-07-13 18:25 ` Denys Vlasenko
2015-04-20 23:46 ` Peter Zijlstra
2015-04-22 14:10 ` [tip:x86/asm] x86/asm: Always inline atomics tip-bot for Hagen Paul Pfeifer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150421225723.GA5927@virgo.local \
--to=hagen@jauu.net \
--cc=akpm@linux-foundation.org \
--cc=bp@alien8.de \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.