All of lore.kernel.org
 help / color / mirror / Atom feed
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

  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.