Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752828AbdHOPZp (ORCPT ); Tue, 15 Aug 2017 11:25:45 -0400 Received: from mail.kernel.org ([198.145.29.99]:36654 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752514AbdHOPZo (ORCPT ); Tue, 15 Aug 2017 11:25:44 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org F340722C90 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: Tue, 15 Aug 2017 12:25:40 -0300 From: Arnaldo Carvalho de Melo To: Thomas Richter Cc: wangnan0@huawei.com, linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org, brueckner@linux.vnet.ibm.com Subject: Re: [PATCHv3] perf bpf: Fix endianness problem when loading parameters in prologue Message-ID: <20170815152540.GB22988@kernel.org> References: <20170815092159.31912-1-tmricht@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170815092159.31912-1-tmricht@linux.vnet.ibm.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: 3890 Lines: 125 Em Tue, Aug 15, 2017 at 11:21:59AM +0200, Thomas Richter escreveu: Ok, I'm applying this, the only missing bit was the following line, right at the start of the patch message body; From: Wang Nan To state that the patch was wrote by Wang, the fact that you participated in it with some adjustment is implied by having you sign off the patch, ok? - Arnaldo > 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: Wang Nan > Signed-off-by: Thomas Richter > --- > 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 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); > > -- > 2.9.3