Re: [PATCH] objpool: fix choosing allocation for percpu slots

["wuqiang.matt" <wuqiang.matt@bytedance.com>]

On 2024/8/22 16:25, Viktor Malik wrote:
> objpool intends to use vmalloc for default (non-atomic) allocations of
> percpu slots and objects. However, the condition checking if GFP flags
> are equal to GFP_ATOMIC is wrong and causes kmalloc to be used in most
> cases (even if GFP_KERNEL is requested). Since kmalloc cannot allocate
> large amounts of memory, this may lead to unexpected OOM errors.

Sure, good catch. Don't notice that GFP_ATOMIC is not atomic. My original
intention is using kmalloc only if GFP_ATOMIC is specified and other flags
should go with vmalloc, but (pool->gfp == GFP_ATOMIC) is not accurate.

Masami, please help review and include this patch into your patch set if
it's appropriate to you. Thanks.

Reviewed-by: Matt Wu <wuqiang.matt@bytedance.com>

> For instance, objpool is used by fprobe rethook which in turn is used by
> BPF kretprobe.multi and kprobe.session probe types. Trying to attach
> these to all kernel functions with libbpf using
> 
>      SEC("kprobe.session/*")
>      int kprobe(struct pt_regs *ctx)
>      {
>          [...]
>      }
> 
> fails on objpool slot allocation with ENOMEM.
> 
> Fix the condition to truly use vmalloc by default.
> 
> Fixes: b4edb8d2d464 ("lib: objpool added: ring-array based lockless MPMC")
> Signed-off-by: Viktor Malik <vmalik@redhat.com>
> ---
>   lib/objpool.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/objpool.c b/lib/objpool.c
> index 234f9d0bd081..fd108fe0d095 100644
> --- a/lib/objpool.c
> +++ b/lib/objpool.c
> @@ -76,7 +76,7 @@ objpool_init_percpu_slots(struct objpool_head *pool, int nr_objs,
>   		 * mimimal size of vmalloc is one page since vmalloc would
>   		 * always align the requested size to page size
>   		 */
> -		if (pool->gfp & GFP_ATOMIC)
> +		if ((pool->gfp & GFP_ATOMIC) == GFP_ATOMIC)
>   			slot = kmalloc_node(size, pool->gfp, cpu_to_node(i));
>   		else
>   			slot = __vmalloc_node(size, sizeof(void *), pool->gfp,