Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1710802 > unrolled thread

Re: perf bpf: Reworked fix endianness problem when loading parameters in prologue

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: perf bpf: Reworked fix endianness problem when loading parameters  in prologue "Wangnan (F)" <wangnan0@huawei.com> - 2017-08-14 12:40 +0200
    Re: perf bpf: Reworked fix endianness problem when loading parameters  in prologue Thomas-Mich Richter <tmricht@linux.vnet.ibm.com> - 2017-08-14 13:00 +0200
      Re: perf bpf: Reworked fix endianness problem when loading parameters  in prologue "Wangnan (F)" <wangnan0@huawei.com> - 2017-08-14 13:10 +0200

#1710802 — Re: perf bpf: Reworked fix endianness problem when loading parameters in prologue

Hi Thomas,

Your patch looks good to me. I've tested in my environment and it works.
Please resend it to lkml and let Arnaldo to collect it.

Thank you.

On 2017/8/14 17:47, Thomas Richter wrote:
> Perf BPF prologue generator unconditionally fetches 8 bytes for function
> parameters, which causes problem on big endian machine. Thomas gives a
> detail analysis for this problem:
>
>   http://lkml.kernel.org/r/968ebda5-abe4-8830-8d69-49f62529d151@linux.vnet.ibm.com
>
> This patch parses the type of each argument and converts data from
> memory to expected type.
>
> Now the test runs successfully on 4.13.0-rc5:
> [root@s8360046 perf]# ./perf test  bpf
> 38: BPF filter                                 :
> 38.1: Basic BPF filtering                      : Ok
> 38.2: BPF pinning                              : Ok
> 38.3: BPF prologue generation                  : Ok
> 38.4: BPF relocation checker                   : Ok
> [root@s8360046 perf]#
>
> Signed-off-by: Thomas Richter <tmricht@linux.vnet.ibm.com>
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Thomas Richter <tmricht@linux.vnet.ibm.com>
> Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Cc: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
> Cc: Li Zefan <lizefan@huawei.com>
> ---
>   tools/perf/util/bpf-prologue.c | 49 ++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/bpf-prologue.c b/tools/perf/util/bpf-prologue.c
> index 1356220..827f914 100644
> --- a/tools/perf/util/bpf-prologue.c
> +++ b/tools/perf/util/bpf-prologue.c
> @@ -58,6 +58,46 @@ check_pos(struct bpf_insn_pos *pos)
>   	return 0;
>   }
>   
> +/*
> + * Convert type string (u8/u16/u32/u64/s8/s16/s32/s64 ..., see
> + * Documentation/trace/kprobetrace.txt) to size field of BPF_LDX_MEM
> + * instruction (BPF_{B,H,W,DW}).
> + */
> +static int
> +argtype_to_ldx_size(const char *type)
> +{
> +	int arg_size = type ? atoi(&type[1]) : 64;
> +
> +	switch (arg_size) {
> +	case 8:
> +		return BPF_B;
> +	case 16:
> +		return BPF_H;
> +	case 32:
> +		return BPF_W;
> +	case 64:
> +	default:
> +		return BPF_DW;
> +	}
> +}
> +
> +static const char *
> +insn_sz_to_str(int insn_sz)
> +{
> +	switch (insn_sz) {
> +	case BPF_B:
> +		return "BPF_B";
> +	case BPF_H:
> +		return "BPF_H";
> +	case BPF_W:
> +		return "BPF_W";
> +	case BPF_DW:
> +		return "BPF_DW";
> +	default:
> +		return "UNKNOWN";
> +	}
> +}
> +
>   /* Give it a shorter name */
>   #define ins(i, p) append_insn((i), (p))
>   
> @@ -258,9 +298,14 @@ gen_prologue_slowpath(struct bpf_insn_pos *pos,
>   	}
>   
>   	/* Final pass: read to registers */
> -	for (i = 0; i < nargs; i++)
> -		ins(BPF_LDX_MEM(BPF_DW, BPF_PROLOGUE_START_ARG_REG + i,
> +	for (i = 0; i < nargs; i++) {
> +		int insn_sz = (args[i].ref) ? argtype_to_ldx_size(args[i].type) : BPF_DW;
> +
> +		pr_debug("prologue: load arg %d, insn_sz is %s\n",
> +			 i, insn_sz_to_str(insn_sz));
> +		ins(BPF_LDX_MEM(insn_sz, BPF_PROLOGUE_START_ARG_REG + i,
>   				BPF_REG_FP, -BPF_REG_SIZE * (i + 1)), pos);
> +	}
>   
>   	ins(BPF_JMP_IMM(BPF_JA, BPF_REG_0, 0, JMP_TO_SUCCESS_CODE), pos);
>   

[toc] | [next] | [standalone]

#1710812

On 08/14/2017 12:30 PM, Wangnan (F) wrote:
> Hi Thomas,
> 
> Your patch looks good to me. I've tested in my environment and it works.
> Please resend it to lkml and let Arnaldo to collect it.
> 
> Thank you.
> 

Will do, I take this as an Acked-by and Tested-by, ok?

Thanks
-- 
Thomas Richter, Dept 3303, IBM LTC Boeblingen Germany
--
Vorsitzende des Aufsichtsrats: Martina Koederitz 
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294

[toc] | [prev] | [next] | [standalone]

#1710828

OK.


On 2017/8/14 18:58, Thomas-Mich Richter wrote:
> On 08/14/2017 12:30 PM, Wangnan (F) wrote:
>> Hi Thomas,
>>
>> Your patch looks good to me. I've tested in my environment and it works.
>> Please resend it to lkml and let Arnaldo to collect it.
>>
>> Thank you.
>>
> Will do, I take this as an Acked-by and Tested-by, ok?
>
> Thanks

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web