All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eduard Zingerman <eddyz87@gmail.com>
To: bpf@vger.kernel.org, ast@kernel.org
Cc: andrii@kernel.org, daniel@iogearbox.net, martin.lau@linux.dev,
	kernel-team@fb.com, yonghong.song@linux.dev, memxor@gmail.com,
	Eduard Zingerman <eddyz87@gmail.com>
Subject: [RFC bpf-next 01/11] bpf: use branch predictions in opt_hard_wire_dead_code_branches()
Date: Thu,  7 Nov 2024 09:50:30 -0800	[thread overview]
Message-ID: <20241107175040.1659341-2-eddyz87@gmail.com> (raw)
In-Reply-To: <20241107175040.1659341-1-eddyz87@gmail.com>

Consider dead code elimination problem for program like below:

    main:
      1: r1 = 42
      2: call <subprogram>;
      3: exit

    subprogram:
      4: r0 = 1
      5: if r1 != 42 goto +1
      6: r0 = 2
      7: exit;

Here verifier would visit every instruction and thus
bpf_insn_aux_data->seen flag would be set for both true (7)
and falltrhough (6) branches of conditional (5).
Hence opt_hard_wire_dead_code_branches() will not replace
conditional (5) with unconditional jump.

To cover such cases:
- add two fields in struct bpf_insn_aux_data:
  - true_branch_taken;
  - false_branch_taken;
- adjust check_cond_jmp_op() to set the fields according to jump
  predictions;
- handle these flags in the opt_hard_wire_dead_code_branches():
  - true_branch_taken && !false_branch_taken
    always jump, replace instruction with 'goto off';
  - !true_branch_taken && false_branch_taken
    always falltrhough, replace with 'goto +0';
  - true_branch_taken && false_branch_taken
    jump and falltrhough are possible, don't change the instruction;
  - !true_branch_taken && !false_branch_taken
    neither jump, nor falltrhough are possible, if condition itself
    must be a dead code (should be removed by opt_remove_dead_code).

Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
 include/linux/bpf_verifier.h |  4 +++-
 kernel/bpf/verifier.c        | 16 ++++++++++++----
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 4513372c5bc8..ed4eacfd4db7 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -570,7 +570,9 @@ struct bpf_insn_aux_data {
 	struct btf_struct_meta *kptr_struct_meta;
 	u64 map_key_state; /* constant (32 bit) key tracking for maps */
 	int ctx_field_size; /* the ctx field size for load insn, maybe 0 */
-	u32 seen; /* this insn was processed by the verifier at env->pass_cnt */
+	bool seen; /* this insn was processed by the verifier at env->pass_cnt */
+	bool true_branch_taken; /* for cond jumps, set if verifier ever took true branch */
+	bool false_branch_taken; /* for cond jumps, set if verifier ever took false branch */
 	bool sanitize_stack_spill; /* subject to Spectre v4 sanitation */
 	bool zext_dst; /* this insn zero extends dst reg */
 	bool needs_zext; /* alu op needs to clear upper bits */
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 7958d6ff6b73..3bae0bbc1da9 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -13265,7 +13265,7 @@ static void sanitize_mark_insn_seen(struct bpf_verifier_env *env)
 	 * rewrite/sanitize them.
 	 */
 	if (!vstate->speculative)
-		env->insn_aux_data[env->insn_idx].seen = env->pass_cnt;
+		env->insn_aux_data[env->insn_idx].seen = env->pass_cnt > 0;
 }
 
 static int sanitize_err(struct bpf_verifier_env *env,
@@ -15484,6 +15484,7 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
 {
 	struct bpf_verifier_state *this_branch = env->cur_state;
 	struct bpf_verifier_state *other_branch;
+	struct bpf_insn_aux_data *aux = &env->insn_aux_data[*insn_idx];
 	struct bpf_reg_state *regs = this_branch->frame[this_branch->curframe]->regs;
 	struct bpf_reg_state *dst_reg, *other_branch_regs, *src_reg = NULL;
 	struct bpf_reg_state *eq_branch_regs;
@@ -15510,6 +15511,8 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
 				insn->off, insn->imm);
 			return -EINVAL;
 		}
+		aux->true_branch_taken = true;
+		aux->false_branch_taken = true;
 		prev_st = find_prev_entry(env, cur_st->parent, idx);
 
 		/* branch out 'fallthrough' insn as a new state to explore */
@@ -15579,6 +15582,7 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
 		 * the fall-through branch for simulation under speculative
 		 * execution.
 		 */
+		aux->true_branch_taken = true;
 		if (!env->bypass_spec_v1 &&
 		    !sanitize_speculative_path(env, insn, *insn_idx + 1,
 					       *insn_idx))
@@ -15592,6 +15596,7 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
 		 * program will go. If needed, push the goto branch for
 		 * simulation under speculative execution.
 		 */
+		aux->false_branch_taken = true;
 		if (!env->bypass_spec_v1 &&
 		    !sanitize_speculative_path(env, insn,
 					       *insn_idx + insn->off + 1,
@@ -15602,6 +15607,9 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
 		return 0;
 	}
 
+	aux->true_branch_taken = true;
+	aux->false_branch_taken = true;
+
 	/* Push scalar registers sharing same ID to jump history,
 	 * do this before creating 'other_branch', so that both
 	 * 'this_branch' and 'other_branch' share this history
@@ -19288,7 +19296,7 @@ static void adjust_insn_aux_data(struct bpf_verifier_env *env,
 {
 	struct bpf_insn_aux_data *old_data = env->insn_aux_data;
 	struct bpf_insn *insn = new_prog->insnsi;
-	u32 old_seen = old_data[off].seen;
+	bool old_seen = old_data[off].seen;
 	u32 prog_len;
 	int i;
 
@@ -19608,9 +19616,9 @@ static void opt_hard_wire_dead_code_branches(struct bpf_verifier_env *env)
 		if (!insn_is_cond_jump(insn->code))
 			continue;
 
-		if (!aux_data[i + 1].seen)
+		if (aux_data[i].true_branch_taken && !aux_data[i].false_branch_taken)
 			ja.off = insn->off;
-		else if (!aux_data[i + 1 + insn->off].seen)
+		else if (!aux_data[i].true_branch_taken && aux_data[i].false_branch_taken)
 			ja.off = 0;
 		else
 			continue;
-- 
2.47.0


  reply	other threads:[~2024-11-07 17:51 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-07 17:50 [RFC bpf-next 00/11] bpf: inlinable kfuncs for BPF Eduard Zingerman
2024-11-07 17:50 ` Eduard Zingerman [this message]
2024-11-14 22:20   ` [RFC bpf-next 01/11] bpf: use branch predictions in opt_hard_wire_dead_code_branches() Eduard Zingerman
2024-11-15  0:17     ` Andrii Nakryiko
2024-11-15  0:19       ` Andrii Nakryiko
2024-11-15  0:50         ` Eduard Zingerman
2024-11-15  3:03           ` Andrii Nakryiko
2024-11-15  0:20       ` Eduard Zingerman
2024-11-15  0:27         ` Alexei Starovoitov
2024-11-15  0:33           ` Eduard Zingerman
2024-11-15  0:38             ` Alexei Starovoitov
2024-11-15  0:43               ` Eduard Zingerman
2024-11-15  0:16   ` Andrii Nakryiko
2024-11-07 17:50 ` [RFC bpf-next 02/11] selftests/bpf: tests for opt_hard_wire_dead_code_branches() Eduard Zingerman
2024-11-07 17:50 ` [RFC bpf-next 03/11] bpf: shared BPF/native kfuncs Eduard Zingerman
2024-11-08  1:43   ` kernel test robot
2024-11-08 20:43   ` Toke Høiland-Jørgensen
2024-11-08 21:25     ` Eduard Zingerman
2024-11-11 18:41       ` Toke Høiland-Jørgensen
2024-11-15  0:27   ` Andrii Nakryiko
2024-11-07 17:50 ` [RFC bpf-next 04/11] bpf: allow specifying inlinable kfuncs in modules Eduard Zingerman
2024-11-09  6:57   ` kernel test robot
2024-11-09  7:07   ` kernel test robot
2024-11-07 17:50 ` [RFC bpf-next 05/11] bpf: dynamic allocation for bpf_verifier_env->subprog_info Eduard Zingerman
2024-11-07 17:50 ` [RFC bpf-next 06/11] bpf: KERNEL_VALUE register type Eduard Zingerman
2024-11-07 17:50 ` [RFC bpf-next 07/11] bpf: instantiate inlinable kfuncs before verification Eduard Zingerman
2024-11-07 17:50 ` [RFC bpf-next 08/11] bpf: special rules for kernel function calls inside inlinable kfuncs Eduard Zingerman
2024-11-07 17:50 ` [RFC bpf-next 09/11] bpf: move selected dynptr kfuncs to inlinable_kfuncs.c Eduard Zingerman
2024-11-07 17:50 ` [RFC bpf-next 10/11] selftests/bpf: tests to verify handling of inlined kfuncs Eduard Zingerman
2024-11-07 22:04   ` Jeff Johnson
2024-11-07 22:08     ` Eduard Zingerman
2024-11-07 22:19       ` Jeff Johnson
2024-11-07 23:00         ` Eduard Zingerman
2024-11-07 17:50 ` [RFC bpf-next 11/11] selftests/bpf: dynptr_slice benchmark Eduard Zingerman
2024-11-08 20:41 ` [RFC bpf-next 00/11] bpf: inlinable kfuncs for BPF Toke Høiland-Jørgensen
2024-11-08 23:01   ` Eduard Zingerman
2024-11-11 18:42     ` Toke Høiland-Jørgensen

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=20241107175040.1659341-2-eddyz87@gmail.com \
    --to=eddyz87@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kernel-team@fb.com \
    --cc=martin.lau@linux.dev \
    --cc=memxor@gmail.com \
    --cc=yonghong.song@linux.dev \
    /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.