Messages in this thread | | | Date | Thu, 22 Nov 2018 12:13:41 +0100 | From | Ingo Molnar <> | Subject | Re: [PATCH] x86: only use ERMS for user copies for larger sizes |
| |
* Ingo Molnar <mingo@kernel.org> wrote:
> The kernel text size reduction with Jen's patch is small but real: > > text data bss dec hex filename > 19572694 11516934 19873888 50963516 309a43c vmlinux.before > 19572468 11516934 19873888 50963290 309a35a vmlinux.after > > But I checked the disassembly, and it's not a real win, the new code is > actually more complex than the old one, as expected, but GCC (7.3.0) does > some particularly stupid things which bloats the generated code.
So I dug into this some more:
1)
Firstly I tracked down GCC bloating the might_fault() checks and the related out-of-line code exception handling which bloats the full generated function.
2)
But with even that complication eliminated, there's a size reduction when Jen's patch is applied, which is puzzling:
19563640 11516790 19882080 50962510 309a04e vmlinux.before 19563274 11516790 19882080 50962144 3099ee0 vmlinux.after
but this is entirely due to the .altinstructions section being counted as 'text' part of the vmlinux - while in reality it's not:
3)
The _real_ part of the vmlinux gets bloated by Jen's patch:
ffffffff81000000 <_stext>:
before: ffffffff81b0e5e0 <__clear_user> after: ffffffff81b0e670 <__clear_user>:
I.e. we get a e5e0 => e670 bloat, as expected.
In the config I tested a later section of the kernel image first aligns away the bloat:
before: ffffffff82fa6321 <.altinstr_aux>: after: ffffffff82fa6321 <.altinstr_aux>:
and then artificially debloats the modified kernel via the altinstructions section:
before: Disassembly of section .exit.text: ffffffff83160798 <intel_uncore_exit> after: Disassembly of section .exit.text: ffffffff83160608 <intel_uncore_exit>
Note that there's a third level of obfuscation here: Jen's patch actually *adds* a new altinstructions statement:
+ /* + * For smaller copies, don't use ERMS as it's slower. + */ + if (len < 128) { + alternative_call(copy_user_generic_unrolled, + copy_user_generic_string, X86_FEATURE_REP_GOOD, + ASM_OUTPUT2("=a" (ret), "=D" (to), "=S" (from), + "=d" (len)), + "1" (to), "2" (from), "3" (len) + : "memory", "rcx", "r8", "r9", "r10", "r11"); + return ret; + } + /* * If CPU has ERMS feature, use copy_user_enhanced_fast_string. * Otherwise, if CPU has rep_good feature, use copy_user_generic_string. * Otherwise, use copy_user_generic_unrolled. */ alternative_call_2(copy_user_generic_unrolled, - copy_user_generic_string, - X86_FEATURE_REP_GOOD, - copy_user_enhanced_fast_string, - X86_FEATURE_ERMS, + copy_user_generic_string, X86_FEATURE_REP_GOOD, + copy_user_enhanced_fast_string, X86_FEATURE_ERMS, ASM_OUTPUT2("=a" (ret), "=D" (to), "=S" (from), "=d" (len)), "1" (to), "2" (from), "3" (len)
So how can this change possibly result in a *small* altinstructions section?
4)
The reason is GCC's somewhat broken __builtin_constant() logic, which leaves ~10% of the constant call sites actually active, but which are then optimized by GCC's later stages, and the alternative_call_2() gets optimized out and replaced with the alternative_call() call.
This is where Jens's patch 'debloats' the vmlinux and confuses the 'size' utility and gains its code reduction street cred.
Note to self: watch out for patches that change altinstructions and don't make premature vmlinux size impact assumptions. :-)
Thanks,
Ingo
| |