Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753714AbdHKTXt (ORCPT ); Fri, 11 Aug 2017 15:23:49 -0400 Received: from mail.kernel.org ([198.145.29.99]:57850 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753654AbdHKTXs (ORCPT ); Fri, 11 Aug 2017 15:23:48 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 52A7622BCD Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=acme@kernel.org Date: Fri, 11 Aug 2017 16:23:44 -0300 From: Arnaldo Carvalho de Melo To: Thomas-Mich Richter , "Wangnan (F)" Cc: tmricht@linux.vnet.ibm.com, linux-kernel@vger.kernel.org, Li Zefan Subject: Re: [PATCH] perf bpf: Fix endianness problem when loading parameters in prologue Message-ID: <20170811192344.GA6073@kernel.org> References: <20170812184959.32694-1-wangnan0@huawei.com> <0e9620f7-e0d9-e8a4-ee87-3284bc9b8cc2@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0e9620f7-e0d9-e8a4-ee87-3284bc9b8cc2@huawei.com> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.8.3 (2017-05-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3916 Lines: 117 Em Fri, Aug 11, 2017 at 06:47:56PM +0800, Wangnan (F) escreveu: > Hi Thomas, > > Please try this patch on your machine and give me the result. Right, I'm waiting for test results for the last two patches from Wang: (3.0K) [PATCH] perf bpf: Fix endianness problem when loading parameters in prologue (1.2K) [PATCH] perf test llvm: Fix f_mode endianness problem Thanks, - Arnaldo > Thank you. > > On 2017/8/13 2:49, Wang Nan 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. > > > > Signed-off-by: Wang Nan > > Cc: Arnaldo Carvalho de Melo > > Cc: Thomas Richter > > Cc: Alexei Starovoitov > > Cc: Hendrik Brueckner > > Cc: Li Zefan > > --- > > tools/perf/tests/bpf-script-test-prologue.c | 4 ++- > > tools/perf/util/bpf-prologue.c | 49 +++++++++++++++++++++++++++-- > > 2 files changed, 50 insertions(+), 3 deletions(-) > > > > diff --git a/tools/perf/tests/bpf-script-test-prologue.c b/tools/perf/tests/bpf-script-test-prologue.c > > index b4ebc75..43f1e16 100644 > > --- a/tools/perf/tests/bpf-script-test-prologue.c > > +++ b/tools/perf/tests/bpf-script-test-prologue.c > > @@ -26,9 +26,11 @@ static void (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) = > > (void *) 6; > > SEC("func=null_lseek file->f_mode offset orig") > > -int bpf_func__null_lseek(void *ctx, int err, unsigned long f_mode, > > +int bpf_func__null_lseek(void *ctx, int err, unsigned long _f_mode, > > unsigned long offset, unsigned long orig) > > { > > + fmode_t f_mode = (fmode_t)_f_mode; > > + > > if (err) > > return 0; > > if (f_mode & FMODE_WRITE) > > diff --git a/tools/perf/util/bpf-prologue.c b/tools/perf/util/bpf-prologue.c > > index 6cdbee1..ce28993 100644 > > --- a/tools/perf/util/bpf-prologue.c > > +++ b/tools/perf/util/bpf-prologue.c > > @@ -57,6 +57,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)) > > @@ -257,9 +297,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 = argtype_to_ldx_size(args[i].type); > > + > > + 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); >