Re: [PATCH 7/9] x86/unwind/orc: Fall back to using frame pointers for generated code

From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: x86@kernel.org, linux-kernel@vger.kernel.org,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	netdev@vger.kernel.org, bpf@vger.kernel.org,
	Peter Zijlstra <peterz@infradead.org>,
	Song Liu <songliubraving@fb.com>, Kairui Song <kasong@redhat.com>
Subject: Re: [PATCH 7/9] x86/unwind/orc: Fall back to using frame pointers for generated code
Date: Thu, 13 Jun 2019 18:42:45 -0700	[thread overview]
Message-ID: <20190614014244.st7fbr6areazmyrb@ast-mbp.dhcp.thefacebook.com> (raw)
In-Reply-To: <20190614013051.6gnwduy4dsygbamj@treble>

On Thu, Jun 13, 2019 at 08:30:51PM -0500, Josh Poimboeuf wrote:
> On Thu, Jun 13, 2019 at 03:00:55PM -0700, Alexei Starovoitov wrote:
> > > @@ -392,8 +402,16 @@ bool unwind_next_frame(struct unwind_state *state)
> > >  	 * calls and calls to noreturn functions.
> > >  	 */
> > >  	orc = orc_find(state->signal ? state->ip : state->ip - 1);
> > > -	if (!orc)
> > > -		goto err;
> > > +	if (!orc) {
> > > +		/*
> > > +		 * As a fallback, try to assume this code uses a frame pointer.
> > > +		 * This is useful for generated code, like BPF, which ORC
> > > +		 * doesn't know about.  This is just a guess, so the rest of
> > > +		 * the unwind is no longer considered reliable.
> > > +		 */
> > > +		orc = &orc_fp_entry;
> > > +		state->error = true;
> > 
> > That seems fragile.
> 
> I don't think so.  The unwinder has sanity checks to make sure it
> doesn't go off the rails.  And it works just fine.  The beauty is that
> it should work for all generated code (not just BPF).
> 
> > Can't we populate orc_unwind tables after JIT ?
> 
> As I mentioned it would introduce a lot more complexity.  For each JIT
> function, BPF would have to tell ORC the following:
> 
> - where the BPF function lives
> - how big the stack frame is
> - where RBP and other callee-saved regs are on the stack

that sounds like straightforward addition that ORC should have anyway.
right now we're not using rbp in the jit-ed code,
but one day we definitely will.
Same goes for r12. It's reserved right now for 'strategic use'.
We've been thinking to add another register to bpf isa.
It will map to r12 on x86. arm64 and others have plenty of regs to use.
The programs are getting bigger and register spill/fill starting to
become a performance concern. Extra register will give us more room.