public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
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


  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