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

From: Puranjay Mohan <puranjay12@gmail.com>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	Florent Revest <revest@chromium.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	Martin KaFai Lau <martin.lau@linux.dev>,
	Song Liu <song@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	bpf <bpf@vger.kernel.org>, KP Singh <kpsingh@kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH bpf-next v4 0/3] bpf, arm64: use BPF prog pack allocator in BPF JIT
Date: Fri, 4 Aug 2023 17:11:25 +0200	[thread overview]
Message-ID: <CANk7y0hQcuabELOH-QHiqNAJhuCZYeWim7AJ125zS7_GnKwcGQ@mail.gmail.com> (raw)
In-Reply-To: <ZMuLvKRbPfOK0IpN@FVFF77S0Q05N>

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