Hi Alexei,
On 11/20/2024 9:16 AM, Alexei Starovoitov wrote:
> On Sun, Nov 17, 2024 at 4:56 PM Hou Tao <houtao@xxxxxxxxxxxxxxx> wrote:
>>
>> +enum {
>> + LPM_TRIE_MA_IM = 0,
>> + LPM_TRIE_MA_LEAF,
>> + LPM_TRIE_MA_CNT,
>> +};
>> +
>> struct lpm_trie {
>> struct bpf_map map;
>> struct lpm_trie_node __rcu *root;
>> + struct bpf_mem_alloc ma[LPM_TRIE_MA_CNT];
>> + struct bpf_mem_alloc *im_ma;
>> + struct bpf_mem_alloc *leaf_ma;
> We cannot use bpf_ma-s liberally like that.
> Freelists are not huge, but we shouldn't be adding new bpf_ma
> in every map and every use case.
>
> bpf_mem_cache_is_mergeable() in the previous patch also
> leaks implementation details.
>
> Can you use bpf_global_ma for all nodes?
Will try. However, there are mainly two differences between
bpf_global_ma and map specific bpf_mem_alloc. The first one is the
memory accounting problem. All memories allocated from bpf_global_ma
will be accounted to the root memory cgroup instead of the current
memory cgroup (due to the return value of get_memcg()). I think we could
fix this partially by returning NULL instead of root_mem_cgroup when
c->objcg is NULL. However, even with the fix, the memory account is
still inaccurate, because these pre-allocated objects may be used by
other maps instead of the map which triggers the pre-allocation. The
second one is the freeing of freed objects when destroying the map. For
a map specific bpf_mem_alloc, most of these freed objects could be freed
immediately back to slub, However, it is not true for the bpf_global_ma,
because we could not tell whether the object belongs to a to-be-freed
map or not. And also we can not drain the bpf_global_ma just like we do
for bpf_mem_alloc.
>
> pw-bot: cr
> .
