GFP_ATOMIC doesn't cooperate well with memcg pressure so far, especially if we allocate too much GFP_ATOMIC memory. For example, when we set the memcg limit to limit a non-preallocated bpf memory, the GFP_ATOMIC can easily break the memcg limit by force charge. So it is very dangerous to use GFP_ATOMIC in non-preallocated case. One way to make it safe is to remove __GFP_HIGH from GFP_ATOMIC, IOW, use (__GFP_ATOMIC | __GFP_KSWAPD_RECLAIM) instead, then it will be limited if we allocate too much memory. There's a plan to completely remove __GFP_ATOMIC in the mm side[1], so let's use GFP_NOWAIT instead. We introduced BPF_F_NO_PREALLOC is because full map pre-allocation is too memory expensive for some cases. That means removing __GFP_HIGH doesn't break the rule of BPF_F_NO_PREALLOC, but has the same goal with it-avoiding issues caused by too much memory. So let's remove it. This fix can also apply to other run-time allocations, for example, the allocation in lpm trie, local storage and devmap. So let fix it consistently over the bpf code It also fixes a typo in the comment. [1]. https://lore.kernel.org/linux-mm/163712397076.13692.4727608274002939094@noble.neil.brown.name/
Unified · Split
| @@ -845,7 +845,7 @@ static struct bpf_dtab_netdev *__dev_map_alloc_node(struct net *net, · 844 unmodified lines | |||
| 845 | struct bpf_dtab_netdev *dev; | 845 | struct bpf_dtab_netdev *dev; |
| 846 | 846 | ||
| 847 | dev = bpf_map_kmalloc_node(&dtab->map, sizeof(*dev), | 847 | dev = bpf_map_kmalloc_node(&dtab->map, sizeof(*dev), |
| 848 | GFP_ATOMIC | __GFP_NOWARN, | 848 | GFP_NOWAIT | __GFP_NOWARN, |
| 849 | dtab->map.numa_node); | 849 | dtab->map.numa_node); |
| 850 | if (!dev) | 850 | if (!dev) |
| 851 | return ERR_PTR(-ENOMEM); | 851 | return ERR_PTR(-ENOMEM); |
| @@ -61,7 +61,7 @@ · 60 unmodified lines | |||
| 61 | * | 61 | * |
| 62 | * As regular device interrupt handlers and soft interrupts are forced into | 62 | * As regular device interrupt handlers and soft interrupts are forced into |
| 63 | * thread context, the existing code which does | 63 | * thread context, the existing code which does |
| 64 | * spin_lock*(); alloc(GPF_ATOMIC); spin_unlock*(); | 64 | * spin_lock*(); alloc(GFP_ATOMIC); spin_unlock*(); |
| 65 | * just works. | 65 | * just works. |
| 66 | * | 66 | * |
| 67 | * In theory the BPF locks could be converted to regular spinlocks as well, | 67 | * In theory the BPF locks could be converted to regular spinlocks as well, |
| @@ -978,7 +978,7 @@ static struct htab_elem *alloc_htab_elem(struct bpf_htab *htab, void *key, · 910 unmodified lines | |||
| 978 | goto dec_count; | 978 | goto dec_count; |
| 979 | } | 979 | } |
| 980 | l_new = bpf_map_kmalloc_node(&htab->map, htab->elem_size, | 980 | l_new = bpf_map_kmalloc_node(&htab->map, htab->elem_size, |
| 981 | GFP_ATOMIC | __GFP_NOWARN, | 981 | GFP_NOWAIT | __GFP_NOWARN, |
| 982 | htab->map.numa_node); | 982 | htab->map.numa_node); |
| 983 | if (!l_new) { | 983 | if (!l_new) { |
| 984 | l_new = ERR_PTR(-ENOMEM); | 984 | l_new = ERR_PTR(-ENOMEM); |
| @@ -996,7 +996,7 @@ static struct htab_elem *alloc_htab_elem(struct bpf_htab *htab, void *key, · 11 unmodified lines | |||
| 996 | } else { | 996 | } else { |
| 997 | /* alloc_percpu zero-fills */ | 997 | /* alloc_percpu zero-fills */ |
| 998 | pptr = bpf_map_alloc_percpu(&htab->map, size, 8, | 998 | pptr = bpf_map_alloc_percpu(&htab->map, size, 8, |
| 999 | GFP_ATOMIC | __GFP_NOWARN); | 999 | GFP_NOWAIT | __GFP_NOWARN); |
| 1000 | if (!pptr) { | 1000 | if (!pptr) { |
| 1001 | kfree(l_new); | 1001 | kfree(l_new); |
| 1002 | l_new = ERR_PTR(-ENOMEM); | 1002 | l_new = ERR_PTR(-ENOMEM); |
| @@ -165,7 +165,7 @@ static int cgroup_storage_update_elem(struct bpf_map *map, void *key, · 164 unmodified lines | |||
| 165 | } | 165 | } |
| 166 | 166 | ||
| 167 | new = bpf_map_kmalloc_node(map, struct_size(new, data, map->value_size), | 167 | new = bpf_map_kmalloc_node(map, struct_size(new, data, map->value_size), |
| 168 | __GFP_ZERO | GFP_ATOMIC | __GFP_NOWARN, | 168 | __GFP_ZERO | GFP_NOWAIT | __GFP_NOWARN, |
| 169 | map->numa_node); | 169 | map->numa_node); |
| 170 | if (!new) | 170 | if (!new) |
| 171 | return -ENOMEM; | 171 | return -ENOMEM; |
| @@ -285,7 +285,7 @@ static struct lpm_trie_node *lpm_trie_node_alloc(const struct lpm_trie *trie, · 284 unmodified lines | |||
| 285 | if (value) | 285 | if (value) |
| 286 | size += trie->map.value_size; | 286 | size += trie->map.value_size; |
| 287 | 287 | ||
| 288 | node = bpf_map_kmalloc_node(&trie->map, size, GFP_ATOMIC | __GFP_NOWARN, | 288 | node = bpf_map_kmalloc_node(&trie->map, size, GFP_NOWAIT | __GFP_NOWARN, |
| 289 | trie->map.numa_node); | 289 | trie->map.numa_node); |
| 290 | if (!node) | 290 | if (!node) |
| 291 | return NULL; | 291 | return NULL; |