On 3/6/23 7:47 PM, Alexei Starovoitov wrote:
@@ -80,12 +80,32 @@ bpf_selem_alloc(struct bpf_local_storage_map *smap, void *owner,
if (charge_mem && mem_charge(smap, owner, smap->elem_size))
return NULL;
- selem = bpf_map_kzalloc(&smap->map, smap->elem_size,
- gfp_flags | __GFP_NOWARN);
+ migrate_disable();
+ selem = bpf_mem_cache_alloc(&smap->selem_ma);
+ migrate_enable();
+ if (!selem && (gfp_flags & GFP_KERNEL)) {
+ void *ma_node;
+
+ ma_node = bpf_map_kzalloc(&smap->map,
+ BPF_MA_SIZE(smap->elem_size),
+ gfp_flags | __GFP_NOWARN);
+ if (ma_node)
+ selem = BPF_MA_PTR(ma_node);
+ }
If I understand it correctly the code is not trying
to free selem the same way it allocated it.
So we can have kzalloc-ed selems freed into bpf_mem_cache_alloc free-list.
That feels dangerous.
I don't think we can do such things in local storage,
but if we add this api to bpf_mem_alloc it might be acceptable.
I mean mem alloc will try to take from the free list and if empty
and GFP_KERNEL it will kzalloc it.
The knowledge of hidden llist_node shouldn't leave the bpf/memalloc.c file.
reuse_now should probably be a memalloc api flag too.
The implementation detail that it's scary but ok-ish to kfree or
bpf_mem_cache_free depending on circumstances should stay in memalloc.c
All make sense. I will create a bpf_mem_cache_alloc_flags(..., gfp_t flags) to
hide the llist_node and kzalloc details. For free, local storage still needs to
use the selem->rcu head in its call_rcu_tasks_trace(), so I will create a
bpf_mem_cache_raw_free(void *ptr) to hide the llist_node details, like:
/* 'struct bpf_mem_alloc *ma' is not available at this
* point but the caller knows it is percpu or not and
* call different raw_free function.
*/
void bpf_mem_cache_raw_free(void *ptr)
{
kfree(ptr - LLIST_NODE_SZ);
}
