From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Hagen Paul Pfeifer <hagen@jauu.net>
Cc: Markus Trippelsdorf <markus@trippelsdorf.de>,
Andrew Morton <akpm@linux-foundation.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
"David S. Miller" <davem@davemloft.net>,
x86@kernel.org
Subject: Re: [PATCH] enforce function inlining for hot functions
Date: Sat, 25 Apr 2015 06:51:35 -0700 [thread overview]
Message-ID: <20150425135135.GN5561@linux.vnet.ibm.com> (raw)
In-Reply-To: <CAPh34mfAhXqfd8wOuXUM_VSHOWUxh-S3o-Lz=0+sRKBzz-Pe6Q@mail.gmail.com>
On Sat, Apr 25, 2015 at 03:26:48PM +0200, Hagen Paul Pfeifer wrote:
> On 25 April 2015 at 12:31, Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
>
> > 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.
>
> Hey Paul,
>
> yes, with DEBUG_LOCK_ALLOC disabled all rcu_read_lock and unlock
> functions are perfectly inlined.
Whew!!! ;-)
> So now we have the following
> situation: depending on the gcc version and the particular kernel
> configuration some hot functions are not inlined - they are duplicated
> hundred times. Which is bad no matter how you consider
> gcc/kernel-configuration. I think this should *never* happened.
>
> With the patch we can make sure that hot functions are *always*
> inlined - no matter what gcc version and kernel configuration is used.
>
> Furthermore, as Markus already noted: compiled with -O2 this do not
> happened. Duplicates are only generated for -Os[1]. Ok, but now the
> question: should this happened for Os? I don't think so. I think we
> can do it better and mark these few functions as always inline. For
> the remaining inlined marked function we should provide gcc the
> flexibility and do not artificially enforce inlining. The current
> situation is bad: OPTIMIZE_INLINING is default no, which defacto
> enforces inlining for *all* inlined marked functions. GCC inlining
> mechanism is defacto disabled, which is also bad. Last but not least:
> the patch do not change anything for the current user, because we will
> still disable OPTIMIZE_INLINING (resulting in __always_inline for all
> inlined marked functions). The patch effects users who enable
> OPTIMIZE_INLINING and trust the compiler.
>
> Hagen
>
> PS: thank you Markus for the comment.
>
> [1] which is nonsense: the functions are not inlined yet, but are
> copied hundred times for "size optimized builds". gcc should rather
> redeclare the functions global, define it one time and call this
> function every time. But implementing such a scheme is probably a
> monster of itself and LTO is required so solve all issues with such a
> concept.
I am guessing that there is only one duplicate per compilation unit?
I would also guess that the LTO guys would have a ready solution. ;-)
That said, if a function was invoked extremely many times, it might
make sense to duplicate it even within a single compilation unit if
doing so allowed saving more than the size of the function in the
form of call instructions with shorter address fields. But I have
no idea whether or not gcc would do this sort of thing.
Thanx, Paul
next prev parent reply other threads:[~2015-04-25 13:51 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-23 21:40 [PATCH] enforce function inlining for hot functions Hagen Paul Pfeifer
2015-04-24 19:49 ` Andrew Morton
2015-04-24 20:13 ` Paul E. McKenney
2015-04-24 20:44 ` Hagen Paul Pfeifer
2015-04-24 23:10 ` Hagen Paul Pfeifer
2015-04-25 10:31 ` Paul E. McKenney
2015-04-25 13:26 ` Hagen Paul Pfeifer
2015-04-25 13:51 ` Paul E. McKenney [this message]
2015-04-24 20:39 ` Hagen Paul Pfeifer
2015-04-25 10:53 ` Markus Trippelsdorf
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=20150425135135.GN5561@linux.vnet.ibm.com \
--to=paulmck@linux.vnet.ibm.com \
--cc=akpm@linux-foundation.org \
--cc=davem@davemloft.net \
--cc=hagen@jauu.net \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=markus@trippelsdorf.de \
--cc=mingo@redhat.com \
--cc=x86@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox