Re: [PATCH bpf-next 1/6] bpf: Derive the atomic load register in one place

> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index 4edba8182db1b..15d83684c6e94 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -414,6 +414,30 @@ static inline bool bpf_atomic_is_load_acq(const struct bpf_insn *insn)
>  	       insn->imm == BPF_LOAD_ACQ;
>  }
>
> +/*
> + * Given an instruction @insn, return the number of the BPF register that a
> + * BPF_ATOMIC reads the value at its memory operand into, or -1 if there is
> + * no such register. That is the register a BPF_PROBE_ATOMIC has to clear when
> + * the access faults. Like bpf_atomic_is_load_acq(), @insn is not assumed to
> + * be a BPF_ATOMIC here.
> + */
> +static inline int bpf_atomic_load_reg(const struct bpf_insn *insn)
> +{
> +	if (BPF_CLASS(insn->code) != BPF_STX ||
> +	    (BPF_MODE(insn->code) != BPF_ATOMIC &&
> +	     BPF_MODE(insn->code) != BPF_PROBE_ATOMIC))
> +		return -1;
> +
> +	switch (insn->imm) {
> +	case BPF_LOAD_ACQ:
> +		return insn->dst_reg;
> +	case BPF_CMPXCHG:
> +		return BPF_REG_0;
> +	default:
> +		return (insn->imm & BPF_FETCH) ? insn->src_reg : -1;
> +	}
> +}
> +
>  /* Memory store, *(uint *) (dst_reg + off16) = imm32 */

This isn't a bug, but could insn_def_regno()'s BPF_STX arm (and the
similar switches in const_fold.c and liveness.c) call
bpf_atomic_load_reg(), so the mapping really does live in one place?

The helper is bit-for-bit equivalent to the BPF_STX arm of
insn_def_regno() in kernel/bpf/fixups.c:

	case BPF_STX:
		if (BPF_MODE(insn->code) == BPF_ATOMIC ||
		    BPF_MODE(insn->code) == BPF_PROBE_ATOMIC) {
			if (insn->imm == BPF_CMPXCHG)
				return BPF_REG_0;
			else if (insn->imm == BPF_LOAD_ACQ)
				return insn->dst_reg;
			else if (insn->imm & BPF_FETCH)
				return insn->src_reg;
		}
		return -1;

The same three-way mapping is also open coded in
kernel/bpf/const_fold.c and kernel/bpf/liveness.c.

Since the changelog's stated goal is that the derivation should live in
one place, it is a little surprising that the in-tree duplicates are
left alone.

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31395992453