Re: [PATCH v7 RESEND 5/6] arm64: futex: small optimisation for __llsc_futex_atomic_set()

Will Deacon
2025-09-11T15:28:55Z

On Sat, Aug 16, 2025 at 04:19:28PM +0100, Yeoreum Yun wrote:
> __llsc_futex_atomic_set() is implmented using
> LLSC_FUTEX_ATOMIC_OP() macro with "mov %w3, %w5".
> But this instruction isn't required to implement fux_atomic_set()
> so make a small optimisation by implementing __llsc_futex_atomic_set()
> as separate function.
>
> This will make usage of LLSC_FUTEX_ATOMIC_OP() macro more simple.
>
> Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> ---
> arch/arm64/include/asm/futex.h | 43 ++++++++++++++++++++++++++++------
> 1 file changed, 36 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm64/include/asm/futex.h b/arch/arm64/include/asm/futex.h
> index ab7003cb4724..22a6301a9f3d 100644
> --- a/arch/arm64/include/asm/futex.h
> +++ b/arch/arm64/include/asm/futex.h
> @@ -13,7 +13,7 @@
>
> #define LLSC_MAX_LOOPS 128 /* What's the largest number you can think of? */
>
> -#define LLSC_FUTEX_ATOMIC_OP(op, insn) \
> +#define LLSC_FUTEX_ATOMIC_OP(op, asm_op) \
> static __always_inline int \
> __llsc_futex_atomic_##op(int oparg, u32 __user *uaddr, int *oval) \
> { \
> @@ -24,7 +24,7 @@ __llsc_futex_atomic_##op(int oparg, u32 __user *uaddr, int *oval) \
> asm volatile("// __llsc_futex_atomic_" #op "\n" \
> " prfm pstl1strm, %2\n" \
> "1: ldxr %w1, %2\n" \
> - insn "\n" \
> +" " #asm_op " %w3, %w1, %w5\n" \
> "2: stlxr %w0, %w3, %2\n" \
> " cbz %w0, 3f\n" \
> " sub %w4, %w4, %w0\n" \
> @@ -46,11 +46,40 @@ __llsc_futex_atomic_##op(int oparg, u32 __user *uaddr, int *oval) \
> return ret; \
> }
>
> -LLSC_FUTEX_ATOMIC_OP(add, "add %w3, %w1, %w5")
> -LLSC_FUTEX_ATOMIC_OP(or, "orr %w3, %w1, %w5")
> -LLSC_FUTEX_ATOMIC_OP(and, "and %w3, %w1, %w5")
> -LLSC_FUTEX_ATOMIC_OP(eor, "eor %w3, %w1, %w5")
> -LLSC_FUTEX_ATOMIC_OP(set, "mov %w3, %w5")
> +LLSC_FUTEX_ATOMIC_OP(add, add)
> +LLSC_FUTEX_ATOMIC_OP(or, orr)
> +LLSC_FUTEX_ATOMIC_OP(and, and)
> +LLSC_FUTEX_ATOMIC_OP(eor, eor)
> +
> +static __always_inline int
> +__llsc_futex_atomic_set(int oparg, u32 __user *uaddr, int *oval)
> +{
> + unsigned int loops = LLSC_MAX_LOOPS;
> + int ret, oldval;
> +
> + uaccess_enable_privileged();
> + asm volatile("//__llsc_futex_xchg\n"
> +" prfm pstl1strm, %2\n"
> +"1: ldxr %w1, %2\n"
> +"2: stlxr %w0, %w4, %2\n"
> +" cbz %w3, 3f\n"
> +" sub %w3, %w3, %w0\n"
> +" cbnz %w3, 1b\n"
> +" mov %w0, %w5\n"
> +"3:\n"
> +" dmb ish\n"
> + _ASM_EXTABLE_UACCESS_ERR(1b, 3b, %w0)
> + _ASM_EXTABLE_UACCESS_ERR(2b, 3b, %w0)
> + : "=&r" (ret), "=&r" (oldval), "+Q" (*uaddr), "+r" (loops)
> + : "r" (oparg), "Ir" (-EAGAIN)
> + : "memory");
> + uaccess_disable_privileged();
> +
> + if (!ret)
> + *oval = oldval;

Hmm, I'm really not sure this is worthwhile. I doubt the "optimisation"
actually does anything and adding a whole new block of asm just for the
SET case isn't much of an improvement on the maintainability side, either.

Will