Re: [PATCH] Revert "x86/uaccess: Add stack frame output operand in get_user() inline asm"

[Josh Poimboeuf]

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