mbox series

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

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

Message

Puranjay Mohan June 26, 2023, 8:58 a.m. UTC
BPF programs currently consume a page each on ARM64. 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.

This patch series enables the BPF prog pack allocator for the ARM64 BPF JIT.

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

To test the performance of the BPF prog pack allocator on ARM64, a stresser
tool[2] 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.

In the above environment we try to build Python-3.8.4 and try to find different
iTLB metrics for the compilation done by gcc-12.2.0.

The source code[3] is  configured with the following command:
./configure --enable-optimizations --with-ensurepip=install

Then the runner script is executed with the following command:
./run.sh "perf stat -e ITLB_WALK,L1I_TLB,INST_RETIRED,iTLB-load-misses -a make -j32"

This builds Python while 160 BPF programs are loaded and 100 are being constantly
triggered and measures iTLB related metrics.

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-aarch64 with 32 cpus, 4G memory, -machine virt,
-cpu host, and -enable-kvm.

Results
-------

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

Performance counter stats for 'system wide':

         333278635      ITLB_WALK
     6762692976558      L1I_TLB
    25359571423901      INST_RETIRED
       15824054789      iTLB-load-misses

     189.029769053 seconds time elapsed

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

Performance counter stats for 'system wide':

         190333544      ITLB_WALK
     6712712386528      L1I_TLB
    25278233304411      INST_RETIRED
        5716757866      iTLB-load-misses

     185.392650561 seconds time elapsed

Improvements in metrics
-----------------------

Compilation time                             ---> 1.92% faster
iTLB-load-misses/Sec (Less is better)        ---> 63.16% decrease
ITLB_WALK/1000 INST_RETIRED (Less is better) ---> 42.71% decrease
ITLB_Walk/L1I_TLB (Less is better)           ---> 42.47% decrease

[1] https://lore.kernel.org/bpf/20220204185742.271030-1-song@kernel.org/
[2] https://github.com/puranjaymohan/BPF-Allocator-Bench
[3] https://www.python.org/ftp/python/3.8.4/Python-3.8.4.tgz

Chanes in V3 => V4: Changes only in 3rd patch
1. Fix the I-cache maintenance: Clean the data cache and invalidate the i-Cache
   only *after* the instructions have been copied to the ROX region.

Chanes in V2 => V3: Changes only in 3rd patch
1. Set prog = orig_prog; in the failure path of bpf_jit_binary_pack_finalize()
call.
2. Add comments explaining the usage of the offsets in the exception table.

Changes in v1 => v2:
1. Make the naming consistent in the 3rd patch:
   ro_image and image
   ro_header and header
   ro_image_ptr and image_ptr
2. Use names dst/src in place of addr/opcode in second patch.
3. Add Acked-by: Song Liu <song@kernel.org> in 1st and 2nd patch.

Puranjay Mohan (3):
  bpf: make bpf_prog_pack allocator portable
  arm64: patching: Add aarch64_insn_copy()
  bpf, arm64: use bpf_jit_binary_pack_alloc

 arch/arm64/include/asm/patching.h |   1 +
 arch/arm64/kernel/patching.c      |  39 ++++++++
 arch/arm64/net/bpf_jit_comp.c     | 145 +++++++++++++++++++++++++-----
 kernel/bpf/core.c                 |   8 +-
 4 files changed, 165 insertions(+), 28 deletions(-)

Comments

