Re: [PATCH bpf-next 1/3] bpf: Add a sleepable page allocator for map memory
[email protected] Mon, 27 Jul 2026 06:37:09 +0000
| Newsgroups | dev.linux.lists.linux-rt-devel,org.kernel.vger.bpf,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
Thank you for your contribution! Sashiko AI review found 1 potential issue(= s) to consider: - [Medium] NUMA Mempolicy Bypass in bpf_map_alloc_page_sleepable() and __bp= f_alloc_page() -- --- Patch [1]: [PATCH bpf-next 1/3] bpf: Add a sleepable page allocator for= map memory --- commit 7a6023ef7f19fc884e94ffe457ae83ff0d4b5e1e Author: Jiayuan Chen <[email protected]> bpf: Add a sleepable page allocator for map memory This commit introduces bpf_map_alloc_page_sleepable() to provide a blocking page allocator for callers known to be in a sleepable context, such as user page fault handlers. This avoids the conservative, non-blocking behavior of can_alloc_pages() which is always false under PREEMPT_RT. > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > --- a/kernel/bpf/syscall.c > +++ b/kernel/bpf/syscall.c [ ... ] > @@ -602,15 +602,14 @@ static bool can_alloc_pages(void) > !IS_ENABLED(CONFIG_PREEMPT_RT); > } > =20 > +#define BPF_PAGE_GFP (GFP_KERNEL | __GFP_ZERO | __GFP_ACCOUNT | __GFP_NO= WARN) > + > static struct page *__bpf_alloc_page(int nid) > { > if (!can_alloc_pages()) > return alloc_pages_nolock(__GFP_ACCOUNT, nid, 0); > =20 > - return alloc_pages_node(nid, > - GFP_KERNEL | __GFP_ZERO | __GFP_ACCOUNT > - | __GFP_NOWARN, > - 0); > + return alloc_pages_node(nid, BPF_PAGE_GFP, 0); [Severity: Medium] This isn't a bug introduced by this patch, but does using alloc_pages_node() here bypass the task's NUMA mempolicy? Because alloc_pages_node() ignores task mempolicy, pages might land on the current CPU's local NUMA node instead of the node requested by the task. Could this branch to use a mempolicy-aware allocation when nid is NUMA_NO_NODE? > } [ ... ] > +/* > + * For callers that know they run in a sleepable context, e.g. a user pa= ge > + * 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, int= nid) > +{ > + might_sleep(); > + return alloc_pages_node(nid, BPF_PAGE_GFP, 0); [Severity: Medium] Since this function is intended for user space page fault handlers, will unconditionally calling alloc_pages_node() cause the allocation to ignore the user task's mempolicy? Similar to __bpf_alloc_page(), if nid is NUMA_NO_NODE, replacing this with a mempolicy-aware allocation might prevent silent performance regressions from misaligned memory placement during page faults. > +} --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260727062521.3762= [email protected]?part=3D1