All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alexei Starovoitov" <alexei.starovoitov@gmail.com>
To: "Emil Tsalapatis" <emil@etsalapatis.com>
Cc: <bpf@vger.kernel.org>, <ast@kernel.org>, <andrii@kernel.org>,
	<memxor@gmail.com>, <daniel@iogearbox.net>, <eddyz87@gmail.com>
Subject: Re: [PATCH 2/3] bpf: Add sleepable arena page allocation path
Date: Wed, 23 Sep 2026 23:24:29 +0000	[thread overview]
Message-ID: <DLN30MYFP3U2.1BH9I50MEBUEX@gmail.com> (raw)
In-Reply-To: <CABFh=a4H9_3pkFzgyrooz8eg=8pa-f_JHK3XdLSvTfV_nVN8JA@mail.gmail.com>

On Wed Sep 23, 2026 at 9:48 PM UTC, Emil Tsalapatis wrote:
> On Wed, Aug 26, 2026 at 1:25 AM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
>>
>> On Mon Aug 24, 2026 at 1:25 AM PDT, Emil Tsalapatis wrote:
>> > The bpf_arena_alloc_pages() function currently only allocates pages
>> > inside a spinlock critical section with IRQs off. This forces the use
>> > of alloc_pages_nolock() in the BPF allocator, even when the caller is
>> > a sleepable BPF function. This in turn causes allocation failures even
>> > in cases where falling into the allocator slow path and possibly
>> > sleeping would eventually succeed. This can be triggered consistently
>> > by heavy BPF arena users like scx.
>> >
>> > Add a separate arena page allocation path just for sleepable callers.
>> > The path preallocates the arena memory to be added to the tree before
>> > taking the critical section.
>> >
>> > Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
>> > ---
>> >  include/linux/bpf.h  |  6 ++++
>> >  kernel/bpf/arena.c   | 76 +++++++++++++++++++++++++++++++++++++++++---
>> >  kernel/bpf/syscall.c |  8 +----
>> >  3 files changed, 79 insertions(+), 11 deletions(-)
>> >
>> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> > index ffa5626411ac..d15ed7a3879b 100644
>> > --- a/include/linux/bpf.h
>> > +++ b/include/linux/bpf.h
>> > @@ -710,6 +710,12 @@ void bpf_map_free_internal_structs(struct bpf_map *map, void *obj);
>> >  int bpf_dynptr_from_file_sleepable(struct file *file, u32 flags,
>> >                                  struct bpf_dynptr *ptr__uninit);
>> >
>> > +static inline bool is_bpf_alloc_nonsleepable(void)
>> > +{
>> > +     return preempt_count() > 0 || irqs_disabled() ||
>> > +             IS_ENABLED(CONFIG_PREEMPT_RT);
>> > +}
>> > +
>> >  #if defined(CONFIG_MMU) && defined(CONFIG_64BIT)
>> >  void *bpf_arena_alloc_pages_non_sleepable(void *p__map, void *addr__ign, u32 page_cnt, int node_id,
>> >                                         u64 flags);
>> > diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
>> > index da356989786a..126ddae763f9 100644
>> > --- a/kernel/bpf/arena.c
>> > +++ b/kernel/bpf/arena.c
>> > @@ -683,8 +683,73 @@ static int arena_adjust_tree(struct bpf_arena *arena, long uaddr, long page_cnt,
>> >       return range_tree_clear(&arena->rt, *pgoff, page_cnt);
>> >  }
>> >
>> > -static long arena_alloc_pages_internal(struct bpf_arena *arena, long page_cnt,
>> > -             long uaddr, long pgoff, int node_id, bool sleepable)
>> > +static long arena_alloc_pages_sleepable(struct bpf_arena *arena, long page_cnt,
>> > +             long uaddr, long pgoff, int node_id)
>> > +{
>> > +     u64 kern_vm_start = bpf_arena_get_kern_vm_start(arena);
>> > +     struct apply_range_data data;
>> > +     struct page **pages = NULL;
>> > +     unsigned long flags;
>> > +     u32 uaddr32;
>> > +     int ret, i;
>> > +
>> > +     pages = kvcalloc(page_cnt, sizeof(struct page *), GFP_KERNEL_ACCOUNT);
>> > +     if (!pages)
>> > +             return 0;
>> > +
>> > +     ret = bpf_map_alloc_pages(&arena->map, node_id, page_cnt, pages);
>> > +     if (ret) {
>> > +             kvfree(pages);
>> > +             return 0;
>> > +     }
>> > +
>> > +     data.i = 0;
>> > +     data.pages = pages;
>> > +     data.arena = arena;
>> > +
>>
>> Why have this split?
>> Always use an approach of allocating pages outside of the lock?
>>
>
> Mainly to avoid multiple kmalloc_nolock() allocations outside the lock
> for the sleepable
> path. That should matter for larger allocations. The two separate
> paths also stay simpler
> than if we had a single path (at the cost of a bit of extra
> duplication). I can change to a single
> code path during respin if you feel these are not worth having two
> internal calls.

Hmm. I would think one way to do things is easier. Less branches to worry about.
Give it a shot? If it looks worse, then keep it as-is.

  reply	other threads:[~2026-09-23 23:24 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-24  8:25 [PATCH 0/3] bpf: Make sleepable arena paths use sleepable alloc_pages Emil Tsalapatis
2026-08-24  8:25 ` [PATCH 1/3] bpf: Factor out nonsleepable arena allocation logic Emil Tsalapatis
2026-08-24  9:24   ` bot+bpf-ci
2026-08-24  8:25 ` [PATCH 2/3] bpf: Add sleepable arena page allocation path Emil Tsalapatis
2026-08-24  8:43   ` sashiko-bot
2026-08-24  9:42   ` bot+bpf-ci
2026-08-26  1:25   ` Alexei Starovoitov
2026-09-23 21:48     ` Emil Tsalapatis
2026-09-23 23:24       ` Alexei Starovoitov [this message]
2026-08-24  8:25 ` [PATCH 3/3] selftests/bpf: Test large allocations for both sleepable/nonsleepable arena users Emil Tsalapatis
2026-08-24  8:39   ` sashiko-bot
2026-08-24  9:24   ` bot+bpf-ci

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DLN30MYFP3U2.1BH9I50MEBUEX@gmail.com \
    --to=alexei.starovoitov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=emil@etsalapatis.com \
    --cc=memxor@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.