Florent Revest June 30, 2023, 5:20 p.m. UTC | #1
On Mon, Jun 26, 2023 at 10:58 AM Puranjay Mohan <puranjay12@gmail.com> wrote:
>
> BPF programs currently consume a page each on ARM64. 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.
>
> This patch series enables the BPF prog pack allocator for the ARM64 BPF JIT.
>
> ====================================================
> Performance Analysis of prog pack allocator on ARM64
> ====================================================
>
> To test the performance of the BPF prog pack allocator on ARM64, a stresser
> tool[2] 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.
>
> In the above environment we try to build Python-3.8.4 and try to find different
> iTLB metrics for the compilation done by gcc-12.2.0.
>
> The source code[3] is  configured with the following command:
> ./configure --enable-optimizations --with-ensurepip=install
>
> Then the runner script is executed with the following command:
> ./run.sh "perf stat -e ITLB_WALK,L1I_TLB,INST_RETIRED,iTLB-load-misses -a make -j32"
>
> This builds Python while 160 BPF programs are loaded and 100 are being constantly
> triggered and measures iTLB related metrics.
>
> 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-aarch64 with 32 cpus, 4G memory, -machine virt,
> -cpu host, and -enable-kvm.
>
> Results
> -------
>
> Before enabling prog pack allocator:
> ------------------------------------
>
> Performance counter stats for 'system wide':
>
>          333278635      ITLB_WALK
>      6762692976558      L1I_TLB
>     25359571423901      INST_RETIRED
>        15824054789      iTLB-load-misses
>
>      189.029769053 seconds time elapsed
>
> After enabling prog pack allocator:
> -----------------------------------
>
> Performance counter stats for 'system wide':
>
>          190333544      ITLB_WALK
>      6712712386528      L1I_TLB
>     25278233304411      INST_RETIRED
>         5716757866      iTLB-load-misses
>
>      185.392650561 seconds time elapsed
>
> Improvements in metrics
> -----------------------
>
> Compilation time                             ---> 1.92% faster
> iTLB-load-misses/Sec (Less is better)        ---> 63.16% decrease
> ITLB_WALK/1000 INST_RETIRED (Less is better) ---> 42.71% decrease
> ITLB_Walk/L1I_TLB (Less is better)           ---> 42.47% decrease
>
> [1] https://lore.kernel.org/bpf/20220204185742.271030-1-song@kernel.org/
> [2] https://github.com/puranjaymohan/BPF-Allocator-Bench
> [3] https://www.python.org/ftp/python/3.8.4/Python-3.8.4.tgz
>
> Chanes in V3 => V4: Changes only in 3rd patch
> 1. Fix the I-cache maintenance: Clean the data cache and invalidate the i-Cache
>    only *after* the instructions have been copied to the ROX region.
>
> Chanes in V2 => V3: Changes only in 3rd patch
> 1. Set prog = orig_prog; in the failure path of bpf_jit_binary_pack_finalize()
> call.
> 2. Add comments explaining the usage of the offsets in the exception table.
>
> Changes in v1 => v2:
> 1. Make the naming consistent in the 3rd patch:
>    ro_image and image
>    ro_header and header
>    ro_image_ptr and image_ptr
> 2. Use names dst/src in place of addr/opcode in second patch.
> 3. Add Acked-by: Song Liu <song@kernel.org> in 1st and 2nd patch.
>
> Puranjay Mohan (3):
>   bpf: make bpf_prog_pack allocator portable
>   arm64: patching: Add aarch64_insn_copy()
>   bpf, arm64: use bpf_jit_binary_pack_alloc
>
>  arch/arm64/include/asm/patching.h |   1 +
>  arch/arm64/kernel/patching.c      |  39 ++++++++
>  arch/arm64/net/bpf_jit_comp.c     | 145 +++++++++++++++++++++++++-----
>  kernel/bpf/core.c                 |   8 +-
>  4 files changed, 165 insertions(+), 28 deletions(-)
>
> --
> 2.40.1
>
>

FWIW

Acked-by: Florent Revest <revest@chromium.org>

Thanks for this Puranjay!
Daniel Borkmann July 3, 2023, 4:40 p.m. UTC | #2
Hi Mark,

On 6/26/23 10:58 AM, Puranjay Mohan wrote:
> BPF programs currently consume a page each on ARM64. 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.
> 
> This patch series enables the BPF prog pack allocator for the ARM64 BPF JIT.
> 
> ====================================================
> Performance Analysis of prog pack allocator on ARM64
> ====================================================
> 
> To test the performance of the BPF prog pack allocator on ARM64, a stresser
> tool[2] 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.
> 
> In the above environment we try to build Python-3.8.4 and try to find different
> iTLB metrics for the compilation done by gcc-12.2.0.
> 
> The source code[3] is  configured with the following command:
> ./configure --enable-optimizations --with-ensurepip=install
> 
> Then the runner script is executed with the following command:
> ./run.sh "perf stat -e ITLB_WALK,L1I_TLB,INST_RETIRED,iTLB-load-misses -a make -j32"
> 
> This builds Python while 160 BPF programs are loaded and 100 are being constantly
> triggered and measures iTLB related metrics.
> 
> 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-aarch64 with 32 cpus, 4G memory, -machine virt,
> -cpu host, and -enable-kvm.
> 
> Results
> -------
> 
> Before enabling prog pack allocator:
> ------------------------------------
> 
> Performance counter stats for 'system wide':
> 
>           333278635      ITLB_WALK
>       6762692976558      L1I_TLB
>      25359571423901      INST_RETIRED
>         15824054789      iTLB-load-misses
> 
>       189.029769053 seconds time elapsed
> 
> After enabling prog pack allocator:
> -----------------------------------
> 
> Performance counter stats for 'system wide':
> 
>           190333544      ITLB_WALK
>       6712712386528      L1I_TLB
>      25278233304411      INST_RETIRED
>          5716757866      iTLB-load-misses
> 
>       185.392650561 seconds time elapsed
> 
> Improvements in metrics
> -----------------------
> 
> Compilation time                             ---> 1.92% faster
> iTLB-load-misses/Sec (Less is better)        ---> 63.16% decrease
> ITLB_WALK/1000 INST_RETIRED (Less is better) ---> 42.71% decrease
> ITLB_Walk/L1I_TLB (Less is better)           ---> 42.47% decrease
> 
> [1] https://lore.kernel.org/bpf/20220204185742.271030-1-song@kernel.org/
> [2] https://github.com/puranjaymohan/BPF-Allocator-Bench
> [3] https://www.python.org/ftp/python/3.8.4/Python-3.8.4.tgz
> 
> Chanes in V3 => V4: Changes only in 3rd patch
> 1. Fix the I-cache maintenance: Clean the data cache and invalidate the i-Cache
>     only *after* the instructions have been copied to the ROX region.

If you get a chance to take another look at the v4 changes from Puranjay and
in case they look good to you reply with an Ack, that would be great.

Thanks,
Daniel
Mark Rutland July 3, 2023, 5:15 p.m. UTC | #3
On Mon, Jul 03, 2023 at 06:40:21PM +0200, Daniel Borkmann wrote:
> Hi Mark,

Hi Daniel,

> On 6/26/23 10:58 AM, Puranjay Mohan wrote:
> > BPF programs currently consume a page each on ARM64. 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.
> > 
> > This patch series enables the BPF prog pack allocator for the ARM64 BPF JIT.

> If you get a chance to take another look at the v4 changes from Puranjay and
> in case they look good to you reply with an Ack, that would be great.

Sure -- this is on my queue of things to look at; it might just take me a few
days to get the time to give this a proper look.

Thanks,
Mark.
Daniel Borkmann July 3, 2023, 5:54 p.m. UTC | #4
On 7/3/23 7:15 PM, Mark Rutland wrote:
[...]
>> On 6/26/23 10:58 AM, Puranjay Mohan wrote:
>>> BPF programs currently consume a page each on ARM64. 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.
>>>
>>> This patch series enables the BPF prog pack allocator for the ARM64 BPF JIT.
> 
>> If you get a chance to take another look at the v4 changes from Puranjay and
>> in case they look good to you reply with an Ack, that would be great.
> 
> Sure -- this is on my queue of things to look at; it might just take me a few
> days to get the time to give this a proper look.

