On Mon, 26 Aug 2024 08:07:18 +0200
Viktor Malik <[email protected]> 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 b/c GFP_ATOMIC is a combination of bits
You meant "whether GFP flags sets any bit of GFP_ATOMIC is wrong"?
> (__GFP_HIGH|__GFP_KSWAPD_RECLAIM) and so `pool->gfp & GFP_ATOMIC` will
> be true if either bit is set. Since GFP_ATOMIC and GFP_KERNEL share the
> ___GFP_KSWAPD_RECLAIM bit, kmalloc will be used in cases when GFP_KERNEL
> is specified, i.e. in all current usages of objpool.
>
> This may lead to unexpected OOM errors since kmalloc cannot allocate
> large amounts of memory.
>
> 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.
>
Anyway, this looks good to me.
Thank you,
> Fixes: b4edb8d2d464 ("lib: objpool added: ring-array based lockless MPMC")
> Signed-off-by: Viktor Malik <[email protected]>
> Acked-by: Andrii Nakryiko <[email protected]>
> Reviewed-by: Matt Wu <[email protected]>
> ---
> 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,
> --
> 2.46.0
>
--
Masami Hiramatsu (Google) <[email protected]>
