LinuxLists.cc - [PATCH bpf-next 0/2] bpf, riscv: use BPF prog pack allocator in BPF JIT

2023-07-20 16:11:50

Subject: [PATCH bpf-next 0/2] bpf, riscv: use BPF prog pack allocator in BPF JIT

BPF programs currently consume a page each on RISCV. For systems with many BPF
programs, this adds significant pressure to instruction TLB. High iTLB pressure
usually causes slow down for the whole system.

Song Liu introduced the BPF prog pack allocator[1] to mitigate the above issue.
It packs multiple BPF programs into a single huge page. It is currently only
enabled for the x86_64 BPF JIT.

I enabled this allocator on the ARM64 BPF JIT[2]. It is being reviewed now.

This patch series enables the BPF prog pack allocator for the RISCV BPF JIT.
This series needs a patch[3] from the ARM64 series to work.

======================================================
Performance Analysis of prog pack allocator on RISCV64
======================================================

Test setup:
===========

Host machine: Debian GNU/Linux 11 (bullseye)
Qemu Version: QEMU emulator version 8.0.3 (Debian 1:8.0.3+dfsg-1)
u-boot-qemu Version: 2023.07+dfsg-1
opensbi Version: 1.3-1

To test the performance of the BPF prog pack allocator on RV, a stresser
tool[4] linked below was built. This tool loads 8 BPF programs on the system and
triggers 5 of them in an infinite loop by doing system calls.

The runner script starts 20 instances of the above which loads 8*20=160 BPF
programs on the system, 5*20=100 of which are being constantly triggered.
The script is passed a command which would be run in the above environment.

The script was run with following perf command:
./run.sh "perf stat -a \
-e iTLB-load-misses \
-e dTLB-load-misses \
-e dTLB-store-misses \
-e instructions \
--timeout 60000"

The output of the above command is discussed below before and after enabling the
BPF prog pack allocator.

The tests were run on qemu-system-riscv64 with 8 cpus, 16G memory. The rootfs
was created using Bjorn's riscv-cross-builder[5] docker container linked below.

Results
=======

Before enabling prog pack allocator:
------------------------------------

Performance counter stats for 'system wide':

4939048 iTLB-load-misses
5468689 dTLB-load-misses
465234 dTLB-store-misses
1441082097998 instructions

60.045791200 seconds time elapsed

After enabling prog pack allocator:
-----------------------------------

Performance counter stats for 'system wide':

3430035 iTLB-load-misses
5008745 dTLB-load-misses
409944 dTLB-store-misses
1441535637988 instructions

60.046296600 seconds time elapsed

Improvements in metrics
=======================

It was expected that the iTLB-load-misses would decrease as now a single huge
page is used to keep all the BPF programs compared to a single page for each
program earlier.

--------------------------------------------
The improvement in iTLB-load-misses: -30.5 %
--------------------------------------------

I repeated this expriment more than 100 times in different setups and the
improvement was always greater than 30%.

This patch series is boot tested on the Starfive VisionFive 2 board[6].
The performance analysis was not done on the board because it doesn't
expose iTLB-load-misses, etc. The stresser program was run on the board to test
the loading and unloading of BPF programs

[1] https://lore.kernel.org/bpf/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/
[3] https://lore.kernel.org/all/[email protected]/
[4] https://github.com/puranjaymohan/BPF-Allocator-Bench
[5] https://github.com/bjoto/riscv-cross-builder
[6] https://www.starfivetech.com/en/site/boards

Puranjay Mohan (2):
riscv: Extend patch_text_nosync() for multiple pages
bpf, riscv: use prog pack allocator in the BPF JIT

arch/riscv/kernel/patch.c | 29 ++++++--
arch/riscv/net/bpf_jit.h | 3 +
arch/riscv/net/bpf_jit_comp64.c | 56 ++++++++++++---
arch/riscv/net/bpf_jit_core.c | 117 +++++++++++++++++++++++++++-----
4 files changed, 174 insertions(+), 31 deletions(-)

--
2.40.1

2023-07-20 16:33:38

by Puranjay Mohan

[permalink] [raw]

Subject: [PATCH bpf-next 2/2] bpf, riscv: use prog pack allocator in the BPF JIT

Use bpf_jit_binary_pack_alloc() for memory management of JIT binaries in
RISCV BPF JIT. The bpf_jit_binary_pack_alloc creates a pair of RW and RX
buffers. The JIT writes the program into the RW buffer. When the JIT is
done, the program is copied to the final RX buffer with
bpf_jit_binary_pack_finalize.

Implement bpf_arch_text_copy() and bpf_arch_text_invalidate() for RISCV
JIT as these functions are required by bpf_jit_binary_pack allocator.

Signed-off-by: Puranjay Mohan <[email protected]>
---
arch/riscv/net/bpf_jit.h | 3 +
arch/riscv/net/bpf_jit_comp64.c | 56 ++++++++++++---
arch/riscv/net/bpf_jit_core.c | 117 +++++++++++++++++++++++++++-----
3 files changed, 150 insertions(+), 26 deletions(-)

