From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754745AbcAVRgT (ORCPT ); Fri, 22 Jan 2016 12:36:19 -0500 Received: from mx1.redhat.com ([209.132.183.28]:42644 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754590AbcAVRgR (ORCPT ); Fri, 22 Jan 2016 12:36:17 -0500 Date: Fri, 22 Jan 2016 11:36:14 -0600 From: Josh Poimboeuf To: Alexei Starovoitov Cc: Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , x86@kernel.org, linux-kernel@vger.kernel.org, live-patching@vger.kernel.org, Michal Marek , Peter Zijlstra , Andy Lutomirski , Borislav Petkov , Linus Torvalds , Andi Kleen , Pedro Alves , Namhyung Kim , Bernd Petrovitsch , Chris J Arges , Andrew Morton , Jiri Slaby , Arnaldo Carvalho de Melo , Alexei Starovoitov , netdev@vger.kernel.org, daniel@iogearbox.net Subject: Re: [PATCH 23/33] x86/asm/bpf: Create stack frames in bpf_jit.S Message-ID: <20160122173614.GG20502@treble.redhat.com> References: <20160122024427.GA6000@ast-mbp.thefacebook.com> <20160122035531.GA20502@treble.redhat.com> <20160122041845.GA7175@ast-mbp.thefacebook.com> <20160122155804.GC20502@treble.redhat.com> <20160122171823.GC9608@ast-mbp.thefacebook.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20160122171823.GC9608@ast-mbp.thefacebook.com> User-Agent: Mutt/1.5.23.1-rc1 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jan 22, 2016 at 09:18:23AM -0800, Alexei Starovoitov wrote: > On Fri, Jan 22, 2016 at 09:58:04AM -0600, Josh Poimboeuf wrote: > > On Thu, Jan 21, 2016 at 08:18:46PM -0800, Alexei Starovoitov wrote: > > > On Thu, Jan 21, 2016 at 09:55:31PM -0600, Josh Poimboeuf wrote: > > > > On Thu, Jan 21, 2016 at 06:44:28PM -0800, Alexei Starovoitov wrote: > > > > > On Thu, Jan 21, 2016 at 04:49:27PM -0600, Josh Poimboeuf wrote: > > > > > > bpf_jit.S has several callable non-leaf functions which don't honor > > > > > > CONFIG_FRAME_POINTER, which can result in bad stack traces. > > > > > > > > > > > > Create a stack frame before the call instructions when > > > > > > CONFIG_FRAME_POINTER is enabled. > > > > > > > > > > > > Signed-off-by: Josh Poimboeuf > > > > > > Cc: Alexei Starovoitov > > > > > > Cc: netdev@vger.kernel.org > > > > > > --- > > > > > > arch/x86/net/bpf_jit.S | 9 +++++++-- > ... > > > > > > /* rsi contains offset and can be scratched */ > > > > > > #define bpf_slow_path_common(LEN) \ > > > > > > + lea -MAX_BPF_STACK + 32(%rbp), %rdx;\ > > > > > > + FRAME_BEGIN; \ > > > > > > mov %rbx, %rdi; /* arg1 == skb */ \ > > > > > > push %r9; \ > > > > > > push SKBDATA; \ > > > > > > /* rsi already has offset */ \ > > > > > > mov $LEN,%ecx; /* len */ \ > > > > > > - lea - MAX_BPF_STACK + 32(%rbp),%rdx; \ > > > > > > call skb_copy_bits; \ > > > > > > test %eax,%eax; \ > > > > > > pop SKBDATA; \ > > > > > > - pop %r9; > > > > > > + pop %r9; \ > > > > > > + FRAME_END > ... > > > > Well, but the point of these patches isn't to make the tool happy. It's > > > > really to make sure that runtime stack traces can be made reliable. > > > > Maybe I'm missing something but I don't see why JIT code can't honor > > > > CONFIG_FRAME_POINTER just like any other code. > > > > > > It can if there is no performance cost added. > > > > CONFIG_FRAME_POINTER always adds a small performance cost but as you > > mentioned it only affects the slow path here. And hopefully we'll soon > > have an in-kernel DWARF unwinder on x86 so we can get rid of the need > > for frame pointers. > > ok. fair enough. > Acked-by: Alexei Starovoitov Thanks! Can I assume your ack also applies to the previous patch which adds the ELF annotations ("x86/asm/bpf: Annotate callable functions")? -- Josh