* [PATCH bpf-next 0/2] bpf, mips: Do some small changes @ 2021-10-09 10:57 Tiezhu Yang 2021-10-09 10:57 ` [PATCH bpf-next 1/2] bpf, mips: Clean up config options about JIT Tiezhu Yang 2021-10-09 10:57 ` [PATCH bpf-next 2/2] bpf, mips: Modify check condition about tail call count Tiezhu Yang 0 siblings, 2 replies; 5+ messages in thread From: Tiezhu Yang @ 2021-10-09 10:57 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Thomas Bogendoerfer, Johan Almbladh, Paul Burton Cc: netdev, bpf, linux-mips, linux-kernel, Xuefeng Li, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh This patchset is based on bpf-next tree: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git Tiezhu Yang (2): bpf, mips: Clean up config options about JIT bpf, mips: Modify check condition about tail call count arch/mips/Kconfig | 9 --------- arch/mips/net/Makefile | 6 +++--- arch/mips/net/bpf_jit_comp32.c | 4 ++-- 3 files changed, 5 insertions(+), 14 deletions(-) -- 2.1.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH bpf-next 1/2] bpf, mips: Clean up config options about JIT 2021-10-09 10:57 [PATCH bpf-next 0/2] bpf, mips: Do some small changes Tiezhu Yang @ 2021-10-09 10:57 ` Tiezhu Yang 2021-10-10 17:49 ` Johan Almbladh 2021-10-09 10:57 ` [PATCH bpf-next 2/2] bpf, mips: Modify check condition about tail call count Tiezhu Yang 1 sibling, 1 reply; 5+ messages in thread From: Tiezhu Yang @ 2021-10-09 10:57 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Thomas Bogendoerfer, Johan Almbladh, Paul Burton Cc: netdev, bpf, linux-mips, linux-kernel, Xuefeng Li, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh The config options MIPS_CBPF_JIT and MIPS_EBPF_JIT are useless, remove them in arch/mips/Kconfig, and then modify arch/mips/net/Makefile. Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> --- arch/mips/Kconfig | 9 --------- arch/mips/net/Makefile | 6 +++--- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index 38468f4..9b03c78 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -1214,15 +1214,6 @@ config SYS_SUPPORTS_RELOCATABLE The platform must provide plat_get_fdt() if it selects CONFIG_USE_OF to allow access to command line and entropy sources. -config MIPS_CBPF_JIT - def_bool y - depends on BPF_JIT && HAVE_CBPF_JIT - -config MIPS_EBPF_JIT - def_bool y - depends on BPF_JIT && HAVE_EBPF_JIT - - # # Endianness selection. Sufficiently obscure so many users don't know what to # answer,so we try hard to limit the available choices. Also the use of a diff --git a/arch/mips/net/Makefile b/arch/mips/net/Makefile index 95e8267..e3e6ae6 100644 --- a/arch/mips/net/Makefile +++ b/arch/mips/net/Makefile @@ -1,10 +1,10 @@ # SPDX-License-Identifier: GPL-2.0-only # MIPS networking code -obj-$(CONFIG_MIPS_EBPF_JIT) += bpf_jit_comp.o +obj-$(CONFIG_BPF_JIT) += bpf_jit_comp.o ifeq ($(CONFIG_32BIT),y) - obj-$(CONFIG_MIPS_EBPF_JIT) += bpf_jit_comp32.o + obj-$(CONFIG_BPF_JIT) += bpf_jit_comp32.o else - obj-$(CONFIG_MIPS_EBPF_JIT) += bpf_jit_comp64.o + obj-$(CONFIG_BPF_JIT) += bpf_jit_comp64.o endif -- 2.1.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf, mips: Clean up config options about JIT 2021-10-09 10:57 ` [PATCH bpf-next 1/2] bpf, mips: Clean up config options about JIT Tiezhu Yang @ 2021-10-10 17:49 ` Johan Almbladh 0 siblings, 0 replies; 5+ messages in thread From: Johan Almbladh @ 2021-10-10 17:49 UTC (permalink / raw) To: Tiezhu Yang Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Thomas Bogendoerfer, Paul Burton, Networking, bpf, linux-mips, linux-kernel, Xuefeng Li, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh On Sat, Oct 9, 2021 at 12:58 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote: > > The config options MIPS_CBPF_JIT and MIPS_EBPF_JIT are useless, remove > them in arch/mips/Kconfig, and then modify arch/mips/net/Makefile. > > Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> > --- > arch/mips/Kconfig | 9 --------- > arch/mips/net/Makefile | 6 +++--- > 2 files changed, 3 insertions(+), 12 deletions(-) > > diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig > index 38468f4..9b03c78 100644 > --- a/arch/mips/Kconfig > +++ b/arch/mips/Kconfig > @@ -1214,15 +1214,6 @@ config SYS_SUPPORTS_RELOCATABLE > The platform must provide plat_get_fdt() if it selects CONFIG_USE_OF > to allow access to command line and entropy sources. > > -config MIPS_CBPF_JIT > - def_bool y > - depends on BPF_JIT && HAVE_CBPF_JIT > - > -config MIPS_EBPF_JIT > - def_bool y > - depends on BPF_JIT && HAVE_EBPF_JIT > - > - > # > # Endianness selection. Sufficiently obscure so many users don't know what to > # answer,so we try hard to limit the available choices. Also the use of a > diff --git a/arch/mips/net/Makefile b/arch/mips/net/Makefile > index 95e8267..e3e6ae6 100644 > --- a/arch/mips/net/Makefile > +++ b/arch/mips/net/Makefile > @@ -1,10 +1,10 @@ > # SPDX-License-Identifier: GPL-2.0-only > # MIPS networking code > > -obj-$(CONFIG_MIPS_EBPF_JIT) += bpf_jit_comp.o > +obj-$(CONFIG_BPF_JIT) += bpf_jit_comp.o > > ifeq ($(CONFIG_32BIT),y) > - obj-$(CONFIG_MIPS_EBPF_JIT) += bpf_jit_comp32.o > + obj-$(CONFIG_BPF_JIT) += bpf_jit_comp32.o > else > - obj-$(CONFIG_MIPS_EBPF_JIT) += bpf_jit_comp64.o > + obj-$(CONFIG_BPF_JIT) += bpf_jit_comp64.o > endif > -- > 2.1.0 > Looks good to me. Acked-by: Johan Almbladh <johan.almbladh@anyfinetworks.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH bpf-next 2/2] bpf, mips: Modify check condition about tail call count 2021-10-09 10:57 [PATCH bpf-next 0/2] bpf, mips: Do some small changes Tiezhu Yang 2021-10-09 10:57 ` [PATCH bpf-next 1/2] bpf, mips: Clean up config options about JIT Tiezhu Yang @ 2021-10-09 10:57 ` Tiezhu Yang 2021-10-10 17:30 ` Johan Almbladh 1 sibling, 1 reply; 5+ messages in thread From: Tiezhu Yang @ 2021-10-09 10:57 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Thomas Bogendoerfer, Johan Almbladh, Paul Burton Cc: netdev, bpf, linux-mips, linux-kernel, Xuefeng Li, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh In emit_tail_call() of bpf_jit_comp32.c, "blez t2" (t2 <= 0) is not consistent with the comment "t2 < 0", modify the check condition to keep consistency. Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> --- arch/mips/net/bpf_jit_comp32.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/mips/net/bpf_jit_comp32.c b/arch/mips/net/bpf_jit_comp32.c index 9d7041a..b887c01 100644 --- a/arch/mips/net/bpf_jit_comp32.c +++ b/arch/mips/net/bpf_jit_comp32.c @@ -1312,12 +1312,12 @@ static int emit_tail_call(struct jit_context *ctx) emit(ctx, sltu, t1, ind, t1); /* t1 = ind < t1 */ emit(ctx, beqz, t1, get_offset(ctx, 1)); /* PC += off(1) if t1 == 0 */ /* (next insn delay slot) */ - /* if (TCC-- <= 0) goto out */ + /* if (--TCC < 0) goto out */ emit(ctx, lw, t2, ctx->stack_size, MIPS_R_SP); /* t2 = *(SP + size) */ emit_load_delay(ctx); /* Load delay slot */ - emit(ctx, blez, t2, get_offset(ctx, 1)); /* PC += off(1) if t2 < 0 */ emit(ctx, addiu, t2, t2, -1); /* t2-- (delay slot) */ emit(ctx, sw, t2, ctx->stack_size, MIPS_R_SP); /* *(SP + size) = t2 */ + emit(ctx, bltz, t2, get_offset(ctx, 1)); /* PC += off(1) if t2 < 0 */ /* prog = ary->ptrs[ind] */ off = offsetof(struct bpf_array, ptrs); -- 2.1.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next 2/2] bpf, mips: Modify check condition about tail call count 2021-10-09 10:57 ` [PATCH bpf-next 2/2] bpf, mips: Modify check condition about tail call count Tiezhu Yang @ 2021-10-10 17:30 ` Johan Almbladh 0 siblings, 0 replies; 5+ messages in thread From: Johan Almbladh @ 2021-10-10 17:30 UTC (permalink / raw) To: Tiezhu Yang Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Thomas Bogendoerfer, Paul Burton, Networking, bpf, linux-mips, linux-kernel, Xuefeng Li, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh On Sat, Oct 9, 2021 at 12:58 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote: > > In emit_tail_call() of bpf_jit_comp32.c, "blez t2" (t2 <= 0) is not > consistent with the comment "t2 < 0", modify the check condition to > keep consistency. > > Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> > --- > arch/mips/net/bpf_jit_comp32.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/mips/net/bpf_jit_comp32.c b/arch/mips/net/bpf_jit_comp32.c > index 9d7041a..b887c01 100644 > --- a/arch/mips/net/bpf_jit_comp32.c > +++ b/arch/mips/net/bpf_jit_comp32.c > @@ -1312,12 +1312,12 @@ static int emit_tail_call(struct jit_context *ctx) > emit(ctx, sltu, t1, ind, t1); /* t1 = ind < t1 */ > emit(ctx, beqz, t1, get_offset(ctx, 1)); /* PC += off(1) if t1 == 0 */ > /* (next insn delay slot) */ > - /* if (TCC-- <= 0) goto out */ > + /* if (--TCC < 0) goto out */ > emit(ctx, lw, t2, ctx->stack_size, MIPS_R_SP); /* t2 = *(SP + size) */ > emit_load_delay(ctx); /* Load delay slot */ > - emit(ctx, blez, t2, get_offset(ctx, 1)); /* PC += off(1) if t2 < 0 */ > emit(ctx, addiu, t2, t2, -1); /* t2-- (delay slot) */ > emit(ctx, sw, t2, ctx->stack_size, MIPS_R_SP); /* *(SP + size) = t2 */ > + emit(ctx, bltz, t2, get_offset(ctx, 1)); /* PC += off(1) if t2 < 0 */ If the comment is not consistent with the code, and the code is correct, why did you change the code? Have you seen the JIT fail on any of the tail call test cases? The current code works as intended. The t2 register is decremented in the branch delay slot of the blez. After your change, the the comment still says "delay slot", but it is no longer in the delay slot of a branch. Instead the next instruction emitted, not visible in the patch context, fills the delay slot of the bltz. In this case it probably is ok, but if that instruction is also a branch, the result would be unpredictable. I prefer to emit the delay slot instruction immediately after the branch is emitted when possible. If a branch and its delay slot is separated in the JIT logic, it makes the JIT more brittle IMO. Please keep the original logic, but update the blez comment so it is consistent with the code. > > /* prog = ary->ptrs[ind] */ > off = offsetof(struct bpf_array, ptrs); > -- > 2.1.0 > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-10-10 17:49 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-09 10:57 [PATCH bpf-next 0/2] bpf, mips: Do some small changes Tiezhu Yang 2021-10-09 10:57 ` [PATCH bpf-next 1/2] bpf, mips: Clean up config options about JIT Tiezhu Yang 2021-10-10 17:49 ` Johan Almbladh 2021-10-09 10:57 ` [PATCH bpf-next 2/2] bpf, mips: Modify check condition about tail call count Tiezhu Yang 2021-10-10 17:30 ` Johan Almbladh
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).