diff --git a/arch/riscv/net/bpf_jit.h b/arch/riscv/net/bpf_jit.h
index 2717f5490428..ad69319c8ea7 100644
--- a/arch/riscv/net/bpf_jit.h
+++ b/arch/riscv/net/bpf_jit.h
@@ -68,6 +68,7 @@ static inline bool is_creg(u8 reg)
struct rv_jit_context {
struct bpf_prog *prog;
u16 *insns; /* RV insns */
+ u16 *ro_insns;
int ninsns;
int prologue_len;
int epilogue_offset;
@@ -85,7 +86,9 @@ static inline int ninsns_rvoff(int ninsns)

struct rv_jit_data {
struct bpf_binary_header *header;
+ struct bpf_binary_header *ro_header;
u8 *image;
+ u8 *ro_image;
struct rv_jit_context ctx;
};

diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
index c648864c8cd1..3d6165c6608b 100644
--- a/arch/riscv/net/bpf_jit_comp64.c
+++ b/arch/riscv/net/bpf_jit_comp64.c
@@ -142,7 +142,11 @@ static bool in_auipc_jalr_range(s64 val)
/* Emit fixed-length instructions for address */
static int emit_addr(u8 rd, u64 addr, bool extra_pass, struct rv_jit_context *ctx)
{
- u64 ip = (u64)(ctx->insns + ctx->ninsns);
+ /*
+ * Use the ro_insns(RX) to calculate the offset as the BPF program will
+ * finally run from this memory region.
+ */
+ u64 ip = (u64)(ctx->ro_insns + ctx->ninsns);
s64 off = addr - ip;
s64 upper = (off + (1 << 11)) >> 12;
s64 lower = off & 0xfff;
@@ -463,7 +467,11 @@ static int emit_call(u64 addr, bool fixed_addr, struct rv_jit_context *ctx)
u64 ip;

if (addr && ctx->insns) {
- ip = (u64)(long)(ctx->insns + ctx->ninsns);
+ /*
+ * Use the ro_insns(RX) to calculate the offset as the BPF
+ * program will finally run from this memory region.
+ */
+ ip = (u64)(long)(ctx->ro_insns + ctx->ninsns);
off = addr - ip;
}

@@ -576,7 +584,8 @@ static int add_exception_handler(const struct bpf_insn *insn,
{
struct exception_table_entry *ex;
unsigned long pc;
- off_t offset;
+ off_t ins_offset;
+ off_t fixup_offset;

if (!ctx->insns || !ctx->prog->aux->extable || BPF_MODE(insn->code) != BPF_PROBE_MEM)
return 0;
@@ -591,12 +600,17 @@ static int add_exception_handler(const struct bpf_insn *insn,
return -EINVAL;

ex = &ctx->prog->aux->extable[ctx->nexentries];
- pc = (unsigned long)&ctx->insns[ctx->ninsns - insn_len];
+ pc = (unsigned long)&ctx->ro_insns[ctx->ninsns - insn_len];

- offset = pc - (long)&ex->insn;
- if (WARN_ON_ONCE(offset >= 0 || offset < INT_MIN))
+ /*
+ * This is the relative offset of the instruction that may fault from
+ * the exception table itself. This will be written to the exception
+ * table and if this instruction faults, the destination register will
+ * be set to '0' and the execution will jump to the next instruction.
+ */
+ ins_offset = pc - (long)&ex->insn;
+ if (WARN_ON_ONCE(ins_offset >= 0 || ins_offset < INT_MIN))
return -ERANGE;
- ex->insn = offset;

/*
* Since the extable follows the program, the fixup offset is always
@@ -605,12 +619,25 @@ static int add_exception_handler(const struct bpf_insn *insn,
* bits. We don't need to worry about buildtime or runtime sort
* modifying the upper bits because the table is already sorted, and
* isn't part of the main exception table.
+ *
+ * The fixup_offset is set to the next instruction from the instruction
+ * that may fault. The execution will jump to this after handling the
+ * fault.
*/
- offset = (long)&ex->fixup - (pc + insn_len * sizeof(u16));
- if (!FIELD_FIT(BPF_FIXUP_OFFSET_MASK, offset))
+ fixup_offset = (long)&ex->fixup - (pc + insn_len * sizeof(u16));
+ if (!FIELD_FIT(BPF_FIXUP_OFFSET_MASK, fixup_offset))
return -ERANGE;

- ex->fixup = FIELD_PREP(BPF_FIXUP_OFFSET_MASK, offset) |
+ /*
+ * The offsets above have been calculated using the RO buffer but we
+ * need to use the R/W buffer for writes.
+ * switch ex to rw buffer for writing.
+ */
+ ex = (void *)ctx->insns + ((void *)ex - (void *)ctx->ro_insns);
+
+ ex->insn = ins_offset;
+
+ ex->fixup = FIELD_PREP(BPF_FIXUP_OFFSET_MASK, fixup_offset) |
FIELD_PREP(BPF_FIXUP_REG_MASK, dst_reg);
ex->type = EX_TYPE_BPF;

@@ -995,6 +1022,7 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,

ctx.ninsns = 0;
ctx.insns = NULL;
+ ctx.ro_insns = NULL;
ret = __arch_prepare_bpf_trampoline(im, m, tlinks, func_addr, flags, &ctx);
if (ret < 0)
return ret;
@@ -1003,7 +1031,15 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
return -EFBIG;

ctx.ninsns = 0;
+ /*
+ * The bpf_int_jit_compile() uses a RW buffer (ctx.insns) to write the
+ * JITed instructions and later copies it to a RX region (ctx.ro_insns).
+ * It also uses ctx.ro_insns to calculate offsets for jumps etc. As the
+ * trampoline image uses the same memory area for writing and execution,
+ * both ctx.insns and ctx.ro_insns can be set to image.
+ */
ctx.insns = image;
+ ctx.ro_insns = image;
ret = __arch_prepare_bpf_trampoline(im, m, tlinks, func_addr, flags, &ctx);
if (ret < 0)
return ret;
diff --git a/arch/riscv/net/bpf_jit_core.c b/arch/riscv/net/bpf_jit_core.c
index 7a26a3e1c73c..b0fa0084f493 100644
--- a/arch/riscv/net/bpf_jit_core.c
+++ b/arch/riscv/net/bpf_jit_core.c
@@ -8,6 +8,8 @@

#include <linux/bpf.h>
#include <linux/filter.h>
+#include <linux/memory.h>
+#include <asm/patch.h>
#include "bpf_jit.h"

/* Number of iterations to try until offsets converge. */
@@ -117,16 +119,27 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
sizeof(struct exception_table_entry);
prog_size = sizeof(*ctx->insns) * ctx->ninsns;

- jit_data->header =
- bpf_jit_binary_alloc(prog_size + extable_size,
- &jit_data->image,
- sizeof(u32),
- bpf_fill_ill_insns);
- if (!jit_data->header) {
+ jit_data->ro_header =
+ bpf_jit_binary_pack_alloc(prog_size +
+ extable_size,
+ &jit_data->ro_image,
+ sizeof(u32),
+ &jit_data->header,
+ &jit_data->image,
+ bpf_fill_ill_insns);
+ if (!jit_data->ro_header) {
prog = orig_prog;
goto out_offset;
}

+ /*
+ * Use the image(RW) for writing the JITed instructions. But also save
+ * the ro_image(RX) for calculating the offsets in the image. The RW
+ * image will be later copied to the RX image from where the program
+ * will run. The bpf_jit_binary_pack_finalize() will do this copy in the
+ * final step.
+ */
+ ctx->ro_insns = (u16 *)jit_data->ro_image;
ctx->insns = (u16 *)jit_data->image;
/*
* Now, when the image is allocated, the image can
@@ -138,14 +151,12 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)

if (i == NR_JIT_ITERATIONS) {
pr_err("bpf-jit: image did not converge in <%d passes!\n", i);
- if (jit_data->header)
- bpf_jit_binary_free(jit_data->header);
prog = orig_prog;
- goto out_offset;
+ goto out_free_hdr;
}

if (extable_size)
- prog->aux->extable = (void *)ctx->insns + prog_size;
+ prog->aux->extable = (void *)ctx->ro_insns + prog_size;

skip_init_ctx:
pass++;
@@ -154,23 +165,35 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)

bpf_jit_build_prologue(ctx);
if (build_body(ctx, extra_pass, NULL)) {
- bpf_jit_binary_free(jit_data->header);
prog = orig_prog;
- goto out_offset;
+ goto out_free_hdr;
}
bpf_jit_build_epilogue(ctx);

if (bpf_jit_enable > 1)
bpf_jit_dump(prog->len, prog_size, pass, ctx->insns);

- prog->bpf_func = (void *)ctx->insns;
+ prog->bpf_func = (void *)ctx->ro_insns;
prog->jited = 1;
prog->jited_len = prog_size;

- bpf_flush_icache(jit_data->header, ctx->insns + ctx->ninsns);
-
if (!prog->is_func || extra_pass) {
- bpf_jit_binary_lock_ro(jit_data->header);
+ if (WARN_ON(bpf_jit_binary_pack_finalize(prog,
+ jit_data->ro_header,
+ jit_data->header))) {
+ /* ro_header has been freed */
+ jit_data->ro_header = NULL;
+ prog = orig_prog;
+ goto out_offset;
+ }
+ /*
+ * The instructions have now been copied to the ROX region from
+ * where they will execute.
+ * Write any modified data cache blocks out to memory and
+ * invalidate the corresponding blocks in the instruction cache.
+ */
+ bpf_flush_icache(jit_data->ro_header,
+ ctx->ro_insns + ctx->ninsns);
for (i = 0; i < prog->len; i++)
ctx->offset[i] = ninsns_rvoff(ctx->offset[i]);
bpf_prog_fill_jited_linfo(prog, ctx->offset);
@@ -185,6 +208,15 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
bpf_jit_prog_release_other(prog, prog == orig_prog ?
tmp : orig_prog);
return prog;
+
+out_free_hdr:
+ if (jit_data->header) {
+ bpf_arch_text_copy(&jit_data->ro_header->size,
+ &jit_data->header->size,
+ sizeof(jit_data->header->size));
+ bpf_jit_binary_pack_free(jit_data->ro_header, jit_data->header);
+ }
+ goto out_offset;
}

u64 bpf_jit_alloc_exec_limit(void)
@@ -204,3 +236,56 @@ void bpf_jit_free_exec(void *addr)
{
return vfree(addr);
}
+
+void *bpf_arch_text_copy(void *dst, void *src, size_t len)
+{
+ int ret;
+
+ mutex_lock(&text_mutex);
+ ret = patch_text_nosync(dst, src, len);
+ mutex_unlock(&text_mutex);
+
+ if (ret)
+ return ERR_PTR(-EINVAL);
+
+ return dst;
+}
+
+int bpf_arch_text_invalidate(void *dst, size_t len)
+{
+ __le32 *ptr;
+ int ret = 0;
+ u32 inval = 0;
+
+ for (ptr = dst; ret == 0 && len >= sizeof(u32); len -= sizeof(u32)) {
+ mutex_lock(&text_mutex);
+ ret = patch_text_nosync(ptr++, &inval, sizeof(u32));
+ mutex_unlock(&text_mutex);
+ }
+
+ return ret;
+}
+
+void bpf_jit_free(struct bpf_prog *prog)
+{
+ if (prog->jited) {
+ struct rv_jit_data *jit_data = prog->aux->jit_data;
+ struct bpf_binary_header *hdr;
+
+ /*
+ * If we fail the final pass of JIT (from jit_subprogs),
+ * the program may not be finalized yet. Call finalize here
+ * before freeing it.
+ */
+ if (jit_data) {
+ bpf_jit_binary_pack_finalize(prog, jit_data->ro_header,
+ jit_data->header);
+ kfree(jit_data);
+ }
+ hdr = bpf_jit_binary_pack_hdr(prog);
+ bpf_jit_binary_pack_free(hdr, NULL);
+ WARN_ON_ONCE(!bpf_prog_kallsyms_verify_off(prog));
+ }
+
+ bpf_prog_unlock_free(prog);
+}
--
2.40.1

2023-07-20 16:34:23

by Puranjay Mohan

[permalink] [raw]

Subject: [PATCH bpf-next 1/2] riscv: Extend patch_text_nosync() for multiple pages

The patch_insn_write() function currently doesn't work for multiple
pages of instructions, therefore patch_text_nosync() will fail with a
page fault if called with lengths spanning multiple pages.

This commit extends the patch_insn_write() function to support multiple
pages by copying at max 2 pages at a time in a loop. This implementation
is similar to text_poke_copy() function of x86.

Signed-off-by: Puranjay Mohan <[email protected]>
---
arch/riscv/kernel/patch.c | 29 ++++++++++++++++++++++++-----
1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/arch/riscv/kernel/patch.c b/arch/riscv/kernel/patch.c
index 575e71d6c8ae..b2dbfcfdef85 100644
--- a/arch/riscv/kernel/patch.c
+++ b/arch/riscv/kernel/patch.c
@@ -53,7 +53,7 @@ static void patch_unmap(int fixmap)
}
NOKPROBE_SYMBOL(patch_unmap);

-static int patch_insn_write(void *addr, const void *insn, size_t len)
+static int __patch_insn_write(void *addr, const void *insn, size_t len)
{
void *waddr = addr;
bool across_pages = (((uintptr_t) addr & ~PAGE_MASK) + len) > PAGE_SIZE;
@@ -74,7 +74,7 @@ static int patch_insn_write(void *addr, const void *insn, size_t len)
lockdep_assert_held(&text_mutex);

if (across_pages)
- patch_map(addr + len, FIX_TEXT_POKE1);
+ patch_map(addr + PAGE_SIZE, FIX_TEXT_POKE1);

waddr = patch_map(addr, FIX_TEXT_POKE0);

@@ -87,15 +87,34 @@ static int patch_insn_write(void *addr, const void *insn, size_t len)

return ret;
}
-NOKPROBE_SYMBOL(patch_insn_write);
+NOKPROBE_SYMBOL(__patch_insn_write);
#else
-static int patch_insn_write(void *addr, const void *insn, size_t len)
+static int __patch_insn_write(void *addr, const void *insn, size_t len)
{
return copy_to_kernel_nofault(addr, insn, len);
}
-NOKPROBE_SYMBOL(patch_insn_write);
+NOKPROBE_SYMBOL(__patch_insn_write);
#endif /* CONFIG_MMU */

+static int patch_insn_write(void *addr, const void *insn, size_t len)
+{
+ size_t patched = 0;
+ size_t size;
+ int ret = 0;
+
+ while (patched < len && !ret) {
+ size = min_t(size_t,
+ PAGE_SIZE * 2 - offset_in_page(addr + patched),
+ len - patched);
+ ret = __patch_insn_write(addr + patched, insn + patched, size);
+
+ patched += size;
+ }
+
+ return ret;
+}
+NOKPROBE_SYMBOL(patch_insn_write);
+
int patch_text_nosync(void *addr, const void *insns, size_t len)
{
u32 *tp = addr;
--
2.40.1

2023-07-22 09:36:19

by Björn Töpel

[permalink] [raw]

Subject: Re: [PATCH bpf-next 0/2] bpf, riscv: use BPF prog pack allocator in BPF JIT

2023-08-13 20:44:21

by Björn Töpel

[permalink] [raw]

Subject: Re: [PATCH bpf-next 1/2] riscv: Extend patch_text_nosync() for multiple pages

Puranjay Mohan <[email protected]> writes:

> The patch_insn_write() function currently doesn't work for multiple
> pages of instructions, therefore patch_text_nosync() will fail with a
> page fault if called with lengths spanning multiple pages.
>
> This commit extends the patch_insn_write() function to support multiple
> pages by copying at max 2 pages at a time in a loop. This implementation
> is similar to text_poke_copy() function of x86.
>
> Signed-off-by: Puranjay Mohan <[email protected]>

Reviewed-by: Björn Töpel <[email protected]>

2023-08-13 20:45:34

by Björn Töpel

[permalink] [raw]

Subject: Re: [PATCH bpf-next 0/2] bpf, riscv: use BPF prog pack allocator in BPF JIT

2023-08-14 07:12:29

by Björn Töpel

[permalink] [raw]

Subject: Re: [PATCH bpf-next 0/2] bpf, riscv: use BPF prog pack allocator in BPF JIT

2023-08-14 09:30:00

by Björn Töpel

[permalink] [raw]

Subject: Re: [PATCH bpf-next 0/2] bpf, riscv: use BPF prog pack allocator in BPF JIT

Puranjay Mohan <[email protected]> writes:

> BPF programs currently consume a page each on RISCV. For systems with many BPF
> programs, this adds significant pressure to instruction TLB. High iTLB pressure
> usually causes slow down for the whole system.
>
> Song Liu introduced the BPF prog pack allocator[1] to mitigate the above issue.
> It packs multiple BPF programs into a single huge page. It is currently only
> enabled for the x86_64 BPF JIT.
>
> I enabled this allocator on the ARM64 BPF JIT[2]. It is being reviewed now.
>
> This patch series enables the BPF prog pack allocator for the RISCV BPF JIT.
> This series needs a patch[3] from the ARM64 series to work.
>
> ======================================================
> Performance Analysis of prog pack allocator on RISCV64
> ======================================================
>
> Test setup:
> ===========
>
> Host machine: Debian GNU/Linux 11 (bullseye)
> Qemu Version: QEMU emulator version 8.0.3 (Debian 1:8.0.3+dfsg-1)
> u-boot-qemu Version: 2023.07+dfsg-1
> opensbi Version: 1.3-1
>
> To test the performance of the BPF prog pack allocator on RV, a stresser
> tool[4] linked below was built. This tool loads 8 BPF programs on the system and
> triggers 5 of them in an infinite loop by doing system calls.
>
> The runner script starts 20 instances of the above which loads 8*20=160 BPF
> programs on the system, 5*20=100 of which are being constantly triggered.
> The script is passed a command which would be run in the above environment.
>
> The script was run with following perf command:
> ./run.sh "perf stat -a \
> -e iTLB-load-misses \
> -e dTLB-load-misses \
> -e dTLB-store-misses \
> -e instructions \
> --timeout 60000"
>
> The output of the above command is discussed below before and after enabling the
> BPF prog pack allocator.
>
> The tests were run on qemu-system-riscv64 with 8 cpus, 16G memory. The rootfs
> was created using Bjorn's riscv-cross-builder[5] docker container linked below.
>
> Results
> =======
>
> Before enabling prog pack allocator:
> ------------------------------------
>
> Performance counter stats for 'system wide':
>
> 4939048 iTLB-load-misses
> 5468689 dTLB-load-misses
> 465234 dTLB-store-misses
> 1441082097998 instructions
>
> 60.045791200 seconds time elapsed
>
> After enabling prog pack allocator:
> -----------------------------------
>
> Performance counter stats for 'system wide':
>
> 3430035 iTLB-load-misses
> 5008745 dTLB-load-misses
> 409944 dTLB-store-misses
> 1441535637988 instructions
>
> 60.046296600 seconds time elapsed
>
> Improvements in metrics
> =======================
>
> It was expected that the iTLB-load-misses would decrease as now a single huge
> page is used to keep all the BPF programs compared to a single page for each
> program earlier.
>
> --------------------------------------------
> The improvement in iTLB-load-misses: -30.5 %
> --------------------------------------------
>
> I repeated this expriment more than 100 times in different setups and the
> improvement was always greater than 30%.
>
> This patch series is boot tested on the Starfive VisionFive 2 board[6].
> The performance analysis was not done on the board because it doesn't
> expose iTLB-load-misses, etc. The stresser program was run on the board to test
> the loading and unloading of BPF programs
>
> [1] https://lore.kernel.org/bpf/[email protected]/
> [2] https://lore.kernel.org/all/[email protected]/
> [3] https://lore.kernel.org/all/[email protected]/
> [4] https://github.com/puranjaymohan/BPF-Allocator-Bench
> [5] https://github.com/bjoto/riscv-cross-builder
> [6] https://www.starfivetech.com/en/site/boards
>
> Puranjay Mohan (2):
> riscv: Extend patch_text_nosync() for multiple pages
> bpf, riscv: use prog pack allocator in the BPF JIT

I get a hang for "test_tag", but it's not directly related to your
series, but rather "remote fence.i".

| rcu: INFO: rcu_sched detected stalls on CPUs/tasks:
| rcu: 0-....: (1400 ticks this GP) idle=d5e4/1/0x4000000000000000 softirq=5542/5542 fqs=1862
| rcu: (detected by 1, t=5252 jiffies, g=10253, q=195 ncpus=4)
| Task dump for CPU 0:
| task:kworker/0:5 state:R running task stack:0 pid:319 ppid:2 flags:0x00000008
| Workqueue: events bpf_prog_free_deferred
| Call Trace:
| [<ffffffff80cbc444>] __schedule+0x2d0/0x940
| watchdog: BUG: soft lockup - CPU#0 stuck for 21s! [kworker/0:5:319]
| Modules linked in: nls_iso8859_1 drm fuse i2c_core drm_panel_orientation_quirks backlight dm_mod configfs ip_tables x_tables
| CPU: 0 PID: 319 Comm: kworker/0:5 Not tainted 6.5.0-rc5 #1
| Hardware name: riscv-virtio,qemu (DT)
| Workqueue: events bpf_prog_free_deferred
| epc : __sbi_rfence_v02_call.isra.0+0x74/0x11a
| ra : __sbi_rfence_v02+0xda/0x1a4
| epc : ffffffff8000ab4c ra : ffffffff8000accc sp : ff20000001c9bbd0
| gp : ffffffff82078c48 tp : ff600000888e6a40 t0 : ff20000001c9bd44
| t1 : 0000000000000000 t2 : 0000000000000040 s0 : ff20000001c9bbf0
| s1 : 0000000000000010 a0 : 0000000000000000 a1 : 0000000000000000
| a2 : 0000000000000000 a3 : 0000000000000000 a4 : 0000000000000000
| a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000052464e43
| s2 : 000000000000ffff s3 : 00000000ffffffff s4 : ffffffff81667528
| s5 : 0000000000000000 s6 : 0000000000000000 s7 : 0000000000000000
| s8 : 0000000000000001 s9 : 0000000000000003 s10: 0000000000000040
| s11: ffffffff8207d240 t3 : 000000000000000f t4 : 000000000000002a
| t5 : ff600000872df140 t6 : ffffffff81e26828
| status: 0000000200000120 badaddr: 0000000000000000 cause: 8000000000000005
| [<ffffffff8000ab4c>] __sbi_rfence_v02_call.isra.0+0x74/0x11a
| [<ffffffff8000accc>] __sbi_rfence_v02+0xda/0x1a4
| [<ffffffff8000a886>] sbi_remote_fence_i+0x1e/0x26
| [<ffffffff8000cee2>] flush_icache_all+0x1a/0x48
| [<ffffffff80007736>] patch_text_nosync+0x6c/0x8c
| [<ffffffff8000f0f8>] bpf_arch_text_invalidate+0x62/0xac
| [<ffffffff8016c538>] bpf_prog_pack_free+0x9c/0x1b2
| [<ffffffff8016c84a>] bpf_jit_binary_pack_free+0x20/0x4a
| [<ffffffff8000f198>] bpf_jit_free+0x56/0x9e
| [<ffffffff8016b43a>] bpf_prog_free_deferred+0x15a/0x182
| [<ffffffff800576c4>] process_one_work+0x1b6/0x3d6
| [<ffffffff80057d52>] worker_thread+0x84/0x378
| [<ffffffff8005fc2c>] kthread+0xe8/0x108
| [<ffffffff80003ffa>] ret_from_fork+0xe/0x20

I'm digging into that now, and I would appreciate if you could run the
test_tag on VF2 or similar (I'm missing that HW).

It seems like we're hitting a bug with this series, so let's try to
figure out where the problems is, prior merging it.

Björn

2023-08-14 10:47:29

by Puranjay Mohan

[permalink] [raw]

Subject: Re: [PATCH bpf-next 0/2] bpf, riscv: use BPF prog pack allocator in BPF JIT

Hi Björn,

On Mon, Aug 14, 2023 at 11:12 AM Björn Töpel <[email protected]> wrote:
>
> Puranjay Mohan <[email protected]> writes:
>
> > BPF programs currently consume a page each on RISCV. For systems with many BPF
> > programs, this adds significant pressure to instruction TLB. High iTLB pressure
> > usually causes slow down for the whole system.
> >
> > Song Liu introduced the BPF prog pack allocator[1] to mitigate the above issue.
> > It packs multiple BPF programs into a single huge page. It is currently only
> > enabled for the x86_64 BPF JIT.
> >
> > I enabled this allocator on the ARM64 BPF JIT[2]. It is being reviewed now.
> >
> > This patch series enables the BPF prog pack allocator for the RISCV BPF JIT.
> > This series needs a patch[3] from the ARM64 series to work.
> >
> > ======================================================
> > Performance Analysis of prog pack allocator on RISCV64
> > ======================================================
> >
> > Test setup:
> > ===========
> >
> > Host machine: Debian GNU/Linux 11 (bullseye)
> > Qemu Version: QEMU emulator version 8.0.3 (Debian 1:8.0.3+dfsg-1)
> > u-boot-qemu Version: 2023.07+dfsg-1
> > opensbi Version: 1.3-1
> >
> > To test the performance of the BPF prog pack allocator on RV, a stresser
> > tool[4] linked below was built. This tool loads 8 BPF programs on the system and
> > triggers 5 of them in an infinite loop by doing system calls.
> >
> > The runner script starts 20 instances of the above which loads 8*20=160 BPF
> > programs on the system, 5*20=100 of which are being constantly triggered.
> > The script is passed a command which would be run in the above environment.
> >
> > The script was run with following perf command:
> > ./run.sh "perf stat -a \
> > -e iTLB-load-misses \
> > -e dTLB-load-misses \
> > -e dTLB-store-misses \
> > -e instructions \
> > --timeout 60000"
> >
> > The output of the above command is discussed below before and after enabling the
> > BPF prog pack allocator.
> >
> > The tests were run on qemu-system-riscv64 with 8 cpus, 16G memory. The rootfs
> > was created using Bjorn's riscv-cross-builder[5] docker container linked below.
> >
> > Results
> > =======
> >
> > Before enabling prog pack allocator:
> > ------------------------------------
> >
> > Performance counter stats for 'system wide':
> >
> > 4939048 iTLB-load-misses
> > 5468689 dTLB-load-misses
> > 465234 dTLB-store-misses
> > 1441082097998 instructions
> >
> > 60.045791200 seconds time elapsed
> >
> > After enabling prog pack allocator:
> > -----------------------------------
> >
> > Performance counter stats for 'system wide':
> >
> > 3430035 iTLB-load-misses
> > 5008745 dTLB-load-misses
> > 409944 dTLB-store-misses
> > 1441535637988 instructions
> >
> > 60.046296600 seconds time elapsed
> >
> > Improvements in metrics
> > =======================
> >
> > It was expected that the iTLB-load-misses would decrease as now a single huge
> > page is used to keep all the BPF programs compared to a single page for each
> > program earlier.
> >
> > --------------------------------------------
> > The improvement in iTLB-load-misses: -30.5 %
> > --------------------------------------------
> >
> > I repeated this expriment more than 100 times in different setups and the
> > improvement was always greater than 30%.
> >
> > This patch series is boot tested on the Starfive VisionFive 2 board[6].
> > The performance analysis was not done on the board because it doesn't
> > expose iTLB-load-misses, etc. The stresser program was run on the board to test
> > the loading and unloading of BPF programs
> >
> > [1] https://lore.kernel.org/bpf/[email protected]/
> > [2] https://lore.kernel.org/all/[email protected]/
> > [3] https://lore.kernel.org/all/[email protected]/
> > [4] https://github.com/puranjaymohan/BPF-Allocator-Bench
> > [5] https://github.com/bjoto/riscv-cross-builder
> > [6] https://www.starfivetech.com/en/site/boards
> >
> > Puranjay Mohan (2):
> > riscv: Extend patch_text_nosync() for multiple pages
> > bpf, riscv: use prog pack allocator in the BPF JIT
>
> I get a hang for "test_tag", but it's not directly related to your
> series, but rather "remote fence.i".

I was seeing some stalls like this even without my series but couldn't
debug them at that time.

>
> | rcu: INFO: rcu_sched detected stalls on CPUs/tasks:
> | rcu: 0-....: (1400 ticks this GP) idle=d5e4/1/0x4000000000000000 softirq=5542/5542 fqs=1862
> | rcu: (detected by 1, t=5252 jiffies, g=10253, q=195 ncpus=4)
> | Task dump for CPU 0:
> | task:kworker/0:5 state:R running task stack:0 pid:319 ppid:2 flags:0x00000008
> | Workqueue: events bpf_prog_free_deferred
> | Call Trace:
> | [<ffffffff80cbc444>] __schedule+0x2d0/0x940
> | watchdog: BUG: soft lockup - CPU#0 stuck for 21s! [kworker/0:5:319]
> | Modules linked in: nls_iso8859_1 drm fuse i2c_core drm_panel_orientation_quirks backlight dm_mod configfs ip_tables x_tables
> | CPU: 0 PID: 319 Comm: kworker/0:5 Not tainted 6.5.0-rc5 #1
> | Hardware name: riscv-virtio,qemu (DT)
> | Workqueue: events bpf_prog_free_deferred
> | epc : __sbi_rfence_v02_call.isra.0+0x74/0x11a
> | ra : __sbi_rfence_v02+0xda/0x1a4
> | epc : ffffffff8000ab4c ra : ffffffff8000accc sp : ff20000001c9bbd0
> | gp : ffffffff82078c48 tp : ff600000888e6a40 t0 : ff20000001c9bd44
> | t1 : 0000000000000000 t2 : 0000000000000040 s0 : ff20000001c9bbf0
> | s1 : 0000000000000010 a0 : 0000000000000000 a1 : 0000000000000000
> | a2 : 0000000000000000 a3 : 0000000000000000 a4 : 0000000000000000
> | a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000052464e43
> | s2 : 000000000000ffff s3 : 00000000ffffffff s4 : ffffffff81667528
> | s5 : 0000000000000000 s6 : 0000000000000000 s7 : 0000000000000000
> | s8 : 0000000000000001 s9 : 0000000000000003 s10: 0000000000000040
> | s11: ffffffff8207d240 t3 : 000000000000000f t4 : 000000000000002a
> | t5 : ff600000872df140 t6 : ffffffff81e26828
> | status: 0000000200000120 badaddr: 0000000000000000 cause: 8000000000000005
> | [<ffffffff8000ab4c>] __sbi_rfence_v02_call.isra.0+0x74/0x11a
> | [<ffffffff8000accc>] __sbi_rfence_v02+0xda/0x1a4
> | [<ffffffff8000a886>] sbi_remote_fence_i+0x1e/0x26
> | [<ffffffff8000cee2>] flush_icache_all+0x1a/0x48
> | [<ffffffff80007736>] patch_text_nosync+0x6c/0x8c
> | [<ffffffff8000f0f8>] bpf_arch_text_invalidate+0x62/0xac
> | [<ffffffff8016c538>] bpf_prog_pack_free+0x9c/0x1b2
> | [<ffffffff8016c84a>] bpf_jit_binary_pack_free+0x20/0x4a
> | [<ffffffff8000f198>] bpf_jit_free+0x56/0x9e
> | [<ffffffff8016b43a>] bpf_prog_free_deferred+0x15a/0x182
> | [<ffffffff800576c4>] process_one_work+0x1b6/0x3d6
> | [<ffffffff80057d52>] worker_thread+0x84/0x378
> | [<ffffffff8005fc2c>] kthread+0xe8/0x108
> | [<ffffffff80003ffa>] ret_from_fork+0xe/0x20
>
> I'm digging into that now, and I would appreciate if you could run the
> test_tag on VF2 or similar (I'm missing that HW).

Sure, I will try to run this on the board.
I will rebase my series(+ the patch from arm64 series) on the latest
bpf-next tree and try to run it.
Let me know if I need to add:
+ select HAVE_EFFICIENT_UNALIGNED_ACCESS if MMU && 64BIT

>
> It seems like we're hitting a bug with this series, so let's try to
> figure out where the problems is, prior merging it.
>
>
> Björn

Thanks,
Puranjay

2023-08-14 10:47:48

by Björn Töpel

[permalink] [raw]

Subject: Re: [PATCH bpf-next 0/2] bpf, riscv: use BPF prog pack allocator in BPF JIT

Puranjay Mohan <[email protected]> writes:

>> I get a hang for "test_tag", but it's not directly related to your
>> series, but rather "remote fence.i".
>
> I was seeing some stalls like this even without my series but couldn't
> debug them at that time.

Yeah, I think it's not related to your series -- it's just a good
reproducer. ;-)

>>
>> | rcu: INFO: rcu_sched detected stalls on CPUs/tasks:
>> | rcu: 0-....: (1400 ticks this GP) idle=d5e4/1/0x4000000000000000 softirq=5542/5542 fqs=1862
>> | rcu: (detected by 1, t=5252 jiffies, g=10253, q=195 ncpus=4)
>> | Task dump for CPU 0:
>> | task:kworker/0:5 state:R running task stack:0 pid:319 ppid:2 flags:0x00000008
>> | Workqueue: events bpf_prog_free_deferred
>> | Call Trace:
>> | [<ffffffff80cbc444>] __schedule+0x2d0/0x940
>> | watchdog: BUG: soft lockup - CPU#0 stuck for 21s! [kworker/0:5:319]
>> | Modules linked in: nls_iso8859_1 drm fuse i2c_core drm_panel_orientation_quirks backlight dm_mod configfs ip_tables x_tables
>> | CPU: 0 PID: 319 Comm: kworker/0:5 Not tainted 6.5.0-rc5 #1
>> | Hardware name: riscv-virtio,qemu (DT)
>> | Workqueue: events bpf_prog_free_deferred
>> | epc : __sbi_rfence_v02_call.isra.0+0x74/0x11a
>> | ra : __sbi_rfence_v02+0xda/0x1a4
>> | epc : ffffffff8000ab4c ra : ffffffff8000accc sp : ff20000001c9bbd0
>> | gp : ffffffff82078c48 tp : ff600000888e6a40 t0 : ff20000001c9bd44
>> | t1 : 0000000000000000 t2 : 0000000000000040 s0 : ff20000001c9bbf0
>> | s1 : 0000000000000010 a0 : 0000000000000000 a1 : 0000000000000000
>> | a2 : 0000000000000000 a3 : 0000000000000000 a4 : 0000000000000000
>> | a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000052464e43
>> | s2 : 000000000000ffff s3 : 00000000ffffffff s4 : ffffffff81667528
>> | s5 : 0000000000000000 s6 : 0000000000000000 s7 : 0000000000000000
>> | s8 : 0000000000000001 s9 : 0000000000000003 s10: 0000000000000040
>> | s11: ffffffff8207d240 t3 : 000000000000000f t4 : 000000000000002a
>> | t5 : ff600000872df140 t6 : ffffffff81e26828
>> | status: 0000000200000120 badaddr: 0000000000000000 cause: 8000000000000005
>> | [<ffffffff8000ab4c>] __sbi_rfence_v02_call.isra.0+0x74/0x11a
>> | [<ffffffff8000accc>] __sbi_rfence_v02+0xda/0x1a4
>> | [<ffffffff8000a886>] sbi_remote_fence_i+0x1e/0x26
>> | [<ffffffff8000cee2>] flush_icache_all+0x1a/0x48
>> | [<ffffffff80007736>] patch_text_nosync+0x6c/0x8c
>> | [<ffffffff8000f0f8>] bpf_arch_text_invalidate+0x62/0xac
>> | [<ffffffff8016c538>] bpf_prog_pack_free+0x9c/0x1b2
>> | [<ffffffff8016c84a>] bpf_jit_binary_pack_free+0x20/0x4a
>> | [<ffffffff8000f198>] bpf_jit_free+0x56/0x9e
>> | [<ffffffff8016b43a>] bpf_prog_free_deferred+0x15a/0x182
>> | [<ffffffff800576c4>] process_one_work+0x1b6/0x3d6
>> | [<ffffffff80057d52>] worker_thread+0x84/0x378
>> | [<ffffffff8005fc2c>] kthread+0xe8/0x108
>> | [<ffffffff80003ffa>] ret_from_fork+0xe/0x20
>>
>> I'm digging into that now, and I would appreciate if you could run the
>> test_tag on VF2 or similar (I'm missing that HW).
>
> Sure, I will try to run this on the board.
> I will rebase my series(+ the patch from arm64 series) on the latest
> bpf-next tree and try to run it.

Thank you!

> Let me know if I need to add:
> + select HAVE_EFFICIENT_UNALIGNED_ACCESS if MMU && 64BIT

I usually run with that *on*, for better coverage.

Björn

2023-08-14 12:26:30

by Puranjay Mohan

[permalink] [raw]

Subject: Re: [PATCH bpf-next 0/2] bpf, riscv: use BPF prog pack allocator in BPF JIT

On Mon, Aug 14, 2023 at 12:40 PM Björn Töpel <[email protected]> wrote:
>
> Björn Töpel <[email protected]> writes:
>
> > Puranjay Mohan <[email protected]> writes:
> >
> >> BPF programs currently consume a page each on RISCV. For systems with many BPF
> >> programs, this adds significant pressure to instruction TLB. High iTLB pressure
> >> usually causes slow down for the whole system.
> >>
> >> Song Liu introduced the BPF prog pack allocator[1] to mitigate the above issue.
> >> It packs multiple BPF programs into a single huge page. It is currently only
> >> enabled for the x86_64 BPF JIT.
> >>
> >> I enabled this allocator on the ARM64 BPF JIT[2]. It is being reviewed now.
> >>
> >> This patch series enables the BPF prog pack allocator for the RISCV BPF JIT.
> >> This series needs a patch[3] from the ARM64 series to work.
> >>
> >> ======================================================
> >> Performance Analysis of prog pack allocator on RISCV64
> >> ======================================================
> >>
> >> Test setup:
> >> ===========
> >>
> >> Host machine: Debian GNU/Linux 11 (bullseye)
> >> Qemu Version: QEMU emulator version 8.0.3 (Debian 1:8.0.3+dfsg-1)
> >> u-boot-qemu Version: 2023.07+dfsg-1
> >> opensbi Version: 1.3-1
> >>
> >> To test the performance of the BPF prog pack allocator on RV, a stresser
> >> tool[4] linked below was built. This tool loads 8 BPF programs on the system and
> >> triggers 5 of them in an infinite loop by doing system calls.
> >>
> >> The runner script starts 20 instances of the above which loads 8*20=160 BPF
> >> programs on the system, 5*20=100 of which are being constantly triggered.
> >> The script is passed a command which would be run in the above environment.
> >>
> >> The script was run with following perf command:
> >> ./run.sh "perf stat -a \
> >> -e iTLB-load-misses \
> >> -e dTLB-load-misses \
> >> -e dTLB-store-misses \
> >> -e instructions \
> >> --timeout 60000"
> >>
> >> The output of the above command is discussed below before and after enabling the
> >> BPF prog pack allocator.
> >>
> >> The tests were run on qemu-system-riscv64 with 8 cpus, 16G memory. The rootfs
> >> was created using Bjorn's riscv-cross-builder[5] docker container linked below.
> >>
> >> Results
> >> =======
> >>
> >> Before enabling prog pack allocator:
> >> ------------------------------------
> >>
> >> Performance counter stats for 'system wide':
> >>
> >> 4939048 iTLB-load-misses
> >> 5468689 dTLB-load-misses
> >> 465234 dTLB-store-misses
> >> 1441082097998 instructions
> >>
> >> 60.045791200 seconds time elapsed
> >>
> >> After enabling prog pack allocator:
> >> -----------------------------------
> >>
> >> Performance counter stats for 'system wide':
> >>
> >> 3430035 iTLB-load-misses
> >> 5008745 dTLB-load-misses
> >> 409944 dTLB-store-misses
> >> 1441535637988 instructions
> >>
> >> 60.046296600 seconds time elapsed
> >>
> >> Improvements in metrics
> >> =======================
> >>
> >> It was expected that the iTLB-load-misses would decrease as now a single huge
> >> page is used to keep all the BPF programs compared to a single page for each
> >> program earlier.
> >>
> >> --------------------------------------------
> >> The improvement in iTLB-load-misses: -30.5 %
> >> --------------------------------------------
> >>
> >> I repeated this expriment more than 100 times in different setups and the
> >> improvement was always greater than 30%.
> >>
> >> This patch series is boot tested on the Starfive VisionFive 2 board[6].
> >> The performance analysis was not done on the board because it doesn't
> >> expose iTLB-load-misses, etc. The stresser program was run on the board to test
> >> the loading and unloading of BPF programs
> >>
> >> [1] https://lore.kernel.org/bpf/[email protected]/
> >> [2] https://lore.kernel.org/all/[email protected]/
> >> [3] https://lore.kernel.org/all/[email protected]/
> >> [4] https://github.com/puranjaymohan/BPF-Allocator-Bench
> >> [5] https://github.com/bjoto/riscv-cross-builder
> >> [6] https://www.starfivetech.com/en/site/boards
> >>
> >> Puranjay Mohan (2):
> >> riscv: Extend patch_text_nosync() for multiple pages
> >> bpf, riscv: use prog pack allocator in the BPF JIT
> >
> > I get a hang for "test_tag", but it's not directly related to your
> > series, but rather "remote fence.i".
> >
> > | rcu: INFO: rcu_sched detected stalls on CPUs/tasks:
> > | rcu: 0-....: (1400 ticks this GP) idle=d5e4/1/0x4000000000000000 softirq=5542/5542 fqs=1862
> > | rcu: (detected by 1, t=5252 jiffies, g=10253, q=195 ncpus=4)
> > | Task dump for CPU 0:
> > | task:kworker/0:5 state:R running task stack:0 pid:319 ppid:2 flags:0x00000008
> > | Workqueue: events bpf_prog_free_deferred
> > | Call Trace:
> > | [<ffffffff80cbc444>] __schedule+0x2d0/0x940
> > | watchdog: BUG: soft lockup - CPU#0 stuck for 21s! [kworker/0:5:319]
> > | Modules linked in: nls_iso8859_1 drm fuse i2c_core drm_panel_orientation_quirks backlight dm_mod configfs ip_tables x_tables
> > | CPU: 0 PID: 319 Comm: kworker/0:5 Not tainted 6.5.0-rc5 #1
> > | Hardware name: riscv-virtio,qemu (DT)
> > | Workqueue: events bpf_prog_free_deferred
> > | epc : __sbi_rfence_v02_call.isra.0+0x74/0x11a
> > | ra : __sbi_rfence_v02+0xda/0x1a4
> > | epc : ffffffff8000ab4c ra : ffffffff8000accc sp : ff20000001c9bbd0
> > | gp : ffffffff82078c48 tp : ff600000888e6a40 t0 : ff20000001c9bd44
> > | t1 : 0000000000000000 t2 : 0000000000000040 s0 : ff20000001c9bbf0
> > | s1 : 0000000000000010 a0 : 0000000000000000 a1 : 0000000000000000
> > | a2 : 0000000000000000 a3 : 0000000000000000 a4 : 0000000000000000
> > | a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000052464e43
> > | s2 : 000000000000ffff s3 : 00000000ffffffff s4 : ffffffff81667528
> > | s5 : 0000000000000000 s6 : 0000000000000000 s7 : 0000000000000000
> > | s8 : 0000000000000001 s9 : 0000000000000003 s10: 0000000000000040
> > | s11: ffffffff8207d240 t3 : 000000000000000f t4 : 000000000000002a
> > | t5 : ff600000872df140 t6 : ffffffff81e26828
> > | status: 0000000200000120 badaddr: 0000000000000000 cause: 8000000000000005
> > | [<ffffffff8000ab4c>] __sbi_rfence_v02_call.isra.0+0x74/0x11a
> > | [<ffffffff8000accc>] __sbi_rfence_v02+0xda/0x1a4
> > | [<ffffffff8000a886>] sbi_remote_fence_i+0x1e/0x26
> > | [<ffffffff8000cee2>] flush_icache_all+0x1a/0x48
> > | [<ffffffff80007736>] patch_text_nosync+0x6c/0x8c
> > | [<ffffffff8000f0f8>] bpf_arch_text_invalidate+0x62/0xac
> > | [<ffffffff8016c538>] bpf_prog_pack_free+0x9c/0x1b2
> > | [<ffffffff8016c84a>] bpf_jit_binary_pack_free+0x20/0x4a
> > | [<ffffffff8000f198>] bpf_jit_free+0x56/0x9e
> > | [<ffffffff8016b43a>] bpf_prog_free_deferred+0x15a/0x182
> > | [<ffffffff800576c4>] process_one_work+0x1b6/0x3d6
> > | [<ffffffff80057d52>] worker_thread+0x84/0x378
> > | [<ffffffff8005fc2c>] kthread+0xe8/0x108
> > | [<ffffffff80003ffa>] ret_from_fork+0xe/0x20
> >
> > I'm digging into that now, and I would appreciate if you could run the
> > test_tag on VF2 or similar (I'm missing that HW).
> >
> > It seems like we're hitting a bug with this series, so let's try to
> > figure out where the problems is, prior merging it.
>
> Hmm, it looks like the bpf_arch_text_invalidate() implementation is a
> bit problematic:
>
> +int bpf_arch_text_invalidate(void *dst, size_t len)
> +{
> + __le32 *ptr;
> + int ret = 0;
> + u32 inval = 0;
> +
> + for (ptr = dst; ret == 0 && len >= sizeof(u32); len -= sizeof(u32)) {
> + mutex_lock(&text_mutex);
> + ret = patch_text_nosync(ptr++, &inval, sizeof(u32));
> + mutex_unlock(&text_mutex);
> + }
> +
> + return ret;
> +}
>
> Each patch_text_nosync() is a remote fence.i, and for a big "len", we'll
> be flooded with remote fences.

I understand this now, thanks for debugging this.

We are calling patch_text_nosync() for each word (u32) which calls
flush_icache_range() and therefore "fence.i" is inserted after every word.

I still don't fully understand how it causes this bug because I lack
the prerequisite
knowledge about test_tag and what the failing test is doing.

But to solve this issue we would need a function like the x86
text_poke_set() that will only
insert a single "fence.i" after setting the whole memory area. This
can be done by
implementing a wrapper around patch_insn_write() which would set the memory area
and at the end call flush_icache_range().

Something like:

void *text_set_nosync(void *dst, int c, size_t len)
{
__le32 *ptr;
int ret = 0;

for (ptr = dst; ret == 0 && len >= sizeof(u32); len -= sizeof(u32)) {
ret = patch_insn_write(ptr++, &c, sizeof(u32));
}
if(!ret)
flush_icache_range((uintptr_t) dst, (uintptr_t) dst + len);

return ret;
}

Let me know if this looks correct or we need more details here.
I will then send v2 with this implemented as a separate patch.

>
> I think that's exactly what we hit with "test_tag".
>
>
> Björn

Thanks,
Puranjay

2023-08-14 12:38:18

by Björn Töpel

[permalink] [raw]

Subject: Re: [PATCH bpf-next 0/2] bpf, riscv: use BPF prog pack allocator in BPF JIT

Björn Töpel <[email protected]> writes:

> Puranjay Mohan <[email protected]> writes:
>
>> BPF programs currently consume a page each on RISCV. For systems with many BPF
>> programs, this adds significant pressure to instruction TLB. High iTLB pressure
>> usually causes slow down for the whole system.
>>
>> Song Liu introduced the BPF prog pack allocator[1] to mitigate the above issue.
>> It packs multiple BPF programs into a single huge page. It is currently only
>> enabled for the x86_64 BPF JIT.
>>
>> I enabled this allocator on the ARM64 BPF JIT[2]. It is being reviewed now.
>>
>> This patch series enables the BPF prog pack allocator for the RISCV BPF JIT.
>> This series needs a patch[3] from the ARM64 series to work.
>>
>> ======================================================
>> Performance Analysis of prog pack allocator on RISCV64
>> ======================================================
>>
>> Test setup:
>> ===========
>>
>> Host machine: Debian GNU/Linux 11 (bullseye)
>> Qemu Version: QEMU emulator version 8.0.3 (Debian 1:8.0.3+dfsg-1)
>> u-boot-qemu Version: 2023.07+dfsg-1
>> opensbi Version: 1.3-1
>>
>> To test the performance of the BPF prog pack allocator on RV, a stresser
>> tool[4] linked below was built. This tool loads 8 BPF programs on the system and
>> triggers 5 of them in an infinite loop by doing system calls.
>>
>> The runner script starts 20 instances of the above which loads 8*20=160 BPF
>> programs on the system, 5*20=100 of which are being constantly triggered.
>> The script is passed a command which would be run in the above environment.
>>
>> The script was run with following perf command:
>> ./run.sh "perf stat -a \
>> -e iTLB-load-misses \
>> -e dTLB-load-misses \
>> -e dTLB-store-misses \
>> -e instructions \
>> --timeout 60000"
>>
>> The output of the above command is discussed below before and after enabling the
>> BPF prog pack allocator.
>>
>> The tests were run on qemu-system-riscv64 with 8 cpus, 16G memory. The rootfs
>> was created using Bjorn's riscv-cross-builder[5] docker container linked below.
>>
>> Results
>> =======
>>
>> Before enabling prog pack allocator:
>> ------------------------------------
>>
>> Performance counter stats for 'system wide':
>>
>> 4939048 iTLB-load-misses
>> 5468689 dTLB-load-misses
>> 465234 dTLB-store-misses
>> 1441082097998 instructions
>>
>> 60.045791200 seconds time elapsed
>>
>> After enabling prog pack allocator:
>> -----------------------------------
>>
>> Performance counter stats for 'system wide':
>>
>> 3430035 iTLB-load-misses
>> 5008745 dTLB-load-misses
>> 409944 dTLB-store-misses
>> 1441535637988 instructions
>>
>> 60.046296600 seconds time elapsed
>>
>> Improvements in metrics
>> =======================
>>
>> It was expected that the iTLB-load-misses would decrease as now a single huge
>> page is used to keep all the BPF programs compared to a single page for each
>> program earlier.
>>
>> --------------------------------------------
>> The improvement in iTLB-load-misses: -30.5 %
>> --------------------------------------------
>>
>> I repeated this expriment more than 100 times in different setups and the
>> improvement was always greater than 30%.
>>
>> This patch series is boot tested on the Starfive VisionFive 2 board[6].
>> The performance analysis was not done on the board because it doesn't
>> expose iTLB-load-misses, etc. The stresser program was run on the board to test
>> the loading and unloading of BPF programs
>>
>> [1] https://lore.kernel.org/bpf/[email protected]/
>> [2] https://lore.kernel.org/all/[email protected]/
>> [3] https://lore.kernel.org/all/[email protected]/
>> [4] https://github.com/puranjaymohan/BPF-Allocator-Bench
>> [5] https://github.com/bjoto/riscv-cross-builder
>> [6] https://www.starfivetech.com/en/site/boards
>>
>> Puranjay Mohan (2):
>> riscv: Extend patch_text_nosync() for multiple pages
>> bpf, riscv: use prog pack allocator in the BPF JIT
>
> I get a hang for "test_tag", but it's not directly related to your
> series, but rather "remote fence.i".
>
> | rcu: INFO: rcu_sched detected stalls on CPUs/tasks:
> | rcu: 0-....: (1400 ticks this GP) idle=d5e4/1/0x4000000000000000 softirq=5542/5542 fqs=1862
> | rcu: (detected by 1, t=5252 jiffies, g=10253, q=195 ncpus=4)
> | Task dump for CPU 0:
> | task:kworker/0:5 state:R running task stack:0 pid:319 ppid:2 flags:0x00000008
> | Workqueue: events bpf_prog_free_deferred
> | Call Trace:
> | [<ffffffff80cbc444>] __schedule+0x2d0/0x940
> | watchdog: BUG: soft lockup - CPU#0 stuck for 21s! [kworker/0:5:319]
> | Modules linked in: nls_iso8859_1 drm fuse i2c_core drm_panel_orientation_quirks backlight dm_mod configfs ip_tables x_tables
> | CPU: 0 PID: 319 Comm: kworker/0:5 Not tainted 6.5.0-rc5 #1
> | Hardware name: riscv-virtio,qemu (DT)
> | Workqueue: events bpf_prog_free_deferred
> | epc : __sbi_rfence_v02_call.isra.0+0x74/0x11a
> | ra : __sbi_rfence_v02+0xda/0x1a4
> | epc : ffffffff8000ab4c ra : ffffffff8000accc sp : ff20000001c9bbd0
> | gp : ffffffff82078c48 tp : ff600000888e6a40 t0 : ff20000001c9bd44
> | t1 : 0000000000000000 t2 : 0000000000000040 s0 : ff20000001c9bbf0
> | s1 : 0000000000000010 a0 : 0000000000000000 a1 : 0000000000000000
> | a2 : 0000000000000000 a3 : 0000000000000000 a4 : 0000000000000000
> | a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000052464e43
> | s2 : 000000000000ffff s3 : 00000000ffffffff s4 : ffffffff81667528
> | s5 : 0000000000000000 s6 : 0000000000000000 s7 : 0000000000000000
> | s8 : 0000000000000001 s9 : 0000000000000003 s10: 0000000000000040
> | s11: ffffffff8207d240 t3 : 000000000000000f t4 : 000000000000002a
> | t5 : ff600000872df140 t6 : ffffffff81e26828
> | status: 0000000200000120 badaddr: 0000000000000000 cause: 8000000000000005
> | [<ffffffff8000ab4c>] __sbi_rfence_v02_call.isra.0+0x74/0x11a
> | [<ffffffff8000accc>] __sbi_rfence_v02+0xda/0x1a4
> | [<ffffffff8000a886>] sbi_remote_fence_i+0x1e/0x26
> | [<ffffffff8000cee2>] flush_icache_all+0x1a/0x48
> | [<ffffffff80007736>] patch_text_nosync+0x6c/0x8c
> | [<ffffffff8000f0f8>] bpf_arch_text_invalidate+0x62/0xac
> | [<ffffffff8016c538>] bpf_prog_pack_free+0x9c/0x1b2
> | [<ffffffff8016c84a>] bpf_jit_binary_pack_free+0x20/0x4a
> | [<ffffffff8000f198>] bpf_jit_free+0x56/0x9e
> | [<ffffffff8016b43a>] bpf_prog_free_deferred+0x15a/0x182
> | [<ffffffff800576c4>] process_one_work+0x1b6/0x3d6
> | [<ffffffff80057d52>] worker_thread+0x84/0x378
> | [<ffffffff8005fc2c>] kthread+0xe8/0x108
> | [<ffffffff80003ffa>] ret_from_fork+0xe/0x20
>
> I'm digging into that now, and I would appreciate if you could run the
> test_tag on VF2 or similar (I'm missing that HW).
>
> It seems like we're hitting a bug with this series, so let's try to
> figure out where the problems is, prior merging it.

Hmm, it looks like the bpf_arch_text_invalidate() implementation is a
bit problematic:

+int bpf_arch_text_invalidate(void *dst, size_t len)
+{
+ __le32 *ptr;
+ int ret = 0;
+ u32 inval = 0;
+
+ for (ptr = dst; ret == 0 && len >= sizeof(u32); len -= sizeof(u32)) {
+ mutex_lock(&text_mutex);
+ ret = patch_text_nosync(ptr++, &inval, sizeof(u32));
+ mutex_unlock(&text_mutex);
+ }
+
+ return ret;
+}

Each patch_text_nosync() is a remote fence.i, and for a big "len", we'll
be flooded with remote fences.

I think that's exactly what we hit with "test_tag".

Björn

2023-08-14 14:59:36

by Björn Töpel

[permalink] [raw]

Subject: Re: [PATCH bpf-next 0/2] bpf, riscv: use BPF prog pack allocator in BPF JIT

Puranjay Mohan <[email protected]> writes:

> On Mon, Aug 14, 2023 at 12:40 PM Björn Töpel <[email protected]> wrote:
>>
>> Björn Töpel <[email protected]> writes:
>>
>> > Puranjay Mohan <[email protected]> writes:
>> >
>> >> BPF programs currently consume a page each on RISCV. For systems with many BPF
>> >> programs, this adds significant pressure to instruction TLB. High iTLB pressure
>> >> usually causes slow down for the whole system.
>> >>
>> >> Song Liu introduced the BPF prog pack allocator[1] to mitigate the above issue.
>> >> It packs multiple BPF programs into a single huge page. It is currently only
>> >> enabled for the x86_64 BPF JIT.
>> >>
>> >> I enabled this allocator on the ARM64 BPF JIT[2]. It is being reviewed now.
>> >>
>> >> This patch series enables the BPF prog pack allocator for the RISCV BPF JIT.
>> >> This series needs a patch[3] from the ARM64 series to work.
>> >>
>> >> ======================================================
>> >> Performance Analysis of prog pack allocator on RISCV64
>> >> ======================================================
>> >>
>> >> Test setup:
>> >> ===========
>> >>
>> >> Host machine: Debian GNU/Linux 11 (bullseye)
>> >> Qemu Version: QEMU emulator version 8.0.3 (Debian 1:8.0.3+dfsg-1)
>> >> u-boot-qemu Version: 2023.07+dfsg-1
>> >> opensbi Version: 1.3-1
>> >>
>> >> To test the performance of the BPF prog pack allocator on RV, a stresser
>> >> tool[4] linked below was built. This tool loads 8 BPF programs on the system and
>> >> triggers 5 of them in an infinite loop by doing system calls.
>> >>
>> >> The runner script starts 20 instances of the above which loads 8*20=160 BPF
>> >> programs on the system, 5*20=100 of which are being constantly triggered.
>> >> The script is passed a command which would be run in the above environment.
>> >>
>> >> The script was run with following perf command:
>> >> ./run.sh "perf stat -a \
>> >> -e iTLB-load-misses \
>> >> -e dTLB-load-misses \
>> >> -e dTLB-store-misses \
>> >> -e instructions \
>> >> --timeout 60000"
>> >>
>> >> The output of the above command is discussed below before and after enabling the
>> >> BPF prog pack allocator.
>> >>
>> >> The tests were run on qemu-system-riscv64 with 8 cpus, 16G memory. The rootfs
>> >> was created using Bjorn's riscv-cross-builder[5] docker container linked below.
>> >>
>> >> Results
>> >> =======
>> >>
>> >> Before enabling prog pack allocator:
>> >> ------------------------------------
>> >>
>> >> Performance counter stats for 'system wide':
>> >>
>> >> 4939048 iTLB-load-misses
>> >> 5468689 dTLB-load-misses
>> >> 465234 dTLB-store-misses
>> >> 1441082097998 instructions
>> >>
>> >> 60.045791200 seconds time elapsed
>> >>
>> >> After enabling prog pack allocator:
>> >> -----------------------------------
>> >>
>> >> Performance counter stats for 'system wide':
>> >>
>> >> 3430035 iTLB-load-misses
>> >> 5008745 dTLB-load-misses
>> >> 409944 dTLB-store-misses
>> >> 1441535637988 instructions
>> >>
>> >> 60.046296600 seconds time elapsed
>> >>
>> >> Improvements in metrics
>> >> =======================
>> >>
>> >> It was expected that the iTLB-load-misses would decrease as now a single huge
>> >> page is used to keep all the BPF programs compared to a single page for each
>> >> program earlier.
>> >>
>> >> --------------------------------------------
>> >> The improvement in iTLB-load-misses: -30.5 %
>> >> --------------------------------------------
>> >>
>> >> I repeated this expriment more than 100 times in different setups and the
>> >> improvement was always greater than 30%.
>> >>
>> >> This patch series is boot tested on the Starfive VisionFive 2 board[6].
>> >> The performance analysis was not done on the board because it doesn't
>> >> expose iTLB-load-misses, etc. The stresser program was run on the board to test
>> >> the loading and unloading of BPF programs
>> >>
>> >> [1] https://lore.kernel.org/bpf/[email protected]/
>> >> [2] https://lore.kernel.org/all/[email protected]/
>> >> [3] https://lore.kernel.org/all/[email protected]/
>> >> [4] https://github.com/puranjaymohan/BPF-Allocator-Bench
>> >> [5] https://github.com/bjoto/riscv-cross-builder
>> >> [6] https://www.starfivetech.com/en/site/boards
>> >>
>> >> Puranjay Mohan (2):
>> >> riscv: Extend patch_text_nosync() for multiple pages
>> >> bpf, riscv: use prog pack allocator in the BPF JIT
>> >
>> > I get a hang for "test_tag", but it's not directly related to your
>> > series, but rather "remote fence.i".
>> >
>> > | rcu: INFO: rcu_sched detected stalls on CPUs/tasks:
>> > | rcu: 0-....: (1400 ticks this GP) idle=d5e4/1/0x4000000000000000 softirq=5542/5542 fqs=1862
>> > | rcu: (detected by 1, t=5252 jiffies, g=10253, q=195 ncpus=4)
>> > | Task dump for CPU 0:
>> > | task:kworker/0:5 state:R running task stack:0 pid:319 ppid:2 flags:0x00000008
>> > | Workqueue: events bpf_prog_free_deferred
>> > | Call Trace:
>> > | [<ffffffff80cbc444>] __schedule+0x2d0/0x940
>> > | watchdog: BUG: soft lockup - CPU#0 stuck for 21s! [kworker/0:5:319]
>> > | Modules linked in: nls_iso8859_1 drm fuse i2c_core drm_panel_orientation_quirks backlight dm_mod configfs ip_tables x_tables
>> > | CPU: 0 PID: 319 Comm: kworker/0:5 Not tainted 6.5.0-rc5 #1
>> > | Hardware name: riscv-virtio,qemu (DT)
>> > | Workqueue: events bpf_prog_free_deferred
>> > | epc : __sbi_rfence_v02_call.isra.0+0x74/0x11a
>> > | ra : __sbi_rfence_v02+0xda/0x1a4
>> > | epc : ffffffff8000ab4c ra : ffffffff8000accc sp : ff20000001c9bbd0
>> > | gp : ffffffff82078c48 tp : ff600000888e6a40 t0 : ff20000001c9bd44
>> > | t1 : 0000000000000000 t2 : 0000000000000040 s0 : ff20000001c9bbf0
>> > | s1 : 0000000000000010 a0 : 0000000000000000 a1 : 0000000000000000
>> > | a2 : 0000000000000000 a3 : 0000000000000000 a4 : 0000000000000000
>> > | a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000052464e43
>> > | s2 : 000000000000ffff s3 : 00000000ffffffff s4 : ffffffff81667528
>> > | s5 : 0000000000000000 s6 : 0000000000000000 s7 : 0000000000000000
>> > | s8 : 0000000000000001 s9 : 0000000000000003 s10: 0000000000000040
>> > | s11: ffffffff8207d240 t3 : 000000000000000f t4 : 000000000000002a
>> > | t5 : ff600000872df140 t6 : ffffffff81e26828
>> > | status: 0000000200000120 badaddr: 0000000000000000 cause: 8000000000000005
>> > | [<ffffffff8000ab4c>] __sbi_rfence_v02_call.isra.0+0x74/0x11a
>> > | [<ffffffff8000accc>] __sbi_rfence_v02+0xda/0x1a4
>> > | [<ffffffff8000a886>] sbi_remote_fence_i+0x1e/0x26
>> > | [<ffffffff8000cee2>] flush_icache_all+0x1a/0x48
>> > | [<ffffffff80007736>] patch_text_nosync+0x6c/0x8c
>> > | [<ffffffff8000f0f8>] bpf_arch_text_invalidate+0x62/0xac
>> > | [<ffffffff8016c538>] bpf_prog_pack_free+0x9c/0x1b2
>> > | [<ffffffff8016c84a>] bpf_jit_binary_pack_free+0x20/0x4a
>> > | [<ffffffff8000f198>] bpf_jit_free+0x56/0x9e
>> > | [<ffffffff8016b43a>] bpf_prog_free_deferred+0x15a/0x182
>> > | [<ffffffff800576c4>] process_one_work+0x1b6/0x3d6
>> > | [<ffffffff80057d52>] worker_thread+0x84/0x378
>> > | [<ffffffff8005fc2c>] kthread+0xe8/0x108
>> > | [<ffffffff80003ffa>] ret_from_fork+0xe/0x20
>> >
>> > I'm digging into that now, and I would appreciate if you could run the
>> > test_tag on VF2 or similar (I'm missing that HW).
>> >
>> > It seems like we're hitting a bug with this series, so let's try to
>> > figure out where the problems is, prior merging it.
>>
>> Hmm, it looks like the bpf_arch_text_invalidate() implementation is a
>> bit problematic:
>>
>> +int bpf_arch_text_invalidate(void *dst, size_t len)
>> +{
>> + __le32 *ptr;
>> + int ret = 0;
>> + u32 inval = 0;
>> +
>> + for (ptr = dst; ret == 0 && len >= sizeof(u32); len -= sizeof(u32)) {
>> + mutex_lock(&text_mutex);
>> + ret = patch_text_nosync(ptr++, &inval, sizeof(u32));
>> + mutex_unlock(&text_mutex);
>> + }
>> +
>> + return ret;
>> +}
>>
>> Each patch_text_nosync() is a remote fence.i, and for a big "len", we'll
>> be flooded with remote fences.
>
> I understand this now, thanks for debugging this.
>
> We are calling patch_text_nosync() for each word (u32) which calls
> flush_icache_range() and therefore "fence.i" is inserted after every
> word.

But more importantly, it does a remote fence.i (which is an IPI to all
cores).

> I still don't fully understand how it causes this bug because I lack
> the prerequisite
> knowledge about test_tag and what the failing test is doing.

The test_tag is part of kselftest/bpf:
tools/testing/selftests/bpf/test_tag.c

TL;DR: it generates a bunch of programs, where some have a length of,
e.g, 41024. bpf_arch_text_invalidate() does ~10k of remote fences in
that case.

> But to solve this issue we would need a function like the x86
> text_poke_set() that will only
> insert a single "fence.i" after setting the whole memory area. This
> can be done by
> implementing a wrapper around patch_insn_write() which would set the memory area
> and at the end call flush_icache_range().
>
> Something like:
>
> void *text_set_nosync(void *dst, int c, size_t len)
> {
> __le32 *ptr;
> int ret = 0;
>
> for (ptr = dst; ret == 0 && len >= sizeof(u32); len -= sizeof(u32)) {
> ret = patch_insn_write(ptr++, &c, sizeof(u32));
> }
> if(!ret)
> flush_icache_range((uintptr_t) dst, (uintptr_t) dst + len);
>
> return ret;
> }
>
> Let me know if this looks correct or we need more details here.
> I will then send v2 with this implemented as a separate patch.

Can't we do better here? Perhaps a similar pattern like the 2 page fill?
Otherwise we'll have a bunch of fixmap updates as well.

I'd keep the patch_ prefix in the name for consistency. Please measure
the runtime of test_tag pre/after the change.

I don't know if your arm64 work has similar problems?

Björn

2023-08-14 16:54:22

by Puranjay Mohan

[permalink] [raw]

Subject: Re: [PATCH bpf-next 0/2] bpf, riscv: use BPF prog pack allocator in BPF JIT

Hi Björn,

On Mon, Aug 14, 2023 at 4:29 PM Björn Töpel <[email protected]> wrote:
>
> Puranjay Mohan <[email protected]> writes:
>
> > On Mon, Aug 14, 2023 at 12:40 PM Björn Töpel <[email protected]> wrote:
> >>
> >> Björn Töpel <[email protected]> writes:
> >>
> >> > Puranjay Mohan <[email protected]> writes:
> >> >
> >> >> BPF programs currently consume a page each on RISCV. For systems with many BPF
> >> >> programs, this adds significant pressure to instruction TLB. High iTLB pressure
> >> >> usually causes slow down for the whole system.
> >> >>
> >> >> Song Liu introduced the BPF prog pack allocator[1] to mitigate the above issue.
> >> >> It packs multiple BPF programs into a single huge page. It is currently only
> >> >> enabled for the x86_64 BPF JIT.
> >> >>
> >> >> I enabled this allocator on the ARM64 BPF JIT[2]. It is being reviewed now.
> >> >>
> >> >> This patch series enables the BPF prog pack allocator for the RISCV BPF JIT.
> >> >> This series needs a patch[3] from the ARM64 series to work.
> >> >>
> >> >> ======================================================
> >> >> Performance Analysis of prog pack allocator on RISCV64
> >> >> ======================================================
> >> >>
> >> >> Test setup:
> >> >> ===========
> >> >>
> >> >> Host machine: Debian GNU/Linux 11 (bullseye)
> >> >> Qemu Version: QEMU emulator version 8.0.3 (Debian 1:8.0.3+dfsg-1)
> >> >> u-boot-qemu Version: 2023.07+dfsg-1
> >> >> opensbi Version: 1.3-1
> >> >>
> >> >> To test the performance of the BPF prog pack allocator on RV, a stresser
> >> >> tool[4] linked below was built. This tool loads 8 BPF programs on the system and
> >> >> triggers 5 of them in an infinite loop by doing system calls.
> >> >>
> >> >> The runner script starts 20 instances of the above which loads 8*20=160 BPF
> >> >> programs on the system, 5*20=100 of which are being constantly triggered.
> >> >> The script is passed a command which would be run in the above environment.
> >> >>
> >> >> The script was run with following perf command:
> >> >> ./run.sh "perf stat -a \
> >> >> -e iTLB-load-misses \
> >> >> -e dTLB-load-misses \
> >> >> -e dTLB-store-misses \
> >> >> -e instructions \
> >> >> --timeout 60000"
> >> >>
> >> >> The output of the above command is discussed below before and after enabling the
> >> >> BPF prog pack allocator.
> >> >>
> >> >> The tests were run on qemu-system-riscv64 with 8 cpus, 16G memory. The rootfs
> >> >> was created using Bjorn's riscv-cross-builder[5] docker container linked below.
> >> >>
> >> >> Results
> >> >> =======
> >> >>
> >> >> Before enabling prog pack allocator:
> >> >> ------------------------------------
> >> >>
> >> >> Performance counter stats for 'system wide':
> >> >>
> >> >> 4939048 iTLB-load-misses
> >> >> 5468689 dTLB-load-misses
> >> >> 465234 dTLB-store-misses
> >> >> 1441082097998 instructions
> >> >>
> >> >> 60.045791200 seconds time elapsed
> >> >>
> >> >> After enabling prog pack allocator:
> >> >> -----------------------------------
> >> >>
> >> >> Performance counter stats for 'system wide':
> >> >>
> >> >> 3430035 iTLB-load-misses
> >> >> 5008745 dTLB-load-misses
> >> >> 409944 dTLB-store-misses
> >> >> 1441535637988 instructions
> >> >>
> >> >> 60.046296600 seconds time elapsed
> >> >>
> >> >> Improvements in metrics
> >> >> =======================
> >> >>
> >> >> It was expected that the iTLB-load-misses would decrease as now a single huge
> >> >> page is used to keep all the BPF programs compared to a single page for each
> >> >> program earlier.
> >> >>
> >> >> --------------------------------------------
> >> >> The improvement in iTLB-load-misses: -30.5 %
> >> >> --------------------------------------------
> >> >>
> >> >> I repeated this expriment more than 100 times in different setups and the
> >> >> improvement was always greater than 30%.
> >> >>
> >> >> This patch series is boot tested on the Starfive VisionFive 2 board[6].
> >> >> The performance analysis was not done on the board because it doesn't
> >> >> expose iTLB-load-misses, etc. The stresser program was run on the board to test
> >> >> the loading and unloading of BPF programs
> >> >>
> >> >> [1] https://lore.kernel.org/bpf/[email protected]/
> >> >> [2] https://lore.kernel.org/all/[email protected]/
> >> >> [3] https://lore.kernel.org/all/[email protected]/
> >> >> [4] https://github.com/puranjaymohan/BPF-Allocator-Bench
> >> >> [5] https://github.com/bjoto/riscv-cross-builder
> >> >> [6] https://www.starfivetech.com/en/site/boards
> >> >>
> >> >> Puranjay Mohan (2):
> >> >> riscv: Extend patch_text_nosync() for multiple pages
> >> >> bpf, riscv: use prog pack allocator in the BPF JIT
> >> >
> >> > I get a hang for "test_tag", but it's not directly related to your
> >> > series, but rather "remote fence.i".
> >> >
> >> > | rcu: INFO: rcu_sched detected stalls on CPUs/tasks:
> >> > | rcu: 0-....: (1400 ticks this GP) idle=d5e4/1/0x4000000000000000 softirq=5542/5542 fqs=1862
> >> > | rcu: (detected by 1, t=5252 jiffies, g=10253, q=195 ncpus=4)
> >> > | Task dump for CPU 0:
> >> > | task:kworker/0:5 state:R running task stack:0 pid:319 ppid:2 flags:0x00000008
> >> > | Workqueue: events bpf_prog_free_deferred
> >> > | Call Trace:
> >> > | [<ffffffff80cbc444>] __schedule+0x2d0/0x940
> >> > | watchdog: BUG: soft lockup - CPU#0 stuck for 21s! [kworker/0:5:319]
> >> > | Modules linked in: nls_iso8859_1 drm fuse i2c_core drm_panel_orientation_quirks backlight dm_mod configfs ip_tables x_tables
> >> > | CPU: 0 PID: 319 Comm: kworker/0:5 Not tainted 6.5.0-rc5 #1
> >> > | Hardware name: riscv-virtio,qemu (DT)
> >> > | Workqueue: events bpf_prog_free_deferred
> >> > | epc : __sbi_rfence_v02_call.isra.0+0x74/0x11a
> >> > | ra : __sbi_rfence_v02+0xda/0x1a4
> >> > | epc : ffffffff8000ab4c ra : ffffffff8000accc sp : ff20000001c9bbd0
> >> > | gp : ffffffff82078c48 tp : ff600000888e6a40 t0 : ff20000001c9bd44
> >> > | t1 : 0000000000000000 t2 : 0000000000000040 s0 : ff20000001c9bbf0
> >> > | s1 : 0000000000000010 a0 : 0000000000000000 a1 : 0000000000000000
> >> > | a2 : 0000000000000000 a3 : 0000000000000000 a4 : 0000000000000000
> >> > | a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000052464e43
> >> > | s2 : 000000000000ffff s3 : 00000000ffffffff s4 : ffffffff81667528
> >> > | s5 : 0000000000000000 s6 : 0000000000000000 s7 : 0000000000000000
> >> > | s8 : 0000000000000001 s9 : 0000000000000003 s10: 0000000000000040
> >> > | s11: ffffffff8207d240 t3 : 000000000000000f t4 : 000000000000002a
> >> > | t5 : ff600000872df140 t6 : ffffffff81e26828
> >> > | status: 0000000200000120 badaddr: 0000000000000000 cause: 8000000000000005
> >> > | [<ffffffff8000ab4c>] __sbi_rfence_v02_call.isra.0+0x74/0x11a
> >> > | [<ffffffff8000accc>] __sbi_rfence_v02+0xda/0x1a4
> >> > | [<ffffffff8000a886>] sbi_remote_fence_i+0x1e/0x26
> >> > | [<ffffffff8000cee2>] flush_icache_all+0x1a/0x48
> >> > | [<ffffffff80007736>] patch_text_nosync+0x6c/0x8c
> >> > | [<ffffffff8000f0f8>] bpf_arch_text_invalidate+0x62/0xac
> >> > | [<ffffffff8016c538>] bpf_prog_pack_free+0x9c/0x1b2
> >> > | [<ffffffff8016c84a>] bpf_jit_binary_pack_free+0x20/0x4a
> >> > | [<ffffffff8000f198>] bpf_jit_free+0x56/0x9e
> >> > | [<ffffffff8016b43a>] bpf_prog_free_deferred+0x15a/0x182
> >> > | [<ffffffff800576c4>] process_one_work+0x1b6/0x3d6
> >> > | [<ffffffff80057d52>] worker_thread+0x84/0x378
> >> > | [<ffffffff8005fc2c>] kthread+0xe8/0x108
> >> > | [<ffffffff80003ffa>] ret_from_fork+0xe/0x20
> >> >
> >> > I'm digging into that now, and I would appreciate if you could run the
> >> > test_tag on VF2 or similar (I'm missing that HW).
> >> >
> >> > It seems like we're hitting a bug with this series, so let's try to
> >> > figure out where the problems is, prior merging it.
> >>
> >> Hmm, it looks like the bpf_arch_text_invalidate() implementation is a
> >> bit problematic:
> >>
> >> +int bpf_arch_text_invalidate(void *dst, size_t len)
> >> +{
> >> + __le32 *ptr;
> >> + int ret = 0;
> >> + u32 inval = 0;
> >> +
> >> + for (ptr = dst; ret == 0 && len >= sizeof(u32); len -= sizeof(u32)) {
> >> + mutex_lock(&text_mutex);
> >> + ret = patch_text_nosync(ptr++, &inval, sizeof(u32));
> >> + mutex_unlock(&text_mutex);
> >> + }
> >> +
> >> + return ret;
> >> +}
> >>
> >> Each patch_text_nosync() is a remote fence.i, and for a big "len", we'll
> >> be flooded with remote fences.
> >
> > I understand this now, thanks for debugging this.
> >
> > We are calling patch_text_nosync() for each word (u32) which calls
> > flush_icache_range() and therefore "fence.i" is inserted after every
> > word.
>
> But more importantly, it does a remote fence.i (which is an IPI to all
> cores).
>
> > I still don't fully understand how it causes this bug because I lack
> > the prerequisite
> > knowledge about test_tag and what the failing test is doing.
>
> The test_tag is part of kselftest/bpf:
> tools/testing/selftests/bpf/test_tag.c
>
> TL;DR: it generates a bunch of programs, where some have a length of,
> e.g, 41024. bpf_arch_text_invalidate() does ~10k of remote fences in
> that case.
>
> > But to solve this issue we would need a function like the x86
> > text_poke_set() that will only
> > insert a single "fence.i" after setting the whole memory area. This
> > can be done by
> > implementing a wrapper around patch_insn_write() which would set the memory area
> > and at the end call flush_icache_range().
> >
> > Something like:
> >
> > void *text_set_nosync(void *dst, int c, size_t len)
> > {
> > __le32 *ptr;
> > int ret = 0;
> >
> > for (ptr = dst; ret == 0 && len >= sizeof(u32); len -= sizeof(u32)) {
> > ret = patch_insn_write(ptr++, &c, sizeof(u32));
> > }
> > if(!ret)
> > flush_icache_range((uintptr_t) dst, (uintptr_t) dst + len);
> >
> > return ret;
> > }
> >
> > Let me know if this looks correct or we need more details here.
> > I will then send v2 with this implemented as a separate patch.
>
> Can't we do better here? Perhaps a similar pattern like the 2 page fill?
> Otherwise we'll have a bunch of fixmap updates as well.

I agree that we can make it more efficient by first copying the value to a
RW buffer using normal memcpy() and then copying that area to the RO area
using patch_insn_write(). Then it would solve both problems. Or we implement
a new function like patch_insn_write() that does the 2 page map and
set explicitly.

Which approach would you prefer?
1) Wrapper around patch_insn_write() that first memsets a RW buffer and then
copies the complete RW buffer to the RO area by calling
patch_insn_write() with len.

2) A new function like patch_insn_write() that takes dst, src, len and
maps the dst, 2 pages
at a time and sets it to *src in a loop.

>
> I'd keep the patch_ prefix in the name for consistency. Please measure
> the runtime of test_tag pre/after the change.

test_tag currently wouldn't even complete right? with the current
version of the patch?

>
> I don't know if your arm64 work has similar problems?

Thanks for bringing this up. I will revisit that and verify if
test_tag is working
there. There also the bpf_arch_text_invalidate() is calling
aarch64_insn_patch_text_nosync()
in a loop that in turn calls caches_clean_inval_pou(). So I might see
similar issues there.
I think https://github.com/kernel-patches doesn't run test_tag hence I
might have missed it.

>
>
> Björn

Thanks,
Puranjay

2023-08-14 23:35:05

by Björn Töpel

[permalink] [raw]

Subject: Re: [PATCH bpf-next 0/2] bpf, riscv: use BPF prog pack allocator in BPF JIT

Puranjay Mohan <[email protected]> writes:

> Hi Björn,
>
> On Mon, Aug 14, 2023 at 4:29 PM Björn Töpel <[email protected]> wrote:
>>
>> Puranjay Mohan <[email protected]> writes:
>>
>> > On Mon, Aug 14, 2023 at 12:40 PM Björn Töpel <[email protected]> wrote:
>> >>
>> >> Björn Töpel <[email protected]> writes:
>> >>
>> >> > Puranjay Mohan <[email protected]> writes:
>> >> >
>> >> >> BPF programs currently consume a page each on RISCV. For systems with many BPF
>> >> >> programs, this adds significant pressure to instruction TLB. High iTLB pressure
>> >> >> usually causes slow down for the whole system.
>> >> >>
>> >> >> Song Liu introduced the BPF prog pack allocator[1] to mitigate the above issue.
>> >> >> It packs multiple BPF programs into a single huge page. It is currently only
>> >> >> enabled for the x86_64 BPF JIT.
>> >> >>
>> >> >> I enabled this allocator on the ARM64 BPF JIT[2]. It is being reviewed now.
>> >> >>
>> >> >> This patch series enables the BPF prog pack allocator for the RISCV BPF JIT.
>> >> >> This series needs a patch[3] from the ARM64 series to work.
>> >> >>
>> >> >> ======================================================
>> >> >> Performance Analysis of prog pack allocator on RISCV64
>> >> >> ======================================================
>> >> >>
>> >> >> Test setup:
>> >> >> ===========
>> >> >>
>> >> >> Host machine: Debian GNU/Linux 11 (bullseye)
>> >> >> Qemu Version: QEMU emulator version 8.0.3 (Debian 1:8.0.3+dfsg-1)
>> >> >> u-boot-qemu Version: 2023.07+dfsg-1
>> >> >> opensbi Version: 1.3-1
>> >> >>
>> >> >> To test the performance of the BPF prog pack allocator on RV, a stresser
>> >> >> tool[4] linked below was built. This tool loads 8 BPF programs on the system and
>> >> >> triggers 5 of them in an infinite loop by doing system calls.
>> >> >>
>> >> >> The runner script starts 20 instances of the above which loads 8*20=160 BPF
>> >> >> programs on the system, 5*20=100 of which are being constantly triggered.
>> >> >> The script is passed a command which would be run in the above environment.
>> >> >>
>> >> >> The script was run with following perf command:
>> >> >> ./run.sh "perf stat -a \
>> >> >> -e iTLB-load-misses \
>> >> >> -e dTLB-load-misses \
>> >> >> -e dTLB-store-misses \
>> >> >> -e instructions \
>> >> >> --timeout 60000"
>> >> >>
>> >> >> The output of the above command is discussed below before and after enabling the
>> >> >> BPF prog pack allocator.
>> >> >>
>> >> >> The tests were run on qemu-system-riscv64 with 8 cpus, 16G memory. The rootfs
>> >> >> was created using Bjorn's riscv-cross-builder[5] docker container linked below.
>> >> >>
>> >> >> Results
>> >> >> =======
>> >> >>
>> >> >> Before enabling prog pack allocator:
>> >> >> ------------------------------------
>> >> >>
>> >> >> Performance counter stats for 'system wide':
>> >> >>
>> >> >> 4939048 iTLB-load-misses
>> >> >> 5468689 dTLB-load-misses
>> >> >> 465234 dTLB-store-misses
>> >> >> 1441082097998 instructions
>> >> >>
>> >> >> 60.045791200 seconds time elapsed
>> >> >>
>> >> >> After enabling prog pack allocator:
>> >> >> -----------------------------------
>> >> >>
>> >> >> Performance counter stats for 'system wide':
>> >> >>
>> >> >> 3430035 iTLB-load-misses
>> >> >> 5008745 dTLB-load-misses
>> >> >> 409944 dTLB-store-misses
>> >> >> 1441535637988 instructions
>> >> >>
>> >> >> 60.046296600 seconds time elapsed
>> >> >>
>> >> >> Improvements in metrics
>> >> >> =======================
>> >> >>
>> >> >> It was expected that the iTLB-load-misses would decrease as now a single huge
>> >> >> page is used to keep all the BPF programs compared to a single page for each
>> >> >> program earlier.
>> >> >>
>> >> >> --------------------------------------------
>> >> >> The improvement in iTLB-load-misses: -30.5 %
>> >> >> --------------------------------------------
>> >> >>
>> >> >> I repeated this expriment more than 100 times in different setups and the
>> >> >> improvement was always greater than 30%.
>> >> >>
>> >> >> This patch series is boot tested on the Starfive VisionFive 2 board[6].
>> >> >> The performance analysis was not done on the board because it doesn't
>> >> >> expose iTLB-load-misses, etc. The stresser program was run on the board to test
>> >> >> the loading and unloading of BPF programs
>> >> >>
>> >> >> [1] https://lore.kernel.org/bpf/[email protected]/
>> >> >> [2] https://lore.kernel.org/all/[email protected]/
>> >> >> [3] https://lore.kernel.org/all/[email protected]/
>> >> >> [4] https://github.com/puranjaymohan/BPF-Allocator-Bench
>> >> >> [5] https://github.com/bjoto/riscv-cross-builder
>> >> >> [6] https://www.starfivetech.com/en/site/boards
>> >> >>
>> >> >> Puranjay Mohan (2):
>> >> >> riscv: Extend patch_text_nosync() for multiple pages
>> >> >> bpf, riscv: use prog pack allocator in the BPF JIT
>> >> >
>> >> > I get a hang for "test_tag", but it's not directly related to your
>> >> > series, but rather "remote fence.i".
>> >> >
>> >> > | rcu: INFO: rcu_sched detected stalls on CPUs/tasks:
>> >> > | rcu: 0-....: (1400 ticks this GP) idle=d5e4/1/0x4000000000000000 softirq=5542/5542 fqs=1862
>> >> > | rcu: (detected by 1, t=5252 jiffies, g=10253, q=195 ncpus=4)
>> >> > | Task dump for CPU 0:
>> >> > | task:kworker/0:5 state:R running task stack:0 pid:319 ppid:2 flags:0x00000008
>> >> > | Workqueue: events bpf_prog_free_deferred
>> >> > | Call Trace:
>> >> > | [<ffffffff80cbc444>] __schedule+0x2d0/0x940
>> >> > | watchdog: BUG: soft lockup - CPU#0 stuck for 21s! [kworker/0:5:319]
>> >> > | Modules linked in: nls_iso8859_1 drm fuse i2c_core drm_panel_orientation_quirks backlight dm_mod configfs ip_tables x_tables
>> >> > | CPU: 0 PID: 319 Comm: kworker/0:5 Not tainted 6.5.0-rc5 #1
>> >> > | Hardware name: riscv-virtio,qemu (DT)
>> >> > | Workqueue: events bpf_prog_free_deferred
>> >> > | epc : __sbi_rfence_v02_call.isra.0+0x74/0x11a
>> >> > | ra : __sbi_rfence_v02+0xda/0x1a4
>> >> > | epc : ffffffff8000ab4c ra : ffffffff8000accc sp : ff20000001c9bbd0
>> >> > | gp : ffffffff82078c48 tp : ff600000888e6a40 t0 : ff20000001c9bd44
>> >> > | t1 : 0000000000000000 t2 : 0000000000000040 s0 : ff20000001c9bbf0
>> >> > | s1 : 0000000000000010 a0 : 0000000000000000 a1 : 0000000000000000
>> >> > | a2 : 0000000000000000 a3 : 0000000000000000 a4 : 0000000000000000
>> >> > | a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000052464e43
>> >> > | s2 : 000000000000ffff s3 : 00000000ffffffff s4 : ffffffff81667528
>> >> > | s5 : 0000000000000000 s6 : 0000000000000000 s7 : 0000000000000000
>> >> > | s8 : 0000000000000001 s9 : 0000000000000003 s10: 0000000000000040
>> >> > | s11: ffffffff8207d240 t3 : 000000000000000f t4 : 000000000000002a
>> >> > | t5 : ff600000872df140 t6 : ffffffff81e26828
>> >> > | status: 0000000200000120 badaddr: 0000000000000000 cause: 8000000000000005
>> >> > | [<ffffffff8000ab4c>] __sbi_rfence_v02_call.isra.0+0x74/0x11a
>> >> > | [<ffffffff8000accc>] __sbi_rfence_v02+0xda/0x1a4
>> >> > | [<ffffffff8000a886>] sbi_remote_fence_i+0x1e/0x26
>> >> > | [<ffffffff8000cee2>] flush_icache_all+0x1a/0x48
>> >> > | [<ffffffff80007736>] patch_text_nosync+0x6c/0x8c
>> >> > | [<ffffffff8000f0f8>] bpf_arch_text_invalidate+0x62/0xac
>> >> > | [<ffffffff8016c538>] bpf_prog_pack_free+0x9c/0x1b2
>> >> > | [<ffffffff8016c84a>] bpf_jit_binary_pack_free+0x20/0x4a
>> >> > | [<ffffffff8000f198>] bpf_jit_free+0x56/0x9e
>> >> > | [<ffffffff8016b43a>] bpf_prog_free_deferred+0x15a/0x182
>> >> > | [<ffffffff800576c4>] process_one_work+0x1b6/0x3d6
>> >> > | [<ffffffff80057d52>] worker_thread+0x84/0x378
>> >> > | [<ffffffff8005fc2c>] kthread+0xe8/0x108
>> >> > | [<ffffffff80003ffa>] ret_from_fork+0xe/0x20
>> >> >
>> >> > I'm digging into that now, and I would appreciate if you could run the
>> >> > test_tag on VF2 or similar (I'm missing that HW).
>> >> >
>> >> > It seems like we're hitting a bug with this series, so let's try to
>> >> > figure out where the problems is, prior merging it.
>> >>
>> >> Hmm, it looks like the bpf_arch_text_invalidate() implementation is a
>> >> bit problematic:
>> >>
>> >> +int bpf_arch_text_invalidate(void *dst, size_t len)
>> >> +{
>> >> + __le32 *ptr;
>> >> + int ret = 0;
>> >> + u32 inval = 0;
>> >> +
>> >> + for (ptr = dst; ret == 0 && len >= sizeof(u32); len -= sizeof(u32)) {
>> >> + mutex_lock(&text_mutex);
>> >> + ret = patch_text_nosync(ptr++, &inval, sizeof(u32));
>> >> + mutex_unlock(&text_mutex);
>> >> + }
>> >> +
>> >> + return ret;
>> >> +}
>> >>
>> >> Each patch_text_nosync() is a remote fence.i, and for a big "len", we'll
>> >> be flooded with remote fences.
>> >
>> > I understand this now, thanks for debugging this.
>> >
>> > We are calling patch_text_nosync() for each word (u32) which calls
>> > flush_icache_range() and therefore "fence.i" is inserted after every
>> > word.
>>
>> But more importantly, it does a remote fence.i (which is an IPI to all
>> cores).
>>
>> > I still don't fully understand how it causes this bug because I lack
>> > the prerequisite
>> > knowledge about test_tag and what the failing test is doing.
>>
>> The test_tag is part of kselftest/bpf:
>> tools/testing/selftests/bpf/test_tag.c
>>
>> TL;DR: it generates a bunch of programs, where some have a length of,
>> e.g, 41024. bpf_arch_text_invalidate() does ~10k of remote fences in
>> that case.
>>
>> > But to solve this issue we would need a function like the x86
>> > text_poke_set() that will only
>> > insert a single "fence.i" after setting the whole memory area. This
>> > can be done by
>> > implementing a wrapper around patch_insn_write() which would set the memory area
>> > and at the end call flush_icache_range().
>> >
>> > Something like:
>> >
>> > void *text_set_nosync(void *dst, int c, size_t len)
>> > {
>> > __le32 *ptr;
>> > int ret = 0;
>> >
>> > for (ptr = dst; ret == 0 && len >= sizeof(u32); len -= sizeof(u32)) {
>> > ret = patch_insn_write(ptr++, &c, sizeof(u32));
>> > }
>> > if(!ret)
>> > flush_icache_range((uintptr_t) dst, (uintptr_t) dst + len);
>> >
>> > return ret;
>> > }
>> >
>> > Let me know if this looks correct or we need more details here.
>> > I will then send v2 with this implemented as a separate patch.
>>
>> Can't we do better here? Perhaps a similar pattern like the 2 page fill?
>> Otherwise we'll have a bunch of fixmap updates as well.
>
> I agree that we can make it more efficient by first copying the value to a
> RW buffer using normal memcpy() and then copying that area to the RO area
> using patch_insn_write(). Then it would solve both problems. Or we implement
> a new function like patch_insn_write() that does the 2 page map and
> set explicitly.
>
> Which approach would you prefer?
> 1) Wrapper around patch_insn_write() that first memsets a RW buffer and then
> copies the complete RW buffer to the RO area by calling
> patch_insn_write() with len.
>
> 2) A new function like patch_insn_write() that takes dst, src, len and
> maps the dst, 2 pages
> at a time and sets it to *src in a loop.

A think 2) is the way to go: A "patch_insn_set(void *addr, u16 c, size_t
len)" or smth.

>> I'd keep the patch_ prefix in the name for consistency. Please measure
>> the runtime of test_tag pre/after the change.
>
> test_tag currently wouldn't even complete right? with the current
> version of the patch?

It will, but you'd need to crank up the watchdog timeout. :-) What I
meant was test_tag w/ and w/o your pack allocator.

>> I don't know if your arm64 work has similar problems?
>
> Thanks for bringing this up. I will revisit that and verify if
> test_tag is working
> there. There also the bpf_arch_text_invalidate() is calling
> aarch64_insn_patch_text_nosync()
> in a loop that in turn calls caches_clean_inval_pou(). So I might see
> similar issues there.

Ok!

> I think https://github.com/kernel-patches doesn't run test_tag hence I
> might have missed it.

You're right, it doesn't. I usually run the full suite locally.

Again, thanks a lot for spending time on this! It's nice work!
Björn