Re: [PATCH bpf-next 4/6] bpf, arm64: Clear fetch destination on faulting arena atomic
Eduard Zingerman <[email protected]>
| Newsgroups | org.kernel.vger.bpf |
|---|---|
| Message-ID | <[email protected]> |
On Mon, 2026-08-10 at 19:30 +0100, Puranjay Mohan wrote:
...
> > > arch/arm64/net/bpf_jit_comp.c | 36 +++++++++++++++++++++++++----------
> > > 1 file changed, 26 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
> > > index d14d297ebb96..796ff9193cfb 100644
> > > --- a/arch/arm64/net/bpf_jit_comp.c
> > > +++ b/arch/arm64/net/bpf_jit_comp.c
> >
> > ...
> >
> > > @@ -1183,13 +1187,25 @@ static int add_exception_handler(const struct bpf_insn *insn,
> > > * dst_reg like a BPF_LDX does, hence it must not be treated as a store
> > > * here.
> > > */
> > > - if (BPF_CLASS(insn->code) != BPF_LDX && !bpf_atomic_is_load_acq(insn))
> > > - dst_reg = DONT_CLEAR;
> > > + if (BPF_CLASS(insn->code) != BPF_LDX && !bpf_atomic_is_load_acq(insn)) {
> > > + /*
> > > + * A store has no destination register to clear, except for a
> > > + * read-modify-write with BPF_FETCH, which also reads the old
> > > + * value into src_reg, or into r0 for a BPF_CMPXCHG. Either way
> > > + * the access is still reported as a write.
> > > + */
> > > + int load_reg = bpf_atomic_load_reg(insn);
> >
> > Nit: I think it would be more in line with the current arm64 jit organization
> > if bpf_atomic_load_reg() call is moved to the add_exception_handler()
> > callsite in the build_insn(), where it handles BPF_PROBE_ATOMIC.
> >
>
> Wouldn't that cause more code churn?
Having 'dst' both passed as a parameter and passed from the callsite
is somewhat inconsistent.