Re: [PATCH bpf-next v6 1/4] bpf: Add a sleepable page allocator for map memory

[bot+bpf-ci]

> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index b3cd28d9e3b5..c817c99d2906 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -2784,6 +2784,7 @@ struct bpf_prog *bpf_prog_get_curr_or_next(u32 *id);
>
>  int bpf_map_alloc_pages(const struct bpf_map *map, int nid,
>  			unsigned long nr_pages, struct page **page_array);
> +struct page *bpf_map_alloc_page_sleepable(const struct bpf_map *map);
>  #ifdef CONFIG_MEMCG
>  void bpf_map_memcg_enter(const struct bpf_map *map, struct mem_cgroup **old_memcg,
>  			 struct mem_cgroup **new_memcg);

[ ... ]

> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 6874ba1424af..f9b81638e537 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c

[ ... ]

> @@ -636,6 +635,20 @@ int bpf_map_alloc_pages(const struct bpf_map *map, int nid,
>  	return ret;
>  }
>
> +/*
> + * For callers that know they run in a sleepable context, e.g. a user page
> + * fault handler. can_alloc_pages() is a conservative guess made for BPF
> + * program context - notably it is always false on PREEMPT_RT - so going
> + * through bpf_map_alloc_pages() there would needlessly pick the
> + * non-blocking allocator, which never reclaims and never engages the OOM
> + * machinery.
> + */
> +struct page *bpf_map_alloc_page_sleepable(const struct bpf_map *map)
> +{
> +	might_sleep();
> +	return alloc_pages_node(map->numa_node, BPF_PAGE_GFP, 0);
> +}

In v4 review, Michal Hocko asked how bpf_map_alloc_page_sleepable() achieves
safety from mm reentrancy, noting that sleepable context alone doesn't
guarantee safety. Kumar Kartikeya Dwivedi replied that it's only for the
arena fault handler in task context. Michal's follow-up was:

  "So you rely on callers to know what they are doing. If that is the case
   and generally acceptable by the BPF community (no real saying from me in
   that matter) then make sure all that is properly documented. Because
   sleepable context is not merely enough."

https://lore.kernel.org/bpf/aoxNTXWCCndiuewO@tiehlicka

Should the comment explain the reentrancy safety expectations? The current
documentation emphasizes PREEMPT_RT and the sleepable context, but doesn't
clarify when it's safe to call this from an mm reentrancy perspective, or
that the caller must ensure it's not in a context where the allocator might
reenter mm code that's already on the stack.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32835197153