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

On 7/27/26 2:37 PM, [email protected] wrote:
> 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 __bpf_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);
>>   }
>>   
>> +#define BPF_PAGE_GFP (GFP_KERNEL | __GFP_ZERO | __GFP_ACCOUNT | __GFP_NOWARN)
>> +
>>   static struct page *__bpf_alloc_page(int nid)
>>   {
>>   	if (!can_alloc_pages())
>>   		return alloc_pages_nolock(__GFP_ACCOUNT, nid, 0);
>>   
>> -	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 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, 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.
>
>> +}


You remind me. But I don't think this is really about (task) mempolicy.
We should respect map.numa_node/map->numa_node, just like other maps do
when allocating their elements.