Awesome, thanks Mark!
Puranjay Mohan July 17, 2023, 7:50 a.m. UTC | #5
Hi Mark,

On Mon, Jul 3, 2023 at 7:15 PM Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Mon, Jul 03, 2023 at 06:40:21PM +0200, Daniel Borkmann wrote:
> > Hi Mark,
>
> Hi Daniel,
>
> > On 6/26/23 10:58 AM, Puranjay Mohan wrote:
> > > BPF programs currently consume a page each on ARM64. 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.
> > >
> > > This patch series enables the BPF prog pack allocator for the ARM64 BPF JIT.
>
> > If you get a chance to take another look at the v4 changes from Puranjay and
> > in case they look good to you reply with an Ack, that would be great.
>
> Sure -- this is on my queue of things to look at; it might just take me a few
> days to get the time to give this a proper look.
>
> Thanks,
> Mark.

I am eagerly looking forward to your feedback on this series.

Thanks,
Puranjay
Puranjay Mohan July 30, 2023, 5:22 p.m. UTC | #6
Hi Mark,
I am really looking forward to your feedback on this series.

On Mon, Jul 17, 2023 at 9:50 AM Puranjay Mohan <puranjay12@gmail.com> wrote:
>
> Hi Mark,
>
> On Mon, Jul 3, 2023 at 7:15 PM Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > On Mon, Jul 03, 2023 at 06:40:21PM +0200, Daniel Borkmann wrote:
> > > Hi Mark,
> >
> > Hi Daniel,
> >
> > > On 6/26/23 10:58 AM, Puranjay Mohan wrote:
> > > > BPF programs currently consume a page each on ARM64. 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.
> > > >
> > > > This patch series enables the BPF prog pack allocator for the ARM64 BPF JIT.
> >
> > > If you get a chance to take another look at the v4 changes from Puranjay and
> > > in case they look good to you reply with an Ack, that would be great.
> >
> > Sure -- this is on my queue of things to look at; it might just take me a few
> > days to get the time to give this a proper look.
> >
> > Thanks,
> > Mark.
>
> I am eagerly looking forward to your feedback on this series.
>
> Thanks,
> Puranjay


Thanks,
Puranjay
Alexei Starovoitov Aug. 2, 2023, 9:02 p.m. UTC | #7
On Sun, Jul 30, 2023 at 10:22 AM Puranjay Mohan <puranjay12@gmail.com> wrote:
>
> Hi Mark,
> I am really looking forward to your feedback on this series.
>
> On Mon, Jul 17, 2023 at 9:50 AM Puranjay Mohan <puranjay12@gmail.com> wrote:
> >
> > Hi Mark,
> >
> > On Mon, Jul 3, 2023 at 7:15 PM Mark Rutland <mark.rutland@arm.com> wrote:
> > >
> > > On Mon, Jul 03, 2023 at 06:40:21PM +0200, Daniel Borkmann wrote:
> > > > Hi Mark,
> > >
> > > Hi Daniel,
> > >
> > > > On 6/26/23 10:58 AM, Puranjay Mohan wrote:
> > > > > BPF programs currently consume a page each on ARM64. 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.
> > > > >
> > > > > This patch series enables the BPF prog pack allocator for the ARM64 BPF JIT.
> > >
> > > > If you get a chance to take another look at the v4 changes from Puranjay and
> > > > in case they look good to you reply with an Ack, that would be great.
> > >
> > > Sure -- this is on my queue of things to look at; it might just take me a few
> > > days to get the time to give this a proper look.
> > >
> > > Thanks,
> > > Mark.
> >
> > I am eagerly looking forward to your feedback on this series.

Mark, Catalin, Florent, KP,

