Messages in this thread | | | Date | Fri, 21 Jul 2017 08:24:52 -0500 | From | Josh Poimboeuf <> | Subject | Re: [PATCH] Revert "x86/uaccess: Add stack frame output operand in get_user() inline asm" |
| |
On Fri, Jul 21, 2017 at 12:13:31PM +0300, Andrey Ryabinin wrote: > > Still, unfortunately, I don't think that's going to work for GCC. > > Changing the '__sp' register variable to global in the header file > > causes it to make a *bunch* of changes across the kernel, even in > > functions which don't do inline asm. It seems to be disabling some > > optimizations across the board. > > All I see is just bunch of reordering of independent instructions, like this: > > -ffffffff81012760: 5b pop %rbx > -ffffffff81012761: 31 c0 xor %eax,%eax > +ffffffff81012760: 31 c0 xor %eax,%eax > +ffffffff81012762: 5b pop %rbx > > -ffffffff810c29ae: 48 83 c4 28 add $0x28,%rsp > -ffffffff810c29b2: 89 d8 mov %ebx,%eax > +ffffffff810c29ae: 89 d8 mov %ebx,%eax > +ffffffff810c29b0: 48 83 c4 28 add $0x28,%rsp > > I haven't noticed any single bad/harmful change. The size of .text remained the same.
I compiled with -ffunction-sections to make the comparisons easier. The reordering is much more extreme than your example. (This is with GCC 7, btw). And it's not just reordering of instructions. It's control flow changes as well.
Also, the text size grew a little:
text data bss dec hex filename 10630602 8295074 16461824 35387500 21bf86c vmlinux.before 10634013 8295074 16461824 35390911 21c05bf vmlinux.after
A small two-line change, which is supposed to be a noop, or at least should only affect a small number of functions, but which instead affects optimization decisions across the entire kernel, is actively harmful IMO.
> And btw, arm/arm64 already use global current_stack_pointer just fine.
I wonder if they looked for the impact.
-- Josh
| |