From: Dave Marchevsky <davemarchevsky@fb.com>
To: <bpf@vger.kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Andrii Nakryiko <andrii@kernel.org>,
Kernel Team <kernel-team@fb.com>,
Kumar Kartikeya Dwivedi <memxor@gmail.com>,
Tejun Heo <tj@kernel.org>,
Dave Marchevsky <davemarchevsky@fb.com>
Subject: [PATCH v3 bpf-next 02/11] bpf: Improve bpf_reg_state space usage for non-owning ref lock
Date: Tue, 31 Jan 2023 10:00:07 -0800 [thread overview]
Message-ID: <20230131180016.3368305-3-davemarchevsky@fb.com> (raw)
In-Reply-To: <20230131180016.3368305-1-davemarchevsky@fb.com>
This patch eliminates extra bpf_reg_state memory usage added due to
previous patch keeping a copy of lock identity in reg state for
non-owning refs.
Instead of copying lock identity around, this patch changes
non_owning_ref_lock field to be a bool, taking advantage of the
following:
* There can currently only be one active lock at a time
* non-owning refs are only valid in the critical section
So if a verifier_state has an active_lock, any non-owning ref must've
been obtained under that lock, and any non-owning ref not obtained under
that lock must have been invalidated previously. Therefore if a
non-owning ref is associated with a lock, it's the active_lock of the
current state. So we can keep a bool "are we associated with active_lock
of current state" instead of copying lock identity around.
Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com>
---
include/linux/bpf_verifier.h | 2 +-
kernel/bpf/verifier.c | 20 ++++++++++----------
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 1c6bbde40705..baeb5afb0b81 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -84,7 +84,7 @@ struct bpf_reg_state {
struct {
struct btf *btf;
u32 btf_id;
- struct bpf_active_lock non_owning_ref_lock;
+ bool non_owning_ref_lock;
};
u32 mem_size; /* for PTR_TO_MEM | PTR_TO_MEM_OR_NULL */
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 4b1883ffcf21..ed816e824928 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -935,9 +935,8 @@ static void print_verifier_state(struct bpf_verifier_env *env,
verbose_a("id=%d", reg->id);
if (reg->ref_obj_id)
verbose_a("ref_obj_id=%d", reg->ref_obj_id);
- if (reg->non_owning_ref_lock.ptr)
- verbose_a("non_own_id=(%p,%d)", reg->non_owning_ref_lock.ptr,
- reg->non_owning_ref_lock.id);
+ if (reg->non_owning_ref_lock)
+ verbose(env, "non_own_ref,");
if (t != SCALAR_VALUE)
verbose_a("off=%d", reg->off);
if (type_is_pkt_pointer(t))
@@ -4834,7 +4833,7 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env,
}
if (type_is_alloc(reg->type) && !reg->ref_obj_id &&
- !reg->non_owning_ref_lock.ptr) {
+ !reg->non_owning_ref_lock) {
verbose(env, "verifier internal error: ref_obj_id for allocated object must be non-zero\n");
return -EFAULT;
}
@@ -7085,13 +7084,15 @@ static int release_reference(struct bpf_verifier_env *env,
static void invalidate_non_owning_refs(struct bpf_verifier_env *env,
struct bpf_active_lock *lock)
{
+ struct bpf_active_lock *cur_state_lock;
struct bpf_func_state *unused;
struct bpf_reg_state *reg;
+ cur_state_lock = &env->cur_state->active_lock;
bpf_for_each_reg_in_vstate(env->cur_state, unused, reg, ({
- if (reg->non_owning_ref_lock.ptr &&
- reg->non_owning_ref_lock.ptr == lock->ptr &&
- reg->non_owning_ref_lock.id == lock->id)
+ if (reg->non_owning_ref_lock &&
+ cur_state_lock->ptr == lock->ptr &&
+ cur_state_lock->id == lock->id)
__mark_reg_unknown(env, reg);
}));
}
@@ -8718,13 +8719,12 @@ static int ref_set_non_owning_lock(struct bpf_verifier_env *env, struct bpf_reg_
return -EFAULT;
}
- if (reg->non_owning_ref_lock.ptr) {
+ if (reg->non_owning_ref_lock) {
verbose(env, "verifier internal error: non_owning_ref_lock already set\n");
return -EFAULT;
}
- reg->non_owning_ref_lock.id = state->active_lock.id;
- reg->non_owning_ref_lock.ptr = state->active_lock.ptr;
+ reg->non_owning_ref_lock = true;
return 0;
}
--
2.30.2
next prev parent reply other threads:[~2023-01-31 18:01 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-31 18:00 [PATCH v3 bpf-next 00/11] BPF rbtree next-gen datastructure Dave Marchevsky
2023-01-31 18:00 ` [PATCH v3 bpf-next 01/11] bpf: Migrate release_on_unlock logic to non-owning ref semantics Dave Marchevsky
2023-01-31 18:00 ` Dave Marchevsky [this message]
2023-01-31 18:25 ` [PATCH v3 bpf-next 02/11] bpf: Improve bpf_reg_state space usage for non-owning ref lock Dave Marchevsky
2023-02-01 21:36 ` Alexei Starovoitov
2023-01-31 18:00 ` [PATCH v3 bpf-next 03/11] selftests/bpf: Update linked_list tests for non-owning ref semantics Dave Marchevsky
2023-01-31 18:00 ` [PATCH v3 bpf-next 04/11] bpf: Add basic bpf_rb_{root,node} support Dave Marchevsky
2023-01-31 18:00 ` [PATCH v3 bpf-next 05/11] bpf: Add bpf_rbtree_{add,remove,first} kfuncs Dave Marchevsky
2023-01-31 18:00 ` [PATCH v3 bpf-next 06/11] bpf: Add support for bpf_rb_root and bpf_rb_node in kfunc args Dave Marchevsky
2023-01-31 18:00 ` [PATCH v3 bpf-next 07/11] bpf: Add callback validation to kfunc verifier logic Dave Marchevsky
2023-01-31 18:00 ` [PATCH v3 bpf-next 08/11] bpf: Special verifier handling for bpf_rbtree_{remove, first} Dave Marchevsky
2023-01-31 18:00 ` [PATCH v3 bpf-next 09/11] bpf: Add bpf_rbtree_{add,remove,first} decls to bpf_experimental.h Dave Marchevsky
2023-01-31 18:00 ` [PATCH v3 bpf-next 10/11] selftests/bpf: Add rbtree selftests Dave Marchevsky
2023-01-31 18:00 ` [PATCH v3 bpf-next 11/11] bpf, documentation: Add graph documentation for non-owning refs Dave Marchevsky
2023-01-31 21:40 ` Dave Marchevsky
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=20230131180016.3368305-3-davemarchevsky@fb.com \
--to=davemarchevsky@fb.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kernel-team@fb.com \
--cc=memxor@gmail.com \
--cc=tj@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.