This patch set was submitted on June 26 !
It's not acceptable to delay review for so long.
Please review asap.
Mark Rutland Aug. 3, 2023, 11:13 a.m. UTC | #8
Hi Alexei,

On Wed, Aug 02, 2023 at 02:02:39PM -0700, Alexei Starovoitov wrote:
> On Sun, Jul 30, 2023 at 10:22 AM Puranjay Mohan <puranjay12@gmail.com> wrote:
> >
> > Hi Mark,
> > I am really looking forward to your feedback on this series.
> >
> > On Mon, Jul 17, 2023 at 9:50 AM Puranjay Mohan <puranjay12@gmail.com> wrote:
> > >
> > > Hi Mark,
> > >
> > > On Mon, Jul 3, 2023 at 7:15 PM Mark Rutland <mark.rutland@arm.com> wrote:
> > > >
> > > > On Mon, Jul 03, 2023 at 06:40:21PM +0200, Daniel Borkmann wrote:
> > > > > Hi Mark,
> > > >
> > > > Hi Daniel,
> > > >
> > > > > On 6/26/23 10:58 AM, Puranjay Mohan wrote:
> > > > > > BPF programs currently consume a page each on ARM64. 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.
> > > > > >
> > > > > > This patch series enables the BPF prog pack allocator for the ARM64 BPF JIT.
> > > >
> > > > > If you get a chance to take another look at the v4 changes from Puranjay and
> > > > > in case they look good to you reply with an Ack, that would be great.
> > > >
> > > > Sure -- this is on my queue of things to look at; it might just take me a few
> > > > days to get the time to give this a proper look.
> > > >
> > > > Thanks,
> > > > Mark.
> > >
> > > I am eagerly looking forward to your feedback on this series.
> 
> Mark, Catalin, Florent, KP,
> 
> This patch set was submitted on June 26 !

I appreciate this was sent a while ago, but I have been stuck on some urgent
bug-fixing for the last few weeks, and my review bandwidth is therfore very
limited.

Given Puranjay had previously told me he was doing this as a side project for
fun, and given no-one had told me this was urgent, I assumed that this wasn't a
major blocker and could wait.

I should have sent a holding reply to that effect; sorry.

The series addresses my original concern. However, in looking at it I think
there may me a wider potential isssue w.r.t. the way instruction memory gets
reused, because as writtten today the architecture doesn't seem to have a
guarantee on when instruction fetches are completed and therefore when it's
safe to modify instruction memory. Usually we're saved by TLB maintenance,
which this series avoids by design.

I unfortunately haven't had the time to dig into that, poke our architects,
etc.

So how urgent is this?

Thanks,
Mark.
Alexei Starovoitov Aug. 3, 2023, 4:15 p.m. UTC | #9
On Thu, Aug 3, 2023 at 4:13 AM Mark Rutland <mark.rutland@arm.com> wrote:
>
> Hi Alexei,
>
> On Wed, Aug 02, 2023 at 02:02:39PM -0700, Alexei Starovoitov wrote:
> > On Sun, Jul 30, 2023 at 10:22 AM Puranjay Mohan <puranjay12@gmail.com> wrote:
> > >
> > > Hi Mark,
> > > I am really looking forward to your feedback on this series.
> > >
> > > On Mon, Jul 17, 2023 at 9:50 AM Puranjay Mohan <puranjay12@gmail.com> wrote:
> > > >
> > > > Hi Mark,
> > > >
> > > > On Mon, Jul 3, 2023 at 7:15 PM Mark Rutland <mark.rutland@arm.com> wrote:
> > > > >
> > > > > On Mon, Jul 03, 2023 at 06:40:21PM +0200, Daniel Borkmann wrote:
> > > > > > Hi Mark,
> > > > >
> > > > > Hi Daniel,
> > > > >
> > > > > > On 6/26/23 10:58 AM, Puranjay Mohan wrote:
> > > > > > > BPF programs currently consume a page each on ARM64. 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.
> > > > > > >
> > > > > > > This patch series enables the BPF prog pack allocator for the ARM64 BPF JIT.
> > > > >
> > > > > > If you get a chance to take another look at the v4 changes from Puranjay and
> > > > > > in case they look good to you reply with an Ack, that would be great.
> > > > >
> > > > > Sure -- this is on my queue of things to look at; it might just take me a few
> > > > > days to get the time to give this a proper look.
> > > > >
> > > > > Thanks,
> > > > > Mark.
> > > >
> > > > I am eagerly looking forward to your feedback on this series.
> >
> > Mark, Catalin, Florent, KP,
> >
> > This patch set was submitted on June 26 !
>
> I appreciate this was sent a while ago, but I have been stuck on some urgent
> bug-fixing for the last few weeks, and my review bandwidth is therfore very
> limited.
>
> Given Puranjay had previously told me he was doing this as a side project for
> fun, and given no-one had told me this was urgent, I assumed that this wasn't a
> major blocker and could wait.
>
> I should have sent a holding reply to that effect; sorry.
>
> The series addresses my original concern. However, in looking at it I think
> there may me a wider potential isssue w.r.t. the way instruction memory gets
> reused, because as writtten today the architecture doesn't seem to have a
> guarantee on when instruction fetches are completed and therefore when it's
> safe to modify instruction memory. Usually we're saved by TLB maintenance,
> which this series avoids by design.
>
> I unfortunately haven't had the time to dig into that, poke our architects,
> etc.
>
> So how urgent is this?

The performance wins are substantial.
We'd like to realize them sooner than later.
Florent Revest Aug. 3, 2023, 4:32 p.m. UTC | #10
On Thu, Aug 3, 2023 at 6:16 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Thu, Aug 3, 2023 at 4:13 AM Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > Hi Alexei,
> >
> > On Wed, Aug 02, 2023 at 02:02:39PM -0700, Alexei Starovoitov wrote:
> > > Mark, Catalin, Florent, KP,

Maybe you've missed my Acked-by for the series Alexei ?
https://lore.kernel.org/all/CABRcYmLAzhG=o2wcBNBtFP34Aj3+eYsEMtMREDT7SqNzBc9-qw@mail.gmail.com/

> > > This patch set was submitted on June 26 !
> >
> > I appreciate this was sent a while ago, but I have been stuck on some urgent
> > bug-fixing for the last few weeks, and my review bandwidth is therfore very
> > limited.
> >
> > Given Puranjay had previously told me he was doing this as a side project for
> > fun, and given no-one had told me this was urgent, I assumed that this wasn't a
> > major blocker and could wait.
> >
> > I should have sent a holding reply to that effect; sorry.
> >
> > The series addresses my original concern. However, in looking at it I think
> > there may me a wider potential isssue w.r.t. the way instruction memory gets
> > reused, because as writtten today the architecture doesn't seem to have a
> > guarantee on when instruction fetches are completed and therefore when it's
> > safe to modify instruction memory. Usually we're saved by TLB maintenance,
> > which this series avoids by design.

But I must say that this sits firmly outside of my knowledge of the
arm architectural details and I would totally miss this sort of nuance
so this is best handled by arm64 maintainers :)

> > I unfortunately haven't had the time to dig into that, poke our architects,
> > etc.
> >
> > So how urgent is this?
>
> The performance wins are substantial.
> We'd like to realize them sooner than later.

I've worked with Mark before, I know for a fact that he is dragged in
all directions. Until we figure out a way to clone him we should try
to not burn him out too often... :)
Puranjay Mohan Aug. 4, 2023, 3:11 p.m. UTC | #11
Hi Mark,

