From: Jiri Slaby <[email protected]> Date: Mon, 24 Apr 2017 08:45:11 +0200
> On 04/21/2017, 09:32 PM, Alexei Starovoitov wrote: >> On Fri, Apr 21, 2017 at 04:12:43PM +0200, Jiri Slaby wrote: >>> Do not use a custom macro FUNC for starts of the global functions, use >>> ENTRY instead. >>> >>> And while at it, annotate also ends of the functions by ENDPROC. >>> >>> Signed-off-by: Jiri Slaby <[email protected]> >>> Cc: "David S. Miller" <[email protected]> >>> Cc: Alexey Kuznetsov <[email protected]> >>> Cc: James Morris <[email protected]> >>> Cc: Hideaki YOSHIFUJI <[email protected]> >>> Cc: Patrick McHardy <[email protected]> >>> Cc: Thomas Gleixner <[email protected]> >>> Cc: Ingo Molnar <[email protected]> >>> Cc: "H. Peter Anvin" <[email protected]> >>> Cc: [email protected] >>> Cc: [email protected] >>> --- >>> arch/x86/net/bpf_jit.S | 32 ++++++++++++++++++-------------- >>> 1 file changed, 18 insertions(+), 14 deletions(-) >>> >>> diff --git a/arch/x86/net/bpf_jit.S b/arch/x86/net/bpf_jit.S >>> index f2a7faf4706e..762c29fb8832 100644 >>> --- a/arch/x86/net/bpf_jit.S >>> +++ b/arch/x86/net/bpf_jit.S >>> @@ -23,16 +23,12 @@ >>> 32 /* space for rbx,r13,r14,r15 */ + \ >>> 8 /* space for skb_copy_bits */) >>> >>> -#define FUNC(name) \ >>> - .globl name; \ >>> - .type name, @function; \ >>> - name: >>> - >>> -FUNC(sk_load_word) >>> +ENTRY(sk_load_word) >>> test %esi,%esi >>> js bpf_slow_path_word_neg >>> +ENDPROC(sk_load_word) >> >> this doens't look right. >> It will add alignment nops in critical paths of these pseudo functions. >> I'm also not sure whether it will still work afterwards. >> Was it tested? >> I'd prefer if this code kept as-is. > > It cannot stay as-is simply because we want to know where the functions > end to inject debuginfo properly. The code above does not warrant for > any exception. I totally and completely disagree. > Executing a nop takes a little and having externally-callable functions > aligned can actually help performance (no, I haven't measured nor tested > the code). But sure, the tool is generic, so I can introduce a local > macros to avoid alignments in the functions: Not for this case, it's a bunch of entry points all packed together intentionally so that SKB accesses of different access sizes (which is almost always the case) from BPF programs use the smallest amount of I-cache as possible.
