On 7/21/2023 4:44 AM, YiFei Zhu wrote:
> Sometimes during prefill all precpu chunks are full and atomic
> __alloc_percpu_gfp would not allocate new chunks. This will cause
> -ENOMEM immediately upon next unit_alloc.
>
> Prefill phase does not actually run in atomic context, so we can
> use this fact to allocate non-atomically with GFP_KERNEL instead
> of GFP_NOWAIT. This avoids the immediate -ENOMEM. Unfortunately
> unit_alloc runs in atomic context, even from map item allocation in
> syscalls, due to rcu_read_lock, so we can't do non-atomic
> workarounds in unit_alloc.
>
> Fixes: 4ab67149f3c6 ("bpf: Add percpu allocation support to bpf_mem_alloc.")
> Signed-off-by: YiFei Zhu <zhuyifei@xxxxxxxxxx>
Make sense to me, so
Acked-by: Hou Tao <houtao1@xxxxxxxxxx>
But I don't know whether or not it is suitable for bpf tree.
> ---
> kernel/bpf/memalloc.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/bpf/memalloc.c b/kernel/bpf/memalloc.c
> index 0668bcd7c926..016249672b43 100644
> --- a/kernel/bpf/memalloc.c
> +++ b/kernel/bpf/memalloc.c
> @@ -154,13 +154,17 @@ static struct mem_cgroup *get_memcg(const struct bpf_mem_cache *c)
> }
>
> /* Mostly runs from irq_work except __init phase. */
> -static void alloc_bulk(struct bpf_mem_cache *c, int cnt, int node)
> +static void alloc_bulk(struct bpf_mem_cache *c, int cnt, int node, bool atomic)
> {
> struct mem_cgroup *memcg = NULL, *old_memcg;
> unsigned long flags;
> + gfp_t gfp;
> void *obj;
> int i;
>
> + gfp = __GFP_NOWARN | __GFP_ACCOUNT;
> + gfp |= atomic ? GFP_NOWAIT : GFP_KERNEL;
> +
> memcg = get_memcg(c);
> old_memcg = set_active_memcg(memcg);
> for (i = 0; i < cnt; i++) {
> @@ -183,7 +187,7 @@ static void alloc_bulk(struct bpf_mem_cache *c, int cnt, int node)
> * will allocate from the current numa node which is what we
> * want here.
> */
> - obj = __alloc(c, node, GFP_NOWAIT | __GFP_NOWARN | __GFP_ACCOUNT);
> + obj = __alloc(c, node, gfp);
> if (!obj)
> break;
> }
> @@ -321,7 +325,7 @@ static void bpf_mem_refill(struct irq_work *work)
> /* irq_work runs on this cpu and kmalloc will allocate
> * from the current numa node which is what we want here.
> */
> - alloc_bulk(c, c->batch, NUMA_NO_NODE);
> + alloc_bulk(c, c->batch, NUMA_NO_NODE, true);
> else if (cnt > c->high_watermark)
> free_bulk(c);
> }
> @@ -367,7 +371,7 @@ static void prefill_mem_cache(struct bpf_mem_cache *c, int cpu)
> * prog won't be doing more than 4 map_update_elem from
> * irq disabled region
> */
> - alloc_bulk(c, c->unit_size <= 256 ? 4 : 1, cpu_to_node(cpu));
> + alloc_bulk(c, c->unit_size <= 256 ? 4 : 1, cpu_to_node(cpu), false);
> }
>
> /* When size != 0 bpf_mem_cache for each cpu.
