* [PATCH bpf-next v2 1/2] bpf: removed unused 'env' parameter from is_reg64 and insn_has_def32
2025-08-07 1:02 [PATCH bpf-next v2 0/2] bpf: use vrealloc() in bpf_patch_insn_data() Eduard Zingerman
@ 2025-08-07 1:02 ` Eduard Zingerman
2025-08-07 15:26 ` Kumar Kartikeya Dwivedi
2025-08-07 1:02 ` [PATCH bpf-next v2 2/2] bpf: use realloc in bpf_patch_insn_data Eduard Zingerman
2025-08-07 16:20 ` [PATCH bpf-next v2 0/2] bpf: use vrealloc() in bpf_patch_insn_data() patchwork-bot+netdevbpf
2 siblings, 1 reply; 7+ messages in thread
From: Eduard Zingerman @ 2025-08-07 1:02 UTC (permalink / raw)
To: bpf, ast, andrii; +Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87
Parameter 'env' is not used by is_reg64() and insn_has_def32()
functions. Remove the parameter to make it clear that neither function
depends on 'env' state, e.g. env->insn_aux_data.
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
kernel/bpf/verifier.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 0806295945e4..69eb2b5c2218 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3663,7 +3663,7 @@ static int mark_irq_flag_read(struct bpf_verifier_env *env, struct bpf_reg_state
* code only. It returns TRUE if the source or destination register operates
* on 64-bit, otherwise return FALSE.
*/
-static bool is_reg64(struct bpf_verifier_env *env, struct bpf_insn *insn,
+static bool is_reg64(struct bpf_insn *insn,
u32 regno, struct bpf_reg_state *reg, enum reg_arg_type t)
{
u8 code, class, op;
@@ -3774,14 +3774,14 @@ static int insn_def_regno(const struct bpf_insn *insn)
}
/* Return TRUE if INSN has defined any 32-bit value explicitly. */
-static bool insn_has_def32(struct bpf_verifier_env *env, struct bpf_insn *insn)
+static bool insn_has_def32(struct bpf_insn *insn)
{
int dst_reg = insn_def_regno(insn);
if (dst_reg == -1)
return false;
- return !is_reg64(env, insn, dst_reg, NULL, DST_OP);
+ return !is_reg64(insn, dst_reg, NULL, DST_OP);
}
static void mark_insn_zext(struct bpf_verifier_env *env,
@@ -3812,7 +3812,7 @@ static int __check_reg_arg(struct bpf_verifier_env *env, struct bpf_reg_state *r
mark_reg_scratched(env, regno);
reg = ®s[regno];
- rw64 = is_reg64(env, insn, regno, reg, t);
+ rw64 = is_reg64(insn, regno, reg, t);
if (t == SRC_OP) {
/* check whether register used as source operand can be read */
if (reg->type == NOT_INIT) {
@@ -20712,7 +20712,7 @@ static void adjust_insn_aux_data(struct bpf_verifier_env *env,
* (cnt == 1) is taken or not. There is no guarantee INSN at OFF is the
* original insn at old prog.
*/
- old_data[off].zext_dst = insn_has_def32(env, insn + off + cnt - 1);
+ old_data[off].zext_dst = insn_has_def32(insn + off + cnt - 1);
if (cnt == 1)
return;
@@ -20724,7 +20724,7 @@ static void adjust_insn_aux_data(struct bpf_verifier_env *env,
for (i = off; i < off + cnt - 1; i++) {
/* Expand insni[off]'s seen count to the patched range. */
new_data[i].seen = old_seen;
- new_data[i].zext_dst = insn_has_def32(env, insn + i);
+ new_data[i].zext_dst = insn_has_def32(insn + i);
}
env->insn_aux_data = new_data;
vfree(old_data);
@@ -21131,7 +21131,7 @@ static int opt_subreg_zext_lo32_rnd_hi32(struct bpf_verifier_env *env,
* BPF_STX + SRC_OP, so it is safe to pass NULL
* here.
*/
- if (is_reg64(env, &insn, load_reg, NULL, DST_OP)) {
+ if (is_reg64(&insn, load_reg, NULL, DST_OP)) {
if (class == BPF_LD &&
BPF_MODE(code) == BPF_IMM)
i++;
--
2.47.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH bpf-next v2 1/2] bpf: removed unused 'env' parameter from is_reg64 and insn_has_def32
2025-08-07 1:02 ` [PATCH bpf-next v2 1/2] bpf: removed unused 'env' parameter from is_reg64 and insn_has_def32 Eduard Zingerman
@ 2025-08-07 15:26 ` Kumar Kartikeya Dwivedi
0 siblings, 0 replies; 7+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2025-08-07 15:26 UTC (permalink / raw)
To: Eduard Zingerman
Cc: bpf, ast, andrii, daniel, martin.lau, kernel-team, yonghong.song
On Thu, 7 Aug 2025 at 03:03, Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> Parameter 'env' is not used by is_reg64() and insn_has_def32()
> functions. Remove the parameter to make it clear that neither function
> depends on 'env' state, e.g. env->insn_aux_data.
>
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> ---
Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH bpf-next v2 2/2] bpf: use realloc in bpf_patch_insn_data
2025-08-07 1:02 [PATCH bpf-next v2 0/2] bpf: use vrealloc() in bpf_patch_insn_data() Eduard Zingerman
2025-08-07 1:02 ` [PATCH bpf-next v2 1/2] bpf: removed unused 'env' parameter from is_reg64 and insn_has_def32 Eduard Zingerman
@ 2025-08-07 1:02 ` Eduard Zingerman
2025-08-07 16:00 ` Kumar Kartikeya Dwivedi
2025-08-07 16:02 ` Anton Protopopov
2025-08-07 16:20 ` [PATCH bpf-next v2 0/2] bpf: use vrealloc() in bpf_patch_insn_data() patchwork-bot+netdevbpf
2 siblings, 2 replies; 7+ messages in thread
From: Eduard Zingerman @ 2025-08-07 1:02 UTC (permalink / raw)
To: bpf, ast, andrii
Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87,
Alexei Starovoitov
Avoid excessive vzalloc/vfree calls when patching instructions in
do_misc_fixups(). bpf_patch_insn_data() uses vzalloc to allocate new
memory for env->insn_aux_data for each patch as follows:
struct bpf_prog *bpf_patch_insn_data(env, ...)
{
...
new_data = vzalloc(... O(program size) ...);
...
adjust_insn_aux_data(env, new_data, ...);
...
}
void adjust_insn_aux_data(env, new_data, ...)
{
...
memcpy(new_data, env->insn_aux_data);
vfree(env->insn_aux_data);
env->insn_aux_data = new_data;
...
}
The vzalloc/vfree pair is hot in perf report collected for e.g.
pyperf180 test case. It can be replaced with a call to vrealloc in
order to reduce the number of actual memory allocations.
This is a stop-gap solution, as bpf_patch_insn_data is still hot in
the profile. More comprehansive solutions had been discussed before
e.g. as in [1].
[1] https://lore.kernel.org/bpf/CAEf4BzY_E8MSL4mD0UPuuiDcbJhh9e2xQo2=5w+ppRWWiYSGvQ@mail.gmail.com/
Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
kernel/bpf/verifier.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 69eb2b5c2218..a61d57996692 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -20699,12 +20699,11 @@ static void convert_pseudo_ld_imm64(struct bpf_verifier_env *env)
* [0, off) and [off, end) to new locations, so the patched range stays zero
*/
static void adjust_insn_aux_data(struct bpf_verifier_env *env,
- struct bpf_insn_aux_data *new_data,
struct bpf_prog *new_prog, u32 off, u32 cnt)
{
- struct bpf_insn_aux_data *old_data = env->insn_aux_data;
+ struct bpf_insn_aux_data *data = env->insn_aux_data;
struct bpf_insn *insn = new_prog->insnsi;
- u32 old_seen = old_data[off].seen;
+ u32 old_seen = data[off].seen;
u32 prog_len;
int i;
@@ -20712,22 +20711,20 @@ static void adjust_insn_aux_data(struct bpf_verifier_env *env,
* (cnt == 1) is taken or not. There is no guarantee INSN at OFF is the
* original insn at old prog.
*/
- old_data[off].zext_dst = insn_has_def32(insn + off + cnt - 1);
+ data[off].zext_dst = insn_has_def32(insn + off + cnt - 1);
if (cnt == 1)
return;
prog_len = new_prog->len;
- memcpy(new_data, old_data, sizeof(struct bpf_insn_aux_data) * off);
- memcpy(new_data + off + cnt - 1, old_data + off,
- sizeof(struct bpf_insn_aux_data) * (prog_len - off - cnt + 1));
+ memmove(data + off + cnt - 1, data + off,
+ sizeof(struct bpf_insn_aux_data) * (prog_len - off - cnt + 1));
+ memset(data + off, 0, sizeof(struct bpf_insn_aux_data) * (cnt - 1));
for (i = off; i < off + cnt - 1; i++) {
/* Expand insni[off]'s seen count to the patched range. */
- new_data[i].seen = old_seen;
- new_data[i].zext_dst = insn_has_def32(insn + i);
+ data[i].seen = old_seen;
+ data[i].zext_dst = insn_has_def32(insn + i);
}
- env->insn_aux_data = new_data;
- vfree(old_data);
}
static void adjust_subprog_starts(struct bpf_verifier_env *env, u32 off, u32 len)
@@ -20765,10 +20762,14 @@ static struct bpf_prog *bpf_patch_insn_data(struct bpf_verifier_env *env, u32 of
struct bpf_insn_aux_data *new_data = NULL;
if (len > 1) {
- new_data = vzalloc(array_size(env->prog->len + len - 1,
- sizeof(struct bpf_insn_aux_data)));
+ new_data = vrealloc(env->insn_aux_data,
+ array_size(env->prog->len + len - 1,
+ sizeof(struct bpf_insn_aux_data)),
+ GFP_KERNEL_ACCOUNT | __GFP_ZERO);
if (!new_data)
return NULL;
+
+ env->insn_aux_data = new_data;
}
new_prog = bpf_patch_insn_single(env->prog, off, patch, len);
@@ -20780,7 +20781,7 @@ static struct bpf_prog *bpf_patch_insn_data(struct bpf_verifier_env *env, u32 of
vfree(new_data);
return NULL;
}
- adjust_insn_aux_data(env, new_data, new_prog, off, len);
+ adjust_insn_aux_data(env, new_prog, off, len);
adjust_subprog_starts(env, off, len);
adjust_poke_descs(new_prog, off, len);
return new_prog;
--
2.47.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH bpf-next v2 2/2] bpf: use realloc in bpf_patch_insn_data
2025-08-07 1:02 ` [PATCH bpf-next v2 2/2] bpf: use realloc in bpf_patch_insn_data Eduard Zingerman
@ 2025-08-07 16:00 ` Kumar Kartikeya Dwivedi
2025-08-07 16:02 ` Anton Protopopov
1 sibling, 0 replies; 7+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2025-08-07 16:00 UTC (permalink / raw)
To: Eduard Zingerman
Cc: bpf, ast, andrii, daniel, martin.lau, kernel-team, yonghong.song,
Alexei Starovoitov
On Thu, 7 Aug 2025 at 03:02, Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> Avoid excessive vzalloc/vfree calls when patching instructions in
> do_misc_fixups(). bpf_patch_insn_data() uses vzalloc to allocate new
> memory for env->insn_aux_data for each patch as follows:
>
> struct bpf_prog *bpf_patch_insn_data(env, ...)
> {
> ...
> new_data = vzalloc(... O(program size) ...);
> ...
> adjust_insn_aux_data(env, new_data, ...);
> ...
> }
>
> void adjust_insn_aux_data(env, new_data, ...)
> {
> ...
> memcpy(new_data, env->insn_aux_data);
> vfree(env->insn_aux_data);
> env->insn_aux_data = new_data;
> ...
> }
>
> The vzalloc/vfree pair is hot in perf report collected for e.g.
> pyperf180 test case. It can be replaced with a call to vrealloc in
> order to reduce the number of actual memory allocations.
>
> This is a stop-gap solution, as bpf_patch_insn_data is still hot in
> the profile. More comprehansive solutions had been discussed before
> e.g. as in [1].
>
> [1] https://lore.kernel.org/bpf/CAEf4BzY_E8MSL4mD0UPuuiDcbJhh9e2xQo2=5w+ppRWWiYSGvQ@mail.gmail.com/
>
> Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> ---
Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> [...]
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH bpf-next v2 2/2] bpf: use realloc in bpf_patch_insn_data
2025-08-07 1:02 ` [PATCH bpf-next v2 2/2] bpf: use realloc in bpf_patch_insn_data Eduard Zingerman
2025-08-07 16:00 ` Kumar Kartikeya Dwivedi
@ 2025-08-07 16:02 ` Anton Protopopov
1 sibling, 0 replies; 7+ messages in thread
From: Anton Protopopov @ 2025-08-07 16:02 UTC (permalink / raw)
To: Eduard Zingerman
Cc: bpf, ast, andrii, daniel, martin.lau, kernel-team, yonghong.song,
Alexei Starovoitov
On 25/08/06 06:02PM, Eduard Zingerman wrote:
> Avoid excessive vzalloc/vfree calls when patching instructions in
> do_misc_fixups(). bpf_patch_insn_data() uses vzalloc to allocate new
> memory for env->insn_aux_data for each patch as follows:
>
> struct bpf_prog *bpf_patch_insn_data(env, ...)
> {
> ...
> new_data = vzalloc(... O(program size) ...);
> ...
> adjust_insn_aux_data(env, new_data, ...);
> ...
> }
>
> void adjust_insn_aux_data(env, new_data, ...)
> {
> ...
> memcpy(new_data, env->insn_aux_data);
> vfree(env->insn_aux_data);
> env->insn_aux_data = new_data;
> ...
> }
>
> The vzalloc/vfree pair is hot in perf report collected for e.g.
> pyperf180 test case. It can be replaced with a call to vrealloc in
> order to reduce the number of actual memory allocations.
Given that I am in any case looking at the code around,
I've rebased on top of this change and tested it:
Tested-by: Anton Protopopov <a.s.protopopov@gmail.com>
> This is a stop-gap solution, as bpf_patch_insn_data is still hot in
> the profile. More comprehansive solutions had been discussed before
> e.g. as in [1].
>
> [1] https://lore.kernel.org/bpf/CAEf4BzY_E8MSL4mD0UPuuiDcbJhh9e2xQo2=5w+ppRWWiYSGvQ@mail.gmail.com/
>
> Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
[snip]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next v2 0/2] bpf: use vrealloc() in bpf_patch_insn_data()
2025-08-07 1:02 [PATCH bpf-next v2 0/2] bpf: use vrealloc() in bpf_patch_insn_data() Eduard Zingerman
2025-08-07 1:02 ` [PATCH bpf-next v2 1/2] bpf: removed unused 'env' parameter from is_reg64 and insn_has_def32 Eduard Zingerman
2025-08-07 1:02 ` [PATCH bpf-next v2 2/2] bpf: use realloc in bpf_patch_insn_data Eduard Zingerman
@ 2025-08-07 16:20 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-08-07 16:20 UTC (permalink / raw)
To: Eduard Zingerman
Cc: bpf, ast, andrii, daniel, martin.lau, kernel-team, yonghong.song
Hello:
This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:
On Wed, 6 Aug 2025 18:02:03 -0700 you wrote:
> Function bpf_patch_insn_data() uses vzalloc/vfree pair to allocate
> memory for updated insn_aux_data. These operations are expensive for
> big programs where a lot of rewrites happen, e.g. for pyperf180 test
> case. The pair can be replaced with a call to vrealloc in order to
> reduce the number of actual memory allocations.
>
> Perf stat w/o this patch:
>
> [...]
Here is the summary with links:
- [bpf-next,v2,1/2] bpf: removed unused 'env' parameter from is_reg64 and insn_has_def32
https://git.kernel.org/bpf/bpf-next/c/cb070a8156c1
- [bpf-next,v2,2/2] bpf: use realloc in bpf_patch_insn_data
https://git.kernel.org/bpf/bpf-next/c/77620d126739
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 7+ messages in thread