mbox series

[bpf-next,v3,0/3] bpf, riscv: use BPF prog pack allocator in BPF JIT

Message ID 20230828165958.1714079-1-puranjay12@gmail.com (mailing list archive)
Headers show
Series bpf, riscv: use BPF prog pack allocator in BPF JIT | expand

Message

Puranjay Mohan Aug. 28, 2023, 4:59 p.m. UTC
Changes in v2 -> v3:
1. Fix maximum width of code in patches from 80 to 100. [All patches]
2. Add checks for ctx->ro_insns == NULL. [Patch 3]
3. Fix check for edge condition where amount of text to set > 2 * pagesize
   [Patch 1 and 2]
4. Add reviewed-by in patches.
5. Adding results of selftest here:
   Using the command: ./test_progs on qemu
   Without the series: Summary: 336/3162 PASSED, 56 SKIPPED, 90 FAILED
   With this series: Summary: 336/3162 PASSED, 56 SKIPPED, 90 FAILED

Changes in v1 -> v2:
1. Implement a new function patch_text_set_nosync() to be used in bpf_arch_text_invalidate().
   The implementation in v1 called patch_text_nosync() in a loop and it was bad as it would
   call flush_icache_range() for every word making it really slow. This was found by running
   the test_tag selftest which would take forever to complete.

Here is some data to prove the V2 fixes the problem:

Without this series:
root@rv-selftester:~/src/kselftest/bpf# time ./test_tag
test_tag: OK (40945 tests)

real    7m47.562s
user    0m24.145s
sys     6m37.064s

With this series applied:
root@rv-selftester:~/src/selftest/bpf# time ./test_tag
test_tag: OK (40945 tests)

