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.

> -FUNC(sk_load_word_positive_offset)
> +ENTRY(sk_load_word_positive_offset)
>       mov     %r9d,%eax               # hlen
>       sub     %esi,%eax               # hlen - offset
>       cmp     $3,%eax
> @@ -40,12 +36,14 @@ FUNC(sk_load_word_positive_offset)
>       mov     (SKBDATA,%rsi),%eax
>       bswap   %eax                    /* ntohl() */
>       ret
> +ENDPROC(sk_load_word_positive_offset)
>  
> -FUNC(sk_load_half)
> +ENTRY(sk_load_half)
>       test    %esi,%esi
>       js      bpf_slow_path_half_neg
> +ENDPROC(sk_load_half)
>  
> -FUNC(sk_load_half_positive_offset)
> +ENTRY(sk_load_half_positive_offset)
>       mov     %r9d,%eax
>       sub     %esi,%eax               #       hlen - offset
>       cmp     $1,%eax
> @@ -53,16 +51,19 @@ FUNC(sk_load_half_positive_offset)
>       movzwl  (SKBDATA,%rsi),%eax
>       rol     $8,%ax                  # ntohs()
>       ret
> +ENDPROC(sk_load_half_positive_offset)
>  
> -FUNC(sk_load_byte)
> +ENTRY(sk_load_byte)
>       test    %esi,%esi
>       js      bpf_slow_path_byte_neg
> +ENDPROC(sk_load_byte)
>  
> -FUNC(sk_load_byte_positive_offset)
> +ENTRY(sk_load_byte_positive_offset)
>       cmp     %esi,%r9d   /* if (offset >= hlen) goto bpf_slow_path_byte */
>       jle     bpf_slow_path_byte
>       movzbl  (SKBDATA,%rsi),%eax
>       ret
> +ENDPROC(sk_load_byte_positive_offset)
>  
>  /* rsi contains offset and can be scratched */
>  #define bpf_slow_path_common(LEN)            \
> @@ -119,31 +120,34 @@ bpf_slow_path_word_neg:
>       cmp     SKF_MAX_NEG_OFF, %esi   /* test range */
>       jl      bpf_error       /* offset lower -> error  */
>  
> -FUNC(sk_load_word_negative_offset)
> +ENTRY(sk_load_word_negative_offset)
>       sk_negative_common(4)
>       mov     (%rax), %eax
>       bswap   %eax
>       ret
> +ENDPROC(sk_load_word_negative_offset)
>  
>  bpf_slow_path_half_neg:
>       cmp     SKF_MAX_NEG_OFF, %esi
>       jl      bpf_error
>  
> -FUNC(sk_load_half_negative_offset)
> +ENTRY(sk_load_half_negative_offset)
>       sk_negative_common(2)
>       mov     (%rax),%ax
>       rol     $8,%ax
>       movzwl  %ax,%eax
>       ret
> +ENDPROC(sk_load_half_negative_offset)
>  
>  bpf_slow_path_byte_neg:
>       cmp     SKF_MAX_NEG_OFF, %esi
>       jl      bpf_error
>  
> -FUNC(sk_load_byte_negative_offset)
> +ENTRY(sk_load_byte_negative_offset)
>       sk_negative_common(1)
>       movzbl  (%rax), %eax
>       ret
> +ENDPROC(sk_load_byte_negative_offset)
>  
>  bpf_error:
>  # force a return 0 from jit handler
> -- 
> 2.12.2
> 

Reply via email to