On Thu, Aug 3, 2023 at 1:13 PM Mark Rutland <mark.rutland@arm.com> wrote:
>
> Hi Alexei,
>
> On Wed, Aug 02, 2023 at 02:02:39PM -0700, Alexei Starovoitov wrote:
> > On Sun, Jul 30, 2023 at 10:22 AM Puranjay Mohan <puranjay12@gmail.com> wrote:
> > >
> > > Hi Mark,
> > > I am really looking forward to your feedback on this series.
> > >
> > > On Mon, Jul 17, 2023 at 9:50 AM Puranjay Mohan <puranjay12@gmail.com> wrote:
> > > >
> > > > Hi Mark,
> > > >
> > > > On Mon, Jul 3, 2023 at 7:15 PM Mark Rutland <mark.rutland@arm.com> wrote:
> > > > >
> > > > > On Mon, Jul 03, 2023 at 06:40:21PM +0200, Daniel Borkmann wrote:
> > > > > > Hi Mark,
> > > > >
> > > > > Hi Daniel,
> > > > >
> > > > > > On 6/26/23 10:58 AM, Puranjay Mohan wrote:
> > > > > > > BPF programs currently consume a page each on ARM64. 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.
> > > > > > >
> > > > > > > This patch series enables the BPF prog pack allocator for the ARM64 BPF JIT.
> > > > >
> > > > > > If you get a chance to take another look at the v4 changes from Puranjay and
> > > > > > in case they look good to you reply with an Ack, that would be great.
> > > > >
> > > > > Sure -- this is on my queue of things to look at; it might just take me a few
> > > > > days to get the time to give this a proper look.
> > > > >
> > > > > Thanks,
> > > > > Mark.
> > > >
> > > > I am eagerly looking forward to your feedback on this series.
> >
> > Mark, Catalin, Florent, KP,
> >
> > This patch set was submitted on June 26 !
>
> I appreciate this was sent a while ago, but I have been stuck on some urgent
> bug-fixing for the last few weeks, and my review bandwidth is therfore very
> limited.
>
> Given Puranjay had previously told me he was doing this as a side project for
> fun, and given no-one had told me this was urgent, I assumed that this wasn't a
> major blocker and could wait.

Yes, I am just doing it as a side project for fun. It is not a major blocker.

>
> I should have sent a holding reply to that effect; sorry.
>
> The series addresses my original concern. However, in looking at it I think
> there may me a wider potential isssue w.r.t. the way instruction memory gets
> reused, because as writtten today the architecture doesn't seem to have a
> guarantee on when instruction fetches are completed and therefore when it's
> safe to modify instruction memory. Usually we're saved by TLB maintenance,
> which this series avoids by design.
>
> I unfortunately haven't had the time to dig into that, poke our architects,
> etc.
>
> So how urgent is this?

This is not urgent as this is not a blocker for anything.

I just wanted to know if there was something pending from my side.

Please review it whenever you have spare time. Thanks for helping me debug the
issue with the cache maintenance.

Thanks,
Puranjay
Mark Rutland Nov. 2, 2023, 3:59 p.m. UTC | #12
On Thu, Aug 03, 2023 at 12:13:00PM +0100, Mark Rutland wrote:
[...]

> However, in looking at it I think
> there may me a wider potential isssue w.r.t. the way instruction memory gets
> reused, because as writtten today the architecture doesn't seem to have a
> guarantee on when instruction fetches are completed and therefore when it's
> safe to modify instruction memory. Usually we're saved by TLB maintenance,
> which this series avoids by design.

Just to confirm on this point specifically, per discussions with our
architects, the (architectural) execution of an instruction ensures that there
are no outstanding fetches for prior instructions. IIUC that will be clarified 
the next release of the ARM ARM.

So as long as we're certain all threads have left the old code (e.g. via a
flag, RCU tasks rude synchronization, whatever) before we overwrite slots in
the shared buffer, we should be good.

We will need to be very careful with the maintenance when installing new code
into the shared buffer (e.g. we will require an IPI to all other CPUs), but
that should be relatively simple.

I'll go review the latest patches with that in mind.

Thanks,
Mark.