From: Martin KaFai Lau <martin.lau@linux.dev>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: bpf <bpf@vger.kernel.org>, Alexei Starovoitov <ast@kernel.org>,
Andrii Nakryiko <andrii@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Kernel Team <kernel-team@meta.com>,
Namhyung Kim <namhyung@kernel.org>
Subject: Re: [PATCH bpf-next 12/16] bpf: Use bpf_mem_cache_alloc/free in bpf_selem_alloc/free
Date: Tue, 7 Mar 2023 16:38:30 -0800 [thread overview]
Message-ID: <ad7666cc-3ec4-0401-c571-7d07a8f7fbbf@linux.dev> (raw)
In-Reply-To: <CAADnVQJck3WUKNht++fAjKRTBtLUVL2K2FrWeyUr=+MMeiiZvQ@mail.gmail.com>
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);
}
next prev parent reply other threads:[~2023-03-08 0:39 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-06 8:42 [PATCH bpf-next 00/16] bpf: Use bpf_mem_cache_alloc/free in bpf_local_storage Martin KaFai Lau
2023-03-06 8:42 ` [PATCH bpf-next 01/16] bpf: Move a few bpf_local_storage functions to static scope Martin KaFai Lau
2023-03-06 8:42 ` [PATCH bpf-next 02/16] bpf: Refactor codes into bpf_local_storage_destroy Martin KaFai Lau
2023-03-06 8:42 ` [PATCH bpf-next 03/16] bpf: Remove __bpf_local_storage_map_alloc Martin KaFai Lau
2023-03-06 8:42 ` [PATCH bpf-next 04/16] bpf: Remove the preceding __ from __bpf_selem_unlink_storage Martin KaFai Lau
2023-03-06 8:42 ` [PATCH bpf-next 05/16] bpf: Remember smap in bpf_local_storage Martin KaFai Lau
2023-03-06 8:42 ` [PATCH bpf-next 06/16] bpf: Repurpose use_trace_rcu to reuse_now " Martin KaFai Lau
2023-03-06 8:42 ` [PATCH bpf-next 07/16] bpf: Remove bpf_selem_free_fields*_rcu Martin KaFai Lau
2023-03-07 1:35 ` Kumar Kartikeya Dwivedi
2023-03-06 8:42 ` [PATCH bpf-next 08/16] bpf: Add bpf_selem_free_rcu callback Martin KaFai Lau
2023-03-06 8:42 ` [PATCH bpf-next 09/16] bpf: Add bpf_selem_free() Martin KaFai Lau
2023-03-06 8:53 ` Martin KaFai Lau
2023-03-06 8:42 ` [PATCH bpf-next 10/16] bpf: Add bpf_local_storage_rcu callback Martin KaFai Lau
2023-03-06 8:42 ` [PATCH bpf-next 11/16] bpf: Add bpf_local_storage_free() Martin KaFai Lau
2023-03-06 8:42 ` [PATCH bpf-next 12/16] bpf: Use bpf_mem_cache_alloc/free in bpf_selem_alloc/free Martin KaFai Lau
2023-03-07 3:47 ` Alexei Starovoitov
2023-03-08 0:38 ` Martin KaFai Lau [this message]
2023-03-06 8:42 ` [PATCH bpf-next 13/16] bpf: Use bpf_mem_cache_alloc/free for bpf_local_storage Martin KaFai Lau
2023-03-06 8:42 ` [PATCH bpf-next 14/16] selftests/bpf: Replace CHECK with ASSERT in test_local_storage Martin KaFai Lau
2023-03-08 1:15 ` Andrii Nakryiko
2023-03-08 1:24 ` Martin KaFai Lau
2023-03-06 8:42 ` [PATCH bpf-next 15/16] selftests/bpf: Check freeing sk->sk_local_storage with sk_local_storage->smap is NULL Martin KaFai Lau
2023-03-06 8:42 ` [PATCH bpf-next 16/16] selftests/bpf: Add local-storage-create benchmark Martin KaFai Lau
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ad7666cc-3ec4-0401-c571-7d07a8f7fbbf@linux.dev \
--to=martin.lau@linux.dev \
--cc=alexei.starovoitov@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kernel-team@meta.com \
--cc=namhyung@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox