From: Eduard Zingerman <eddyz87@gmail.com>
To: Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>,
bpf@vger.kernel.org, ast@kernel.org, andrii@kernel.org,
daniel@iogearbox.net, kafai@meta.com, kernel-team@meta.com,
memxor@gmail.com
Cc: Mykyta Yatsenko <yatsenko@meta.com>
Subject: Re: [PATCH bpf-next v3 08/10] bpf: verifier: refactor kfunc specialization
Date: Mon, 20 Oct 2025 16:38:31 -0700 [thread overview]
Message-ID: <e6388c88bcb09404209a956e65f4d0510aa13294.camel@gmail.com> (raw)
In-Reply-To: <20251020222538.932915-9-mykyta.yatsenko5@gmail.com>
On Mon, 2025-10-20 at 23:25 +0100, Mykyta Yatsenko wrote:
[...]
> @@ -3375,18 +3366,25 @@ static int add_kfunc_call(struct bpf_verifier_env *env, u32 func_id, s16 offset)
> return err;
> }
>
> + err = btf_distill_func_proto(&env->log, desc_btf,
> + func_proto, func_name,
> + &func_model);
> + if (err)
> + return err;
> +
> + err = kfunc_call_imm(env, addr, func_id, &call_imm);
> + if (err)
> + return err;
> +
Sorry, I should have asked in v1/v2. Is there a reason to call this
function two times? In other words, it looks like doing the following
on top of your changes is sufficient and removes the need to call
kfunc_call_imm twice:
---- 8< -----------------------------------------
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 0418768d13e4..f509f9e0383d 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3373,13 +3373,8 @@ static int add_kfunc_call(struct bpf_verifier_env *env, u32 func_id, s16 offset)
if (err)
return err;
- err = kfunc_call_imm(env, addr, func_id, &call_imm);
- if (err)
- return err;
-
desc = &tab->descs[tab->nr_descs++];
desc->func_id = func_id;
- desc->imm = call_imm;
desc->offset = offset;
desc->addr = addr;
desc->func_model = func_model;
@@ -21892,8 +21887,10 @@ static int specialize_kfunc(struct bpf_verifier_env *env, struct bpf_kfunc_desc
unsigned long addr = 0;
int err;
+ addr = desc->addr;
+
if (offset) /* return if module BTF is used */
- return 0;
+ goto fill_imm;
if (bpf_dev_bound_kfunc_id(func_id)) {
xdp_kfunc = bpf_dev_bound_resolve_kfunc(prog, func_id);
@@ -21922,9 +21919,7 @@ static int specialize_kfunc(struct bpf_verifier_env *env, struct bpf_kfunc_desc
addr = (unsigned long)bpf_dynptr_from_file_sleepable;
}
- if (!addr) /* Nothing to patch with */
- return 0;
-
+fill_imm:
err = kfunc_call_imm(env, addr, func_id, &desc->imm);
if (err)
return err;
----------------------------------------- >8 ----
The desc->imm field is used only from:
- kfunc_desc_cmp_by_imm_off():
- invoked from do_misc_fixups() at the very end,
after all specialize_kfunc() calls are done.
- bpf_jit_find_kfunc_model(), from some jits.
So, that should be safe.
Selftests are passing after this change as well.
> desc = &tab->descs[tab->nr_descs++];
> desc->func_id = func_id;
> desc->imm = call_imm;
> desc->offset = offset;
> desc->addr = addr;
> - err = btf_distill_func_proto(&env->log, desc_btf,
> - func_proto, func_name,
> - &desc->func_model);
> - if (!err)
> - sort(tab->descs, tab->nr_descs, sizeof(tab->descs[0]),
> - kfunc_desc_cmp_by_id_off, NULL);
> - return err;
> + desc->func_model = func_model;
> + sort(tab->descs, tab->nr_descs, sizeof(tab->descs[0]),
> + kfunc_desc_cmp_by_id_off, NULL);
> + return 0;
> }
[...]
next prev parent reply other threads:[~2025-10-20 23:38 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-20 22:25 [PATCH bpf-next v3 00/10] bpf: Introduce file dynptr Mykyta Yatsenko
2025-10-20 22:25 ` [PATCH bpf-next v3 01/10] selftests/bpf: remove unnecessary kfunc prototypes Mykyta Yatsenko
2025-10-20 22:25 ` [PATCH bpf-next v3 02/10] bpf: widen dynptr size/offset to 64 bit Mykyta Yatsenko
2025-10-20 23:01 ` Eduard Zingerman
2025-10-21 11:59 ` Mykyta Yatsenko
2025-10-20 22:25 ` [PATCH bpf-next v3 03/10] lib: move freader into buildid.h Mykyta Yatsenko
2025-10-20 22:25 ` [PATCH bpf-next v3 04/10] lib/freader: support reading more than 2 folios Mykyta Yatsenko
2025-10-20 22:25 ` [PATCH bpf-next v3 05/10] bpf: verifier: centralize const dynptr check in unmark_stack_slots_dynptr() Mykyta Yatsenko
2025-10-20 22:25 ` [PATCH bpf-next v3 06/10] bpf: add plumbing for file-backed dynptr Mykyta Yatsenko
2025-10-20 23:08 ` Eduard Zingerman
2025-10-20 22:25 ` [PATCH bpf-next v3 07/10] bpf: add kfuncs and helpers support for file dynptrs Mykyta Yatsenko
2025-10-20 22:25 ` [PATCH bpf-next v3 08/10] bpf: verifier: refactor kfunc specialization Mykyta Yatsenko
2025-10-20 23:38 ` Eduard Zingerman [this message]
2025-10-21 13:03 ` Mykyta Yatsenko
2025-10-20 22:25 ` [PATCH bpf-next v3 09/10] bpf: dispatch to sleepable file dynptr Mykyta Yatsenko
2025-10-20 22:25 ` [PATCH bpf-next v3 10/10] selftests/bpf: add file dynptr tests Mykyta Yatsenko
2025-10-21 0:45 ` Eduard Zingerman
2025-10-21 13:55 ` Mykyta Yatsenko
2025-10-21 16:40 ` Eduard Zingerman
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=e6388c88bcb09404209a956e65f4d0510aa13294.camel@gmail.com \
--to=eddyz87@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kafai@meta.com \
--cc=kernel-team@meta.com \
--cc=memxor@gmail.com \
--cc=mykyta.yatsenko5@gmail.com \
--cc=yatsenko@meta.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 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.