Re: [PATCH RESEND v7 4/6] arm64: futex: refactor futex atomic operation

Yeoreum Yun
2025-09-11T16:05:31Z

Hi Will,

[...]

> > -#define __futex_atomic_op(insn, ret, oldval, uaddr, tmp, oparg) \
> > -do { \
> > - unsigned int loops = FUTEX_MAX_LOOPS; \
> > +#define LLSC_FUTEX_ATOMIC_OP(op, insn) \
> > +static __always_inline int \
> > +__llsc_futex_atomic_##op(int oparg, u32 __user *uaddr, int *oval) \
> > +{ \
> > + unsigned int loops = LLSC_MAX_LOOPS; \
> > + int ret, oldval, tmp; \
> > \
> > uaccess_enable_privileged(); \
> > - asm volatile( \
> > + asm volatile("// __llsc_futex_atomic_" #op "\n" \
> > " prfm pstl1strm, %2\n" \
> > "1: ldxr %w1, %2\n" \
> > insn "\n" \
> > @@ -35,45 +39,103 @@ do { \
> > : "r" (oparg), "Ir" (-EAGAIN) \
> > : "memory"); \
> > uaccess_disable_privileged(); \
> > -} while (0)
> > + \
> > + if (!ret) \
> > + *oval = oldval; \
>
> Why push the store to '*oval' down into here?

As __llsc_futext_atomic_##op() is declared with inline function
I think it would be better to pass oval from arch_futex_atomic_op_inuser()
as it is for readability.

Is it awful?

Thanks.

--
Sincerely,
Yeoreum Yun