Re: [RFC PATCH 2/4] tracing/probes: Compile all fetchargs into a single BPF program per event

["Alexei Starovoitov" <alexei.starovoitov@gmail.com>]

On Wed Jul 1, 2026 at 6:45 AM PDT, Masami Hiramatsu (Google) wrote:
> +static bool trace_probe_can_compile_bpf(struct trace_probe *tp)
> +{
> +	int i;
> +
> +	if (tp->nr_args == 0)
> +		return false;
> +
> +	for (i = 0; i < tp->nr_args; i++) {
> +		struct probe_arg *parg = &tp->args[i];
> +		struct fetch_insn *code = parg->code;
> +
> +		while (code->op != FETCH_OP_END) {
> +			switch (code->op) {
> +			case FETCH_OP_REG:
> +			case FETCH_OP_IMM:
> +			case FETCH_OP_DEREF:
> +			case FETCH_OP_ST_RAW:
> +			case FETCH_OP_ST_MEM:
> +				break;
> +			case FETCH_OP_ARG:
> +				if (regs_get_kernel_argument_offset(code->param) < 0)
> +					return false;
> +				break;
> +			default:
> +				return false;
> +			}
> +			code++;
> +		}
> +	}
> +	return true;
> +}
> +
> +static void trace_probe_compile_bpf(struct trace_probe *tp)
> +{
> +	struct bpf_insn *insns;
> +	int i = 0;
> +	struct bpf_prog *prog;
> +	int err, idx;
> +
> +	if (!trace_probe_can_compile_bpf(tp))
> +		return;
> +
> +	insns = kmalloc_array(512, sizeof(struct bpf_insn), GFP_KERNEL);
> +	if (!insns)
> +		return;
> +
> +	/* Prologue: R6 = ctx */
> +	insns[i++] = BPF_MOV64_REG(BPF_REG_6, BPF_REG_1);
> +	/* R7 = ctx->rec */
> +	insns[i++] = BPF_LDX_MEM(BPF_DW, BPF_REG_7, BPF_REG_6,
> +				 offsetof(struct fetch_bpf_ctx, rec));
> +	/* R8 = ctx->data */
> +	insns[i++] = BPF_LDX_MEM(BPF_DW, BPF_REG_8, BPF_REG_6,
> +				 offsetof(struct fetch_bpf_ctx, data));
> +	/* R9 = total size (0) */
> +	insns[i++] = BPF_MOV64_IMM(BPF_REG_9, 0);
> +
> +	for (idx = 0; idx < tp->nr_args; idx++) {
> +		struct probe_arg *parg = &tp->args[idx];
> +		struct fetch_insn *code = parg->code;
> +
> +		while (code->op != FETCH_OP_END && i < 500) {
> +			switch (code->op) {
> +			case FETCH_OP_REG:
> +				/* R0 = *(unsigned long *)(R7 + code->param) */
> +				insns[i++] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_7, code->param);
> +				break;
> +			case FETCH_OP_ARG: {
> +				int offset = regs_get_kernel_argument_offset(code->param);
> +				/* R0 = *(unsigned long *)(R7 + offset) */
> +				insns[i++] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_7, offset);
> +				break;
> +			}
> +			case FETCH_OP_IMM:
> +				insns[i++] = BPF_LD_IMM64(BPF_REG_0, code->immediate);
> +				break;
> +			case FETCH_OP_DEREF:
> +				/* Add offset: R3 = R0 + code->offset (src) */
> +				insns[i++] = BPF_MOV64_REG(BPF_REG_2, BPF_REG_0);
> +				if (code->offset)
> +					insns[i++] = BPF_ALU64_IMM(BPF_ADD, BPF_REG_2,
> +								   code->offset);
> +				/* R1 = dst (R10 - 8 on stack) */
> +				insns[i++] = BPF_MOV64_REG(BPF_REG_1, BPF_REG_10);
> +				insns[i++] = BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8);
> +				/* R3 = size */
> +				insns[i++] = BPF_MOV64_IMM(BPF_REG_3, sizeof(unsigned long));
> +				/* Call copy_from_kernel_nofault(dst, src, size) */
> +				insns[i++] = BPF_EMIT_CALL(copy_from_kernel_nofault);
> +				/* if (R0 < 0) return R0; */
> +				insns[i++] = BPF_JMP_IMM(BPF_JSGE, BPF_REG_0, 0, 1);
> +				insns[i++] = BPF_EXIT_INSN();
> +				/* R0 = *(unsigned long *)(R10 - 8) */
> +				insns[i++] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_10, -8);
> +				break;
> +			case FETCH_OP_ST_RAW:
> +				/* Store R0 into R8 (data) + parg->offset based on size */
> +				switch (code->size) {
> +				case 1:
> +					insns[i++] = BPF_STX_MEM(BPF_B, BPF_REG_8, BPF_REG_0,
> +								 parg->offset);
> +					break;
> +				case 2:
> +					insns[i++] = BPF_STX_MEM(BPF_H, BPF_REG_8, BPF_REG_0,
> +								 parg->offset);
> +					break;
> +				case 4:
> +					insns[i++] = BPF_STX_MEM(BPF_W, BPF_REG_8, BPF_REG_0,
> +								 parg->offset);
> +					break;
> +				case 8:
> +					insns[i++] = BPF_STX_MEM(BPF_DW, BPF_REG_8, BPF_REG_0,
> +								  parg->offset);
> +					break;
> +				}
> +				break;
> +			case FETCH_OP_ST_MEM:
> +				/* Add offset: R2 = R0 + code->offset (src) */
> +				insns[i++] = BPF_MOV64_REG(BPF_REG_2, BPF_REG_0);
> +				if (code->offset)
> +					insns[i++] = BPF_ALU64_IMM(BPF_ADD, BPF_REG_2,
> +								   code->offset);
> +				/* R1 = dst (R8 + parg->offset) */
> +				insns[i++] = BPF_MOV64_REG(BPF_REG_1, BPF_REG_8);
> +				if (parg->offset)
> +					insns[i++] = BPF_ALU64_IMM(BPF_ADD, BPF_REG_1,
> +								   parg->offset);
> +				/* R3 = size */
> +				insns[i++] = BPF_MOV64_IMM(BPF_REG_3, code->size);
> +				/* Call copy_from_kernel_nofault(dst, src, size) */
> +				insns[i++] = BPF_EMIT_CALL(copy_from_kernel_nofault);
> +				/* if (R0 < 0) return R0; */
> +				insns[i++] = BPF_JMP_IMM(BPF_JSGE, BPF_REG_0, 0, 1);
> +				insns[i++] = BPF_EXIT_INSN();
> +				break;
> +			default:
> +				goto out;
> +			}
> +			code++;
> +		}
> +	}

Nack.
I really don't like it.
There were days in the past when the kernel generating bpf directly was appealing.
These days are gone. Performance improvements for fetchargs is not a good reason
to add all this complexity and bypass verifier checks.
bpf insns should come from user space.