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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox