On 10/6/24 13:26, Yonghong Song wrote: > > On 6/9/24 12:31 AM, Leon Hwang wrote: >> It's confusing to inspect 'prog->aux->tail_call_reachable' with drgn[0], >> when bpf prog has tail call but 'tail_call_reachable' is false. >> >> This patch corrects 'tail_call_reachable' when bpf prog has tail call. >> >> [0] https://github.com/osandov/drgn >> >> Signed-off-by: Leon Hwang <hffilwlqm@xxxxxxxxx> >> --- >> kernel/bpf/verifier.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c >> index 81a3d2ced78d5..d7045676246a7 100644 >> --- a/kernel/bpf/verifier.c >> +++ b/kernel/bpf/verifier.c >> @@ -2982,8 +2982,10 @@ static int check_subprogs(struct >> bpf_verifier_env *env) >> if (code == (BPF_JMP | BPF_CALL) && >> insn[i].src_reg == 0 && >> - insn[i].imm == BPF_FUNC_tail_call) >> + insn[i].imm == BPF_FUNC_tail_call) { >> subprog[cur_subprog].has_tail_call = true; >> + subprog[cur_subprog].tail_call_reachable = true; > > This tail_call_reachable is handled in jit. For example, in > arch/x86/net/bpf_jit_comp.c: > > static void detect_reg_usage(struct bpf_insn *insn, int insn_cnt, > bool *regs_used, bool *tail_call_seen) > { > int i; > > for (i = 1; i <= insn_cnt; i++, insn++) { > if (insn->code == (BPF_JMP | BPF_TAIL_CALL)) > *tail_call_seen = true; > if (insn->dst_reg == BPF_REG_6 || insn->src_reg == > BPF_REG_6) > regs_used[0] = true; > if (insn->dst_reg == BPF_REG_7 || insn->src_reg == > BPF_REG_7) > regs_used[1] = true; > if (insn->dst_reg == BPF_REG_8 || insn->src_reg == > BPF_REG_8) > regs_used[2] = true; > if (insn->dst_reg == BPF_REG_9 || insn->src_reg == > BPF_REG_9) > regs_used[3] = true; > } > } > > and > > detect_reg_usage(insn, insn_cnt, callee_regs_used, > &tail_call_seen); > /* tail call's presence in current prog implies it is > reachable */ > tail_call_reachable |= tail_call_seen; > > I didn't check other architectures. If other arch is similar to x86 w.r.t. > tail_call_reachable marking, your change looks good. But you should also > make changes in jit to remove those redundent checking. > By searching tail_call_reachable in arch directory, excluding x86, other architectures do not check 'prog->aux->tail_call_reachable'. By checking jit of arm64/loongarch/riscv/s390, they have their own way to handle tail call, unlike x86's way to detect tail_call_reachable. I'll send PATCH v2 to remove the redundant detecting in x86 jit. Thanks, Leon