real    7m29.472s
user    0m25.865s
sys     6m18.401s

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/20220204185742.271030-1-song@kernel.org/
[2] https://lore.kernel.org/all/20230626085811.3192402-1-puranjay12@gmail.com/
[3] https://lore.kernel.org/all/20230626085811.3192402-2-puranjay12@gmail.com/
[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 (3):
  riscv: extend patch_text_nosync() for multiple pages
  riscv: implement a memset like function for text
  bpf, riscv: use prog pack allocator in the BPF JIT

 arch/riscv/include/asm/patch.h  |   1 +
 arch/riscv/kernel/patch.c       | 114 ++++++++++++++++++++++++++++++--
 arch/riscv/net/bpf_jit.h        |   3 +
 arch/riscv/net/bpf_jit_comp64.c |  60 +++++++++++++----
 arch/riscv/net/bpf_jit_core.c   | 106 ++++++++++++++++++++++++-----
 5 files changed, 251 insertions(+), 33 deletions(-)

Comments

Björn Töpel Aug. 29, 2023, 10:06 a.m. UTC | #1
Puranjay Mohan <puranjay12@gmail.com> writes:

> Changes in v2 -> v3:
> 1. Fix maximum width of code in patches from 80 to 100. [All patches]
> 2. Add checks for ctx->ro_insns == NULL. [Patch 3]
> 3. Fix check for edge condition where amount of text to set > 2 * pagesize
>    [Patch 1 and 2]
> 4. Add reviewed-by in patches.
> 5. Adding results of selftest here:
>    Using the command: ./test_progs on qemu
>    Without the series: Summary: 336/3162 PASSED, 56 SKIPPED, 90 FAILED
>    With this series: Summary: 336/3162 PASSED, 56 SKIPPED, 90 FAILED
>
> Changes in v1 -> v2:
> 1. Implement a new function patch_text_set_nosync() to be used in bpf_arch_text_invalidate().
>    The implementation in v1 called patch_text_nosync() in a loop and it was bad as it would
>    call flush_icache_range() for every word making it really slow. This was found by running
>    the test_tag selftest which would take forever to complete.
>
> Here is some data to prove the V2 fixes the problem:
>
> Without this series:
> root@rv-selftester:~/src/kselftest/bpf# time ./test_tag
> test_tag: OK (40945 tests)
>
> real    7m47.562s
> user    0m24.145s
> sys     6m37.064s
>
> With this series applied:
> root@rv-selftester:~/src/selftest/bpf# time ./test_tag
> test_tag: OK (40945 tests)
>
> real    7m29.472s
> user    0m25.865s
> sys     6m18.401s
>
> 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/20220204185742.271030-1-song@kernel.org/
> [2] https://lore.kernel.org/all/20230626085811.3192402-1-puranjay12@gmail.com/
> [3] https://lore.kernel.org/all/20230626085811.3192402-2-puranjay12@gmail.com/
> [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 (3):
>   riscv: extend patch_text_nosync() for multiple pages
>   riscv: implement a memset like function for text
>   bpf, riscv: use prog pack allocator in the BPF JIT

Thank you! For the series:

Acked-by: Björn Töpel <bjorn@kernel.org>
Tested-by: Björn Töpel <bjorn@rivosinc.com>

@Alexei @Daniel This series depends on a core BPF patch from the Arm
                series [3].

@Palmer LMK if you have any concerns taking the RISC-V text patching
        stuff via the BPF tree.


Björn
Daniel Borkmann Aug. 30, 2023, 8:18 a.m. UTC | #2
On 8/29/23 12:06 PM, Björn Töpel wrote:
> Puranjay Mohan <puranjay12@gmail.com> writes:
> 
>> Changes in v2 -> v3:
>> 1. Fix maximum width of code in patches from 80 to 100. [All patches]
>> 2. Add checks for ctx->ro_insns == NULL. [Patch 3]
>> 3. Fix check for edge condition where amount of text to set > 2 * pagesize
>>     [Patch 1 and 2]
>> 4. Add reviewed-by in patches.
>> 5. Adding results of selftest here:
>>     Using the command: ./test_progs on qemu
>>     Without the series: Summary: 336/3162 PASSED, 56 SKIPPED, 90 FAILED
>>     With this series: Summary: 336/3162 PASSED, 56 SKIPPED, 90 FAILED
>>
>> Changes in v1 -> v2:
>> 1. Implement a new function patch_text_set_nosync() to be used in bpf_arch_text_invalidate().
>>     The implementation in v1 called patch_text_nosync() in a loop and it was bad as it would
>>     call flush_icache_range() for every word making it really slow. This was found by running
>>     the test_tag selftest which would take forever to complete.
>>
>> Here is some data to prove the V2 fixes the problem:
>>
>> Without this series:
>> root@rv-selftester:~/src/kselftest/bpf# time ./test_tag
>> test_tag: OK (40945 tests)
>>
>> real    7m47.562s
>> user    0m24.145s
>> sys     6m37.064s
>>
>> With this series applied:
>> root@rv-selftester:~/src/selftest/bpf# time ./test_tag
>> test_tag: OK (40945 tests)
>>
>> real    7m29.472s
>> user    0m25.865s
>> sys     6m18.401s
>>
>> 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/20220204185742.271030-1-song@kernel.org/
>> [2] https://lore.kernel.org/all/20230626085811.3192402-1-puranjay12@gmail.com/
>> [3] https://lore.kernel.org/all/20230626085811.3192402-2-puranjay12@gmail.com/
>> [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 (3):
>>    riscv: extend patch_text_nosync() for multiple pages
>>    riscv: implement a memset like function for text
>>    bpf, riscv: use prog pack allocator in the BPF JIT
> 
> Thank you! For the series:
> 
> Acked-by: Björn Töpel <bjorn@kernel.org>
> Tested-by: Björn Töpel <bjorn@rivosinc.com>
> 
> @Alexei @Daniel This series depends on a core BPF patch from the Arm
>                  series [3].
> 
> @Palmer LMK if you have any concerns taking the RISC-V text patching
>          stuff via the BPF tree.

Palmer, did the riscv PR already go to Linus?

If not yet, perhaps you could ship this series along with your PR to Linus
during this merge window given the big net PR (incl. bpf) was already merged
yesterday. So from our side only fixes ship to Linus.

Otherwise we could take it into bpf-next for the next dev cycle if there are
no objections, let us know.

Thanks,
Daniel
Björn Töpel Aug. 30, 2023, 8:30 a.m. UTC | #3
Daniel Borkmann <daniel@iogearbox.net> writes:

> On 8/29/23 12:06 PM, Björn Töpel wrote:
>> Puranjay Mohan <puranjay12@gmail.com> writes:
>> 
>>> Changes in v2 -> v3:
>>> 1. Fix maximum width of code in patches from 80 to 100. [All patches]
>>> 2. Add checks for ctx->ro_insns == NULL. [Patch 3]
>>> 3. Fix check for edge condition where amount of text to set > 2 * pagesize
>>>     [Patch 1 and 2]
>>> 4. Add reviewed-by in patches.
>>> 5. Adding results of selftest here:
>>>     Using the command: ./test_progs on qemu
>>>     Without the series: Summary: 336/3162 PASSED, 56 SKIPPED, 90 FAILED
>>>     With this series: Summary: 336/3162 PASSED, 56 SKIPPED, 90 FAILED
>>>
>>> Changes in v1 -> v2:
>>> 1. Implement a new function patch_text_set_nosync() to be used in bpf_arch_text_invalidate().
>>>     The implementation in v1 called patch_text_nosync() in a loop and it was bad as it would
>>>     call flush_icache_range() for every word making it really slow. This was found by running
>>>     the test_tag selftest which would take forever to complete.
>>>
>>> Here is some data to prove the V2 fixes the problem:
>>>
>>> Without this series:
>>> root@rv-selftester:~/src/kselftest/bpf# time ./test_tag
>>> test_tag: OK (40945 tests)
>>>
>>> real    7m47.562s
>>> user    0m24.145s
>>> sys     6m37.064s
>>>
>>> With this series applied:
>>> root@rv-selftester:~/src/selftest/bpf# time ./test_tag
>>> test_tag: OK (40945 tests)
>>>
>>> real    7m29.472s
>>> user    0m25.865s
>>> sys     6m18.401s
>>>
>>> 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/20220204185742.271030-1-song@kernel.org/
>>> [2] https://lore.kernel.org/all/20230626085811.3192402-1-puranjay12@gmail.com/
>>> [3] https://lore.kernel.org/all/20230626085811.3192402-2-puranjay12@gmail.com/
>>> [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 (3):
>>>    riscv: extend patch_text_nosync() for multiple pages
>>>    riscv: implement a memset like function for text
>>>    bpf, riscv: use prog pack allocator in the BPF JIT
>> 
>> Thank you! For the series:
>> 
>> Acked-by: Björn Töpel <bjorn@kernel.org>
>> Tested-by: Björn Töpel <bjorn@rivosinc.com>
>> 
>> @Alexei @Daniel This series depends on a core BPF patch from the Arm
>>                  series [3].
>> 
>> @Palmer LMK if you have any concerns taking the RISC-V text patching
>>          stuff via the BPF tree.
>
> Palmer, did the riscv PR already go to Linus?
>
> If not yet, perhaps you could ship this series along with your PR to Linus
> during this merge window given the big net PR (incl. bpf) was already merged
> yesterday. So from our side only fixes ship to Linus.
>
> Otherwise we could take it into bpf-next for the next dev cycle if there are
> no objections, let us know.

Daniel, I'll bring this up on the RISC-V patchwork sync meeting [1]
today.

[1] https://lore.kernel.org/linux-riscv/mhng-775d4068-6c1e-48a4-a1dc-b4a76ff26bb3@palmer-ri-x1c9a/
Palmer Dabbelt Aug. 30, 2023, 1:54 p.m. UTC | #4
On Wed, 30 Aug 2023 01:18:46 PDT (-0700), daniel@iogearbox.net wrote:
> On 8/29/23 12:06 PM, Björn Töpel wrote:
>> Puranjay Mohan <puranjay12@gmail.com> writes:
>>
>>> Changes in v2 -> v3:
>>> 1. Fix maximum width of code in patches from 80 to 100. [All patches]
>>> 2. Add checks for ctx->ro_insns == NULL. [Patch 3]
>>> 3. Fix check for edge condition where amount of text to set > 2 * pagesize
>>>     [Patch 1 and 2]
>>> 4. Add reviewed-by in patches.
>>> 5. Adding results of selftest here:
>>>     Using the command: ./test_progs on qemu
>>>     Without the series: Summary: 336/3162 PASSED, 56 SKIPPED, 90 FAILED
>>>     With this series: Summary: 336/3162 PASSED, 56 SKIPPED, 90 FAILED
>>>
>>> Changes in v1 -> v2:
>>> 1. Implement a new function patch_text_set_nosync() to be used in bpf_arch_text_invalidate().
>>>     The implementation in v1 called patch_text_nosync() in a loop and it was bad as it would
>>>     call flush_icache_range() for every word making it really slow. This was found by running
>>>     the test_tag selftest which would take forever to complete.
>>>
>>> Here is some data to prove the V2 fixes the problem:
>>>
>>> Without this series:
>>> root@rv-selftester:~/src/kselftest/bpf# time ./test_tag
>>> test_tag: OK (40945 tests)
>>>
>>> real    7m47.562s
>>> user    0m24.145s
>>> sys     6m37.064s
>>>
>>> With this series applied:
>>> root@rv-selftester:~/src/selftest/bpf# time ./test_tag
>>> test_tag: OK (40945 tests)
>>>
>>> real    7m29.472s
>>> user    0m25.865s
>>> sys     6m18.401s
>>>
>>> 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/20220204185742.271030-1-song@kernel.org/
>>> [2] https://lore.kernel.org/all/20230626085811.3192402-1-puranjay12@gmail.com/
>>> [3] https://lore.kernel.org/all/20230626085811.3192402-2-puranjay12@gmail.com/
>>> [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 (3):
>>>    riscv: extend patch_text_nosync() for multiple pages
>>>    riscv: implement a memset like function for text
>>>    bpf, riscv: use prog pack allocator in the BPF JIT
>>
>> Thank you! For the series:
>>
>> Acked-by: Björn Töpel <bjorn@kernel.org>
>> Tested-by: Björn Töpel <bjorn@rivosinc.com>
>>
>> @Alexei @Daniel This series depends on a core BPF patch from the Arm
>>                  series [3].
>>
>> @Palmer LMK if you have any concerns taking the RISC-V text patching
>>          stuff via the BPF tree.
>
> Palmer, did the riscv PR already go to Linus?

Not yet, I usually send on Friday mornings -- and I also generally send 
two, as there's some stragglers/fixes for the second week.  I'm fine 
taking it (Bjorn just poked me), can someone provide a base commit?  
Bjorn says it depends on something in Linus' tree, so I'll just pick it 
up as a straggler for next week.

Also, do you mind sending an Ack?

> If not yet, perhaps you could ship this series along with your PR to Linus
> during this merge window given the big net PR (incl. bpf) was already merged
> yesterday. So from our side only fixes ship to Linus.
>
> Otherwise we could take it into bpf-next for the next dev cycle if there are
> no objections, let us know.
>
> Thanks,
> Daniel
Björn Töpel Aug. 30, 2023, 1:59 p.m. UTC | #5
Daniel Borkmann <daniel@iogearbox.net> writes:

> On 8/29/23 12:06 PM, Björn Töpel wrote:
>> Puranjay Mohan <puranjay12@gmail.com> writes:
>> 
>>> Changes in v2 -> v3:
>>> 1. Fix maximum width of code in patches from 80 to 100. [All patches]
>>> 2. Add checks for ctx->ro_insns == NULL. [Patch 3]
>>> 3. Fix check for edge condition where amount of text to set > 2 * pagesize
>>>     [Patch 1 and 2]
>>> 4. Add reviewed-by in patches.
>>> 5. Adding results of selftest here:
>>>     Using the command: ./test_progs on qemu
>>>     Without the series: Summary: 336/3162 PASSED, 56 SKIPPED, 90 FAILED
>>>     With this series: Summary: 336/3162 PASSED, 56 SKIPPED, 90 FAILED
>>>
>>> Changes in v1 -> v2:
>>> 1. Implement a new function patch_text_set_nosync() to be used in bpf_arch_text_invalidate().
>>>     The implementation in v1 called patch_text_nosync() in a loop and it was bad as it would
>>>     call flush_icache_range() for every word making it really slow. This was found by running
>>>     the test_tag selftest which would take forever to complete.
>>>
>>> Here is some data to prove the V2 fixes the problem:
>>>
>>> Without this series:
>>> root@rv-selftester:~/src/kselftest/bpf# time ./test_tag
>>> test_tag: OK (40945 tests)
>>>
>>> real    7m47.562s
>>> user    0m24.145s
>>> sys     6m37.064s
>>>
>>> With this series applied:
>>> root@rv-selftester:~/src/selftest/bpf# time ./test_tag
>>> test_tag: OK (40945 tests)
>>>
>>> real    7m29.472s
>>> user    0m25.865s
>>> sys     6m18.401s
>>>
>>> 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/20220204185742.271030-1-song@kernel.org/
>>> [2] https://lore.kernel.org/all/20230626085811.3192402-1-puranjay12@gmail.com/
>>> [3] https://lore.kernel.org/all/20230626085811.3192402-2-puranjay12@gmail.com/
>>> [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 (3):
>>>    riscv: extend patch_text_nosync() for multiple pages
>>>    riscv: implement a memset like function for text
>>>    bpf, riscv: use prog pack allocator in the BPF JIT
>> 
>> Thank you! For the series:
>> 
>> Acked-by: Björn Töpel <bjorn@kernel.org>
>> Tested-by: Björn Töpel <bjorn@rivosinc.com>
>> 
>> @Alexei @Daniel This series depends on a core BPF patch from the Arm
>>                  series [3].

[snip]
> If not yet, perhaps you could ship this series along with your PR to Linus
> during this merge window given the big net PR (incl. bpf) was already merged
> yesterday. So from our side only fixes ship to Linus.

Are you OK with this patch going thru the riscv tree as well?
Palmer Dabbelt Aug. 30, 2023, 8:43 p.m. UTC | #6
On Wed, 30 Aug 2023 06:59:13 PDT (-0700), bjorn@kernel.org wrote:
> Daniel Borkmann <daniel@iogearbox.net> writes:
>
>> On 8/29/23 12:06 PM, Björn Töpel wrote:
>>> Puranjay Mohan <puranjay12@gmail.com> writes:
>>> 
>>>> Changes in v2 -> v3:
>>>> 1. Fix maximum width of code in patches from 80 to 100. [All patches]
>>>> 2. Add checks for ctx->ro_insns == NULL. [Patch 3]
>>>> 3. Fix check for edge condition where amount of text to set > 2 * pagesize
>>>>     [Patch 1 and 2]
>>>> 4. Add reviewed-by in patches.
>>>> 5. Adding results of selftest here:
>>>>     Using the command: ./test_progs on qemu
>>>>     Without the series: Summary: 336/3162 PASSED, 56 SKIPPED, 90 FAILED
>>>>     With this series: Summary: 336/3162 PASSED, 56 SKIPPED, 90 FAILED
>>>>
>>>> Changes in v1 -> v2:
>>>> 1. Implement a new function patch_text_set_nosync() to be used in bpf_arch_text_invalidate().
>>>>     The implementation in v1 called patch_text_nosync() in a loop and it was bad as it would
>>>>     call flush_icache_range() for every word making it really slow. This was found by running
>>>>     the test_tag selftest which would take forever to complete.
>>>>
>>>> Here is some data to prove the V2 fixes the problem:
>>>>
>>>> Without this series:
>>>> root@rv-selftester:~/src/kselftest/bpf# time ./test_tag
>>>> test_tag: OK (40945 tests)
>>>>
>>>> real    7m47.562s
>>>> user    0m24.145s
>>>> sys     6m37.064s
>>>>
>>>> With this series applied:
>>>> root@rv-selftester:~/src/selftest/bpf# time ./test_tag
>>>> test_tag: OK (40945 tests)
>>>>
>>>> real    7m29.472s
>>>> user    0m25.865s
>>>> sys     6m18.401s
>>>>
>>>> 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/20220204185742.271030-1-song@kernel.org/
>>>> [2] https://lore.kernel.org/all/20230626085811.3192402-1-puranjay12@gmail.com/
>>>> [3] https://lore.kernel.org/all/20230626085811.3192402-2-puranjay12@gmail.com/
>>>> [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 (3):
>>>>    riscv: extend patch_text_nosync() for multiple pages
>>>>    riscv: implement a memset like function for text
>>>>    bpf, riscv: use prog pack allocator in the BPF JIT
>>> 
>>> Thank you! For the series:
>>> 
>>> Acked-by: Björn Töpel <bjorn@kernel.org>
>>> Tested-by: Björn Töpel <bjorn@rivosinc.com>
>>> 
>>> @Alexei @Daniel This series depends on a core BPF patch from the Arm
>>>                  series [3].
>
> [snip]
>> If not yet, perhaps you could ship this series along with your PR to Linus
>> during this merge window given the big net PR (incl. bpf) was already merged
>> yesterday. So from our side only fixes ship to Linus.
>
> Are you OK with this patch going thru the riscv tree as well?

I'm generally fine taking almost anything, as long as whomever usually 
takes them acks it.
Daniel Borkmann Aug. 30, 2023, 10:48 p.m. UTC | #7
On 8/30/23 3:54 PM, Palmer Dabbelt wrote:
> On Wed, 30 Aug 2023 01:18:46 PDT (-0700), daniel@iogearbox.net wrote:
>> On 8/29/23 12:06 PM, Björn Töpel wrote:
>>> Puranjay Mohan <puranjay12@gmail.com> writes:
>>>
>>>> Changes in v2 -> v3:
>>>> 1. Fix maximum width of code in patches from 80 to 100. [All patches]
>>>> 2. Add checks for ctx->ro_insns == NULL. [Patch 3]
>>>> 3. Fix check for edge condition where amount of text to set > 2 * pagesize
>>>>     [Patch 1 and 2]
>>>> 4. Add reviewed-by in patches.
>>>> 5. Adding results of selftest here:
>>>>     Using the command: ./test_progs on qemu
>>>>     Without the series: Summary: 336/3162 PASSED, 56 SKIPPED, 90 FAILED
>>>>     With this series: Summary: 336/3162 PASSED, 56 SKIPPED, 90 FAILED
>>>>
>>>> Changes in v1 -> v2:
>>>> 1. Implement a new function patch_text_set_nosync() to be used in bpf_arch_text_invalidate().
>>>>     The implementation in v1 called patch_text_nosync() in a loop and it was bad as it would
>>>>     call flush_icache_range() for every word making it really slow. This was found by running
>>>>     the test_tag selftest which would take forever to complete.
>>>>
>>>> Here is some data to prove the V2 fixes the problem:
>>>>
>>>> Without this series:
>>>> root@rv-selftester:~/src/kselftest/bpf# time ./test_tag
>>>> test_tag: OK (40945 tests)
>>>>
>>>> real    7m47.562s
>>>> user    0m24.145s
>>>> sys     6m37.064s
>>>>
>>>> With this series applied:
>>>> root@rv-selftester:~/src/selftest/bpf# time ./test_tag
>>>> test_tag: OK (40945 tests)
>>>>
>>>> real    7m29.472s
>>>> user    0m25.865s
>>>> sys     6m18.401s
>>>>
>>>> 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/20220204185742.271030-1-song@kernel.org/
>>>> [2] https://lore.kernel.org/all/20230626085811.3192402-1-puranjay12@gmail.com/
>>>> [3] https://lore.kernel.org/all/20230626085811.3192402-2-puranjay12@gmail.com/
>>>> [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 (3):
>>>>    riscv: extend patch_text_nosync() for multiple pages
>>>>    riscv: implement a memset like function for text
>>>>    bpf, riscv: use prog pack allocator in the BPF JIT
>>>
>>> Thank you! For the series:
>>>
>>> Acked-by: Björn Töpel <bjorn@kernel.org>
>>> Tested-by: Björn Töpel <bjorn@rivosinc.com>
>>>
>>> @Alexei @Daniel This series depends on a core BPF patch from the Arm
>>>                  series [3].
>>>
>>> @Palmer LMK if you have any concerns taking the RISC-V text patching
>>>          stuff via the BPF tree.
>>
>> Palmer, did the riscv PR already go to Linus?
> 
> Not yet, I usually send on Friday mornings -- and I also generally send two, as there's some stragglers/fixes for the second week.  I'm fine taking it (Bjorn just poked me), can someone provide a base commit? Bjorn says it depends on something in Linus' tree, so I'll just pick it up as a straggler for next week.

Okay, sgtm.

> Also, do you mind sending an Ack?

Björn / Puranjay, just to clarify since the arm64 series did not land, you are referring
to this one as a dependency [0], right? Meaning, you'd route [0] + this series via riscv
PR to Linus then during this merge win.

If yes, could one of you send the complete 4-patch series with the prior Acks from [0] + this
series collected to both bpf+riscv list (with the small request to extend the commit desc
in [0] a bit to better document implications of the change itself for other JITs)? After a
final look and if BPF CI goes through we can then ack as well and unblock the routing.

Thanks,
Daniel

   [0] https://lore.kernel.org/all/20230626085811.3192402-2-puranjay12@gmail.com/
Puranjay Mohan Aug. 31, 2023, 1:15 p.m. UTC | #8
On Thu, Aug 31, 2023 at 12:48 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 8/30/23 3:54 PM, Palmer Dabbelt wrote:
> > On Wed, 30 Aug 2023 01:18:46 PDT (-0700), daniel@iogearbox.net wrote:
> >> On 8/29/23 12:06 PM, Björn Töpel wrote:
> >>> Puranjay Mohan <puranjay12@gmail.com> writes:
> >>>
> >>>> Changes in v2 -> v3:
> >>>> 1. Fix maximum width of code in patches from 80 to 100. [All patches]
> >>>> 2. Add checks for ctx->ro_insns == NULL. [Patch 3]
> >>>> 3. Fix check for edge condition where amount of text to set > 2 * pagesize
> >>>>     [Patch 1 and 2]
> >>>> 4. Add reviewed-by in patches.
> >>>> 5. Adding results of selftest here:
> >>>>     Using the command: ./test_progs on qemu
> >>>>     Without the series: Summary: 336/3162 PASSED, 56 SKIPPED, 90 FAILED
> >>>>     With this series: Summary: 336/3162 PASSED, 56 SKIPPED, 90 FAILED
> >>>>
> >>>> Changes in v1 -> v2:
> >>>> 1. Implement a new function patch_text_set_nosync() to be used in bpf_arch_text_invalidate().
> >>>>     The implementation in v1 called patch_text_nosync() in a loop and it was bad as it would
> >>>>     call flush_icache_range() for every word making it really slow. This was found by running
> >>>>     the test_tag selftest which would take forever to complete.
> >>>>
> >>>> Here is some data to prove the V2 fixes the problem:
> >>>>
> >>>> Without this series:
> >>>> root@rv-selftester:~/src/kselftest/bpf# time ./test_tag
> >>>> test_tag: OK (40945 tests)
> >>>>
> >>>> real    7m47.562s
> >>>> user    0m24.145s
> >>>> sys     6m37.064s
> >>>>
> >>>> With this series applied:
> >>>> root@rv-selftester:~/src/selftest/bpf# time ./test_tag
> >>>> test_tag: OK (40945 tests)
> >>>>
> >>>> real    7m29.472s
> >>>> user    0m25.865s
> >>>> sys     6m18.401s
> >>>>
> >>>> 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/20220204185742.271030-1-song@kernel.org/
> >>>> [2] https://lore.kernel.org/all/20230626085811.3192402-1-puranjay12@gmail.com/
> >>>> [3] https://lore.kernel.org/all/20230626085811.3192402-2-puranjay12@gmail.com/
> >>>> [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 (3):
> >>>>    riscv: extend patch_text_nosync() for multiple pages
> >>>>    riscv: implement a memset like function for text
> >>>>    bpf, riscv: use prog pack allocator in the BPF JIT
> >>>
> >>> Thank you! For the series:
> >>>
> >>> Acked-by: Björn Töpel <bjorn@kernel.org>
> >>> Tested-by: Björn Töpel <bjorn@rivosinc.com>
> >>>
> >>> @Alexei @Daniel This series depends on a core BPF patch from the Arm
> >>>                  series [3].
> >>>
> >>> @Palmer LMK if you have any concerns taking the RISC-V text patching
> >>>          stuff via the BPF tree.
> >>
> >> Palmer, did the riscv PR already go to Linus?
> >
> > Not yet, I usually send on Friday mornings -- and I also generally send two, as there's some stragglers/fixes for the second week.  I'm fine taking it (Bjorn just poked me), can someone provide a base commit? Bjorn says it depends on something in Linus' tree, so I'll just pick it up as a straggler for next week.
>
> Okay, sgtm.
>
> > Also, do you mind sending an Ack?
>
> Björn / Puranjay, just to clarify since the arm64 series did not land, you are referring
> to this one as a dependency [0], right? Meaning, you'd route [0] + this series via riscv
> PR to Linus then during this merge win.
>
> If yes, could one of you send the complete 4-patch series with the prior Acks from [0] + this
> series collected to both bpf+riscv list (with the small request to extend the commit desc
> in [0] a bit to better document implications of the change itself for other JITs)? After a
> final look and if BPF CI goes through we can then ack as well and unblock the routing.
>
> Thanks,
> Daniel
>
>    [0] https://lore.kernel.org/all/20230626085811.3192402-2-puranjay12@gmail.com/

Hi Daniel,

I have sent the v4[0] of this with the core patch included.

[0]https://lore.kernel.org/all/20230831131229.497941-1-puranjay12@gmail.com/

Thanks.
Puranjay