From: Nadav Amit <namit@vmware.com>
To: Ingo Molnar <mingo@kernel.org>
Cc: "hpa@zytor.com" <hpa@zytor.com>, Ingo Molnar <mingo@redhat.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"x86@kernel.org" <x86@kernel.org>,
Thomas Gleixner <tglx@linutronix.de>,
Jan Beulich <JBeulich@suse.com>,
Josh Poimboeuf <jpoimboe@redhat.com>,
Linus Torvalds <torvalds@linux-foundation.org>,
Peter Zijlstra <a.p.zijlstra@chello.nl>,
Andy Lutomirski <luto@kernel.org>
Subject: Re: [PATCH v9 04/10] x86: refcount: prevent gcc distortions
Date: Thu, 4 Oct 2018 10:23:29 +0000 [thread overview]
Message-ID: <29591D3B-D49B-4D7A-B280-85A2C3F63F9C@vmware.com> (raw)
In-Reply-To: <20181004094519.GA97692@gmail.com>
at 2:45 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Nadav Amit <namit@vmware.com> wrote:
>
>>> Another, separate question I wanted to ask: how do we ensure that the kernel stays fixed?
>>> I.e. is there some tooling we can use to actually measure whether there's bad inlining decisions
>>> done, to detect all these bad patterns that cause bad GCC code generation?
>>
>> Good question. First, I’ll indicate that this patch-set does not handle all
>> the issues. There is still the issue of conditional use of
>> __builtin_constant_p().
>>
>> One indication for bad inlining decisions is the inlined functions have
>> multiple (non-inlined) instances in the binary and are short. I don’t
>> have an automatic solution, but you can try, for example to run:
>>
>> nm --print-size ./vmlinux | grep ' t ' | cut -d' ' -f2- | sort | uniq -c | \
>> grep -v '^ 1' | sort -n -r | head -n 5
>>
>> There are however many false positives. After these patches, for example, I
>> get:
>>
>> 11 000000000000012f t jhash
>> 7 0000000000000017 t dst_output
>> 6 0000000000000011 t kzalloc
>> 5 000000000000002f t acpi_os_allocate_zeroed
>> 5 0000000000000029 t acpi_os_allocate
>>
>>
>> jhash() should not have been inlined in my mind, and should have a
>> non-inlined implementation. dst_output() is used as a function pointer.
>> kzalloc() and the next two suffer from the __builtin_constant_p() problem I
>> described in the past.
>
> Ok, that's useful info.
>
> The histogram suggests that with all your patches applied the kernel is now in a pretty good
> state in terms of inlining decisions, right?
It was just an example that I ran on the kernel I built right now (with a
custom config). Please don’t regard these results as anything indicative.
> Are you using defconfig or a reasonable distro-config for your tests?
I think it is best to take the kernel and run localyesconfig for testing.
Taking Ubuntu 18.04 and doing the same gives the following results:
12 000000000000012f t jhash
7 0000000000000017 t dst_output
7 0000000000000014 t init_once
5 00000000000000d8 t jhash2
5 000000000000004e t put_page
5 000000000000002f t acpi_os_allocate_zeroed
5 0000000000000029 t acpi_os_allocate
5 0000000000000028 t umask_show
5 0000000000000011 t kzalloc
4 0000000000000053 t trace_xhci_dbg_quirks
Not awful, but not great.
It is a bit hard to fix the __builtin_constant_p() problem without having
some negative side-effects.
Reminder: __builtin_constant_p() is evaluated after the inlining decision
are done. You can use __builtin_choose_expr() instead of an “if”s and
instead of ternary operators when evaluating __builtin_constant_p() to solve
the problem. However, this causes the compiler not to know sometimes that a
value is constant because __builtin_choose_expr () evaluation happens too
early. This __builtin_choose_expr() problem is the reason for put_page() and
kzalloc() are not being inlined. Clang, again, does not suffer from this
problem.
Anyhow, it may be a good practice to try to get rid of the rest. For
example, dst_discard() has four instances because it is always given as a
function pointer. So it should not have been inlined.
Regards,
Nadav
next prev parent reply other threads:[~2018-10-04 10:23 UTC|newest]
Thread overview: 116+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-03 21:30 [PATCH v9 00/10] x86: macrofying inline asm Nadav Amit
2018-10-03 21:30 ` [PATCH v9 01/10] xtensa: defining LINKER_SCRIPT for the linker script Nadav Amit
2018-10-04 10:00 ` [tip:x86/build] kbuild/arch/xtensa: Define " tip-bot for Nadav Amit
2018-10-03 21:30 ` [PATCH v9 02/10] Makefile: Prepare for using macros for inline asm Nadav Amit
2018-10-04 10:01 ` [tip:x86/build] kbuild/Makefile: Prepare for using macros in inline assembly code to work around asm() related GCC inlining bugs tip-bot for Nadav Amit
2018-11-06 18:57 ` [PATCH v9 02/10] Makefile: Prepare for using macros for inline asm Logan Gunthorpe
2018-11-06 19:18 ` Nadav Amit
2018-11-06 20:01 ` Logan Gunthorpe
2018-11-07 18:01 ` Nadav Amit
2018-11-07 18:53 ` Logan Gunthorpe
2018-11-07 18:56 ` Nadav Amit
2018-11-07 21:43 ` Logan Gunthorpe
2018-11-07 21:50 ` hpa
2018-11-08 6:18 ` Nadav Amit
2018-11-08 17:14 ` Logan Gunthorpe
2018-11-08 19:54 ` Nadav Amit
2018-11-08 20:00 ` Logan Gunthorpe
2018-11-08 20:18 ` Nadav Amit
2018-11-10 22:04 ` Nadav Amit
2018-11-13 4:56 ` Logan Gunthorpe
2018-10-03 21:30 ` [PATCH v9 03/10] x86: objtool: use asm macro for better compiler decisions Nadav Amit
2018-10-04 10:02 ` [tip:x86/build] x86/objtool: Use asm macros to work around GCC inlining bugs tip-bot for Nadav Amit
2018-10-03 21:30 ` [PATCH v9 04/10] x86: refcount: prevent gcc distortions Nadav Amit
2018-10-04 7:57 ` Ingo Molnar
2018-10-04 8:33 ` Ingo Molnar
2018-10-04 8:40 ` hpa
2018-10-04 8:56 ` Ingo Molnar
2018-10-04 8:56 ` Nadav Amit
2018-10-04 9:02 ` hpa
2018-10-04 9:16 ` Ingo Molnar
2018-10-04 19:33 ` H. Peter Anvin
2018-10-04 20:05 ` Nadav Amit
2018-10-04 20:08 ` H. Peter Anvin
2018-10-04 20:29 ` Andy Lutomirski
2018-10-04 23:11 ` H. Peter Anvin
2018-10-06 1:40 ` Rasmus Villemoes
2018-10-04 9:12 ` Ingo Molnar
2018-10-04 9:17 ` hpa
2018-10-04 9:30 ` Nadav Amit
2018-10-04 9:45 ` Ingo Molnar
2018-10-04 10:23 ` Nadav Amit [this message]
2018-10-05 9:31 ` Ingo Molnar
2018-10-05 11:20 ` Borislav Petkov
2018-10-05 12:52 ` Ingo Molnar
2018-10-05 20:27 ` [PATCH 0/3] Macrofying inline asm rebased Nadav Amit
2018-10-05 20:27 ` [PATCH 1/3] x86/extable: Macrofy inline assembly code to work around GCC inlining bugs Nadav Amit
2018-10-06 14:42 ` [tip:x86/build] " tip-bot for Nadav Amit
2018-10-05 20:27 ` [PATCH 2/3] x86/cpufeature: " Nadav Amit
2018-10-06 14:43 ` [tip:x86/build] " tip-bot for Nadav Amit
2018-10-05 20:27 ` [PATCH 3/3] x86/jump-labels: " Nadav Amit
2018-10-06 14:44 ` [tip:x86/build] " tip-bot for Nadav Amit
2018-10-08 2:17 ` [PATCH v9 04/10] x86: refcount: prevent gcc distortions Nadav Amit
2018-10-04 8:40 ` Nadav Amit
2018-10-04 9:01 ` Ingo Molnar
2018-10-04 10:02 ` [tip:x86/build] x86/refcount: Work around GCC inlining bug tip-bot for Nadav Amit
2018-10-03 21:30 ` [PATCH v9 05/10] x86: alternatives: macrofy locks for better inlining Nadav Amit
2018-10-04 10:03 ` [tip:x86/build] x86/alternatives: Macrofy lock prefixes to work around GCC inlining bugs tip-bot for Nadav Amit
2018-10-03 21:30 ` [PATCH v9 06/10] x86: bug: prevent gcc distortions Nadav Amit
2018-10-04 10:03 ` [tip:x86/build] x86/bug: Macrofy the BUG table section handling, to work around GCC inlining bugs tip-bot for Nadav Amit
2018-10-03 21:30 ` [PATCH v9 07/10] x86: prevent inline distortion by paravirt ops Nadav Amit
2018-10-04 10:04 ` [tip:x86/build] x86/paravirt: Work around GCC inlining bugs when compiling " tip-bot for Nadav Amit
2018-10-03 21:30 ` [PATCH v9 08/10] x86: extable: use macros instead of inline assembly Nadav Amit
2018-10-03 21:30 ` [PATCH v9 09/10] x86: cpufeature: " Nadav Amit
2018-10-03 21:31 ` [PATCH v9 10/10] x86: jump-labels: " Nadav Amit
2018-10-07 9:18 ` PROPOSAL: Extend inline asm syntax with size spec Borislav Petkov
[not found] ` <20181007132228.GJ29268@gate.crashing.org>
2018-10-07 14:13 ` Borislav Petkov
2018-10-07 15:14 ` Segher Boessenkool
2018-10-08 5:58 ` Ingo Molnar
2018-10-08 7:53 ` Segher Boessenkool
2018-10-07 15:53 ` Michael Matz
2018-10-08 6:13 ` Ingo Molnar
2018-10-08 8:18 ` Segher Boessenkool
2018-10-08 7:31 ` Segher Boessenkool
2018-10-08 9:07 ` Richard Biener
2018-10-08 10:02 ` Segher Boessenkool
2018-10-09 14:53 ` Segher Boessenkool
2018-10-10 6:35 ` Ingo Molnar
2018-10-10 7:12 ` Richard Biener
2018-10-10 7:22 ` Ingo Molnar
2018-10-10 8:03 ` Segher Boessenkool
2018-10-10 8:19 ` Borislav Petkov
2018-10-10 8:35 ` Richard Biener
2018-10-10 18:54 ` Segher Boessenkool
2018-10-10 19:14 ` Borislav Petkov
2018-10-13 19:33 ` Borislav Petkov
2018-10-13 21:14 ` Alexander Monakov
2018-10-13 21:30 ` Borislav Petkov
2018-10-25 10:24 ` Borislav Petkov
2018-10-31 12:55 ` Peter Zijlstra
2018-10-31 13:11 ` Peter Zijlstra
2018-10-31 16:31 ` Segher Boessenkool
2018-11-01 5:20 ` Joe Perches
2018-11-01 9:01 ` Peter Zijlstra
2018-11-01 9:20 ` Joe Perches
2018-11-01 11:15 ` Peter Zijlstra
2018-12-27 4:47 ` Masahiro Yamada
2018-10-10 10:29 ` Richard Biener
2018-10-10 7:53 ` Segher Boessenkool
2018-10-10 16:31 ` Nadav Amit
2018-10-10 19:21 ` Segher Boessenkool
2018-10-11 7:04 ` Richard Biener
2018-11-29 11:46 ` Masahiro Yamada
2018-11-29 12:25 ` Segher Boessenkool
2018-11-30 9:06 ` Boris Petkov
2018-11-30 13:16 ` Segher Boessenkool
2018-12-10 8:16 ` Masahiro Yamada
2018-11-29 13:07 ` Borislav Petkov
2018-11-29 13:09 ` Richard Biener
2018-11-29 13:16 ` Borislav Petkov
2018-11-29 13:24 ` Richard Biener
2018-10-08 16:24 ` David Laight
2018-10-07 16:09 ` Nadav Amit
2018-10-07 16:46 ` Richard Biener
2018-10-07 19:06 ` Nadav Amit
2018-10-07 19:52 ` Jeff Law
2018-10-08 7:46 ` Richard Biener
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=29591D3B-D49B-4D7A-B280-85A2C3F63F9C@vmware.com \
--to=namit@vmware.com \
--cc=JBeulich@suse.com \
--cc=a.p.zijlstra@chello.nl \
--cc=hpa@zytor.com \
--cc=jpoimboe@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mingo@kernel.org \
--cc=mingo@redhat.com \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
--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;
as well as URLs for NNTP newsgroup(s).