Re: [PATCH] bpf: Tweak BPF jump table optimizations for objtool compatibility

From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	netdev@vger.kernel.org, bpf@vger.kernel.org,
	linux-kernel@vger.kernel.org, x86@kernel.org,
	Peter Zijlstra <peterz@infradead.org>,
	Randy Dunlap <rdunlap@infradead.org>,
	Arnd Bergmann <arnd@arndb.de>
Subject: Re: [PATCH] bpf: Tweak BPF jump table optimizations for objtool compatibility
Date: Tue, 5 May 2020 13:11:08 -0500	[thread overview]
Message-ID: <20200505181108.hwcqanvw3qf5qyxk@treble> (raw)
In-Reply-To: <20200505174300.gech3wr5v6kkho35@ast-mbp.dhcp.thefacebook.com>

On Tue, May 05, 2020 at 10:43:00AM -0700, Alexei Starovoitov wrote:
> > Or, if you want to minimize the patch's impact on other arches, and keep
> > the current patch the way it is (with bug fixed and changed patch
> > description), that's fine too.  I can change the patch description
> > accordingly.
> > 
> > Or if you want me to measure the performance impact of the +40% code
> > growth, and *then* decide what to do, that's also fine.  But you'd need
> > to tell me what tests to run.
> 
> I'd like to minimize the risk and avoid code churn,
> so how about we step back and debug it first?
> Which version of gcc are you using and what .config?
> I've tried:
> Linux version 5.7.0-rc2 (gcc version 10.0.1 20200505 (prerelease) (GCC)
> CONFIG_UNWINDER_ORC=y
> # CONFIG_RETPOLINE is not set
> 
> and objtool didn't complain.
> I would like to reproduce it first before making any changes.

Revert

  3193c0836f20 ("bpf: Disable GCC -fgcse optimization for ___bpf_prog_run()")

and compile with retpolines off (and either ORC or FP, doesn't matter).

I'm using GCC 9.3.1:

  kernel/bpf/core.o: warning: objtool: ___bpf_prog_run()+0x8dc: sibling call from callable instruction with modified stack frame

That's the original issue described in that commit.

> Also since objtool cannot follow the optimizations compiler is doing
> how about admit the design failure and teach objtool to build ORC
> (and whatever else it needs to build) based on dwarf for the functions where
> it cannot understand the assembly code ?
> Otherwise objtool will forever be playing whackamole with compilers.

I agree it's not a good long term approach.  But DWARF has its own
issues and we can't rely on it for live patching.

As I mentioned we have a plan to use a compiler plugin to annotate jump
tables (including GCC switch tables).  But the approach taken by this
patch should be good enough for now.

-- 
Josh