From: Martin KaFai Lau <martin.lau@linux.dev>
To: bpf@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>,
Andrii Nakryiko <andrii@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
kernel-team@meta.com, Kumar Kartikeya Dwivedi <memxor@gmail.com>
Subject: [PATCH v2 bpf-next 07/17] bpf: Remove bpf_selem_free_fields*_rcu
Date: Tue, 7 Mar 2023 22:59:26 -0800 [thread overview]
Message-ID: <20230308065936.1550103-8-martin.lau@linux.dev> (raw)
In-Reply-To: <20230308065936.1550103-1-martin.lau@linux.dev>
From: Martin KaFai Lau <martin.lau@kernel.org>
This patch removes the bpf_selem_free_fields*_rcu. The
bpf_obj_free_fields() can be done before the call_rcu_trasks_trace()
and kfree_rcu(). It is needed when a later patch uses
bpf_mem_cache_alloc/free. In bpf hashtab, bpf_obj_free_fields()
is also called before calling bpf_mem_cache_free. The discussion
can be found in
https://lore.kernel.org/bpf/f67021ee-21d9-bfae-6134-4ca542fab843@linux.dev/
Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
---
kernel/bpf/bpf_local_storage.c | 67 +++-------------------------------
1 file changed, 5 insertions(+), 62 deletions(-)
diff --git a/kernel/bpf/bpf_local_storage.c b/kernel/bpf/bpf_local_storage.c
index 70c34a948c3c..715deaaefe13 100644
--- a/kernel/bpf/bpf_local_storage.c
+++ b/kernel/bpf/bpf_local_storage.c
@@ -109,27 +109,6 @@ static void bpf_local_storage_free_rcu(struct rcu_head *rcu)
kfree_rcu(local_storage, rcu);
}
-static void bpf_selem_free_fields_rcu(struct rcu_head *rcu)
-{
- struct bpf_local_storage_elem *selem;
- struct bpf_local_storage_map *smap;
-
- selem = container_of(rcu, struct bpf_local_storage_elem, rcu);
- /* protected by the rcu_barrier*() */
- smap = rcu_dereference_protected(SDATA(selem)->smap, true);
- bpf_obj_free_fields(smap->map.record, SDATA(selem)->data);
- kfree(selem);
-}
-
-static void bpf_selem_free_fields_trace_rcu(struct rcu_head *rcu)
-{
- /* Free directly if Tasks Trace RCU GP also implies RCU GP */
- if (rcu_trace_implies_rcu_gp())
- bpf_selem_free_fields_rcu(rcu);
- else
- call_rcu(rcu, bpf_selem_free_fields_rcu);
-}
-
static void bpf_selem_free_trace_rcu(struct rcu_head *rcu)
{
struct bpf_local_storage_elem *selem;
@@ -151,7 +130,6 @@ static bool bpf_selem_unlink_storage_nolock(struct bpf_local_storage *local_stor
{
struct bpf_local_storage_map *smap;
bool free_local_storage;
- struct btf_record *rec;
void *owner;
smap = rcu_dereference_check(SDATA(selem)->smap, bpf_rcu_lock_held());
@@ -192,26 +170,11 @@ static bool bpf_selem_unlink_storage_nolock(struct bpf_local_storage *local_stor
SDATA(selem))
RCU_INIT_POINTER(local_storage->cache[smap->cache_idx], NULL);
- /* A different RCU callback is chosen whenever we need to free
- * additional fields in selem data before freeing selem.
- * bpf_local_storage_map_free only executes rcu_barrier to wait for RCU
- * callbacks when it has special fields, hence we can only conditionally
- * dereference smap, as by this time the map might have already been
- * freed without waiting for our call_rcu callback if it did not have
- * any special fields.
- */
- rec = smap->map.record;
- if (!reuse_now) {
- if (!IS_ERR_OR_NULL(rec))
- call_rcu_tasks_trace(&selem->rcu, bpf_selem_free_fields_trace_rcu);
- else
- call_rcu_tasks_trace(&selem->rcu, bpf_selem_free_trace_rcu);
- } else {
- if (!IS_ERR_OR_NULL(rec))
- call_rcu(&selem->rcu, bpf_selem_free_fields_rcu);
- else
- kfree_rcu(selem, rcu);
- }
+ bpf_obj_free_fields(smap->map.record, SDATA(selem)->data);
+ if (!reuse_now)
+ call_rcu_tasks_trace(&selem->rcu, bpf_selem_free_trace_rcu);
+ else
+ kfree_rcu(selem, rcu);
if (rcu_access_pointer(local_storage->smap) == smap)
RCU_INIT_POINTER(local_storage->smap, NULL);
@@ -769,26 +732,6 @@ void bpf_local_storage_map_free(struct bpf_map *map,
*/
synchronize_rcu();
- /* Only delay freeing of smap, buckets are not needed anymore */
kvfree(smap->buckets);
-
- /* When local storage has special fields, callbacks for
- * bpf_selem_free_fields_rcu and bpf_selem_free_fields_trace_rcu will
- * keep using the map BTF record, we need to execute an RCU barrier to
- * wait for them as the record will be freed right after our map_free
- * callback.
- */
- if (!IS_ERR_OR_NULL(smap->map.record)) {
- rcu_barrier_tasks_trace();
- /* We cannot skip rcu_barrier() when rcu_trace_implies_rcu_gp()
- * is true, because while call_rcu invocation is skipped in that
- * case in bpf_selem_free_fields_trace_rcu (and all local
- * storage maps pass reuse_now = false), there can be
- * call_rcu callbacks based on reuse_now = true in the
- * while ((selem = ...)) loop above or when owner's free path
- * calls bpf_local_storage_unlink_nolock.
- */
- rcu_barrier();
- }
bpf_map_area_free(smap);
}
--
2.34.1
next prev parent reply other threads:[~2023-03-08 7:00 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-08 6:59 [PATCH v2 bpf-next 00/17] bpf: Use bpf_mem_cache_alloc/free in bpf_local_storage Martin KaFai Lau
2023-03-08 6:59 ` [PATCH v2 bpf-next 01/17] bpf: Move a few bpf_local_storage functions to static scope Martin KaFai Lau
2023-03-08 6:59 ` [PATCH v2 bpf-next 02/17] bpf: Refactor codes into bpf_local_storage_destroy Martin KaFai Lau
2023-03-08 6:59 ` [PATCH v2 bpf-next 03/17] bpf: Remove __bpf_local_storage_map_alloc Martin KaFai Lau
2023-03-08 6:59 ` [PATCH v2 bpf-next 04/17] bpf: Remove the preceding __ from __bpf_selem_unlink_storage Martin KaFai Lau
2023-03-08 6:59 ` [PATCH v2 bpf-next 05/17] bpf: Remember smap in bpf_local_storage Martin KaFai Lau
2023-03-08 6:59 ` [PATCH v2 bpf-next 06/17] bpf: Repurpose use_trace_rcu to reuse_now " Martin KaFai Lau
2023-03-08 6:59 ` Martin KaFai Lau [this message]
2023-03-08 6:59 ` [PATCH v2 bpf-next 08/17] bpf: Add bpf_selem_free_rcu callback Martin KaFai Lau
2023-03-08 6:59 ` [PATCH v2 bpf-next 09/17] bpf: Add bpf_selem_free() Martin KaFai Lau
2023-03-08 6:59 ` [PATCH v2 bpf-next 10/17] bpf: Add bpf_local_storage_rcu callback Martin KaFai Lau
2023-03-08 6:59 ` [PATCH v2 bpf-next 11/17] bpf: Add bpf_local_storage_free() Martin KaFai Lau
2023-03-08 6:59 ` [PATCH v2 bpf-next 12/17] bpf: Add a few bpf mem allocator functions Martin KaFai Lau
2023-03-10 19:19 ` Alexei Starovoitov
2023-03-08 6:59 ` [PATCH v2 bpf-next 13/17] bpf: Use bpf_mem_cache_alloc/free in bpf_selem_alloc/free Martin KaFai Lau
2023-03-08 6:59 ` [PATCH v2 bpf-next 14/17] bpf: Use bpf_mem_cache_alloc/free for bpf_local_storage Martin KaFai Lau
2023-03-08 6:59 ` [PATCH v2 bpf-next 15/17] selftests/bpf: Replace CHECK with ASSERT in test_local_storage Martin KaFai Lau
2023-03-08 6:59 ` [PATCH v2 bpf-next 16/17] selftests/bpf: Check freeing sk->sk_local_storage with sk_local_storage->smap is NULL Martin KaFai Lau
2023-03-08 6:59 ` [PATCH v2 bpf-next 17/17] 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=20230308065936.1550103-8-martin.lau@linux.dev \
--to=martin.lau@linux.dev \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kernel-team@meta.com \
--cc=memxor@gmail.com \
/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