Re: [PATCH RFC v2 2/2] arm64: vdso: Implement __vdso_futex_robust_try_unlock()

André Almeida
2026-04-27T16:27:19Z

Em 26/04/2026 15:30, Thomas Weißschuh escreveu:
> On 2026-04-24 15:56:01-0300, André Almeida wrote:
> (...)
>
>> Signed-off-by: André Almeida <andrealmeid@igalia.com>
>> ---
>> RFC:
>> - Should I duplicate the explanation found in the x86 commit or can I just
>> point to it?
>> - Only LL/SC for now but I can add LSE later if this looks good
>> - It the objdump I see that op_pending is store at x2. But how stable is this,
>> how can I write it in a way that's always x2?
>> ---
>> arch/arm64/Kconfig | 1 +
>> arch/arm64/include/asm/futex_robust.h | 35 +++++++++++++
>> arch/arm64/kernel/vdso/Makefile | 9 +++-
>> arch/arm64/kernel/vdso/vdso.lds.S | 4 ++
>> .../kernel/vdso/vfutex_robust_list_try_unlock.c | 59 ++++++++++++++++++++++
>> 5 files changed, 107 insertions(+), 1 deletion(-)
>
> What about the actual 32-bit vDSO in arch/arm64/kernel/vdso32/ ?
>

Right, I missed that. Then I should move
__vdso_futex_robust_list32_try_unlock() to arch/arm64/kernel/vdso32/ right?

> (...)
>
>> diff --git a/arch/arm64/kernel/vdso/vfutex_robust_list_try_unlock.c b/arch/arm64/kernel/vdso/vfutex_robust_list_try_unlock.c
>> new file mode 100644
>> index 000000000000..e8a8fb22a2fa
>> --- /dev/null
>> +++ b/arch/arm64/kernel/vdso/vfutex_robust_list_try_unlock.c
>> @@ -0,0 +1,59 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +#include <vdso/futex.h>
>> +#include <linux/stringify.h>
>> +
>> +#define LABEL(name, sz) __stringify(__futex_list##sz##_try_unlock_cs_##name)
>
> We should have some defines for these symbols. While they are not
> userspace ABI, they will be used by the selftests.
>

Do you mean to have this defined at include/uapi/linux/futex.h?

>> +#define GLOBLS(sz) ".globl " LABEL(start, sz) ", " LABEL(success, sz) ", " LABEL(end, sz) "\n"
>> +
>> +__u32 __vdso_futex_robust_list64_try_unlock(__u32 *lock, __u32 tid, __u64 *pop)
>> +{
>> + __u32 val, result;
>> +
>> + asm volatile (
>> + GLOBLS(64)
>> + " prfm pstl1strm, %[lock] \n"
>> + LABEL(start, 64)": \n"
>> + " ldxr %[val], %[lock] \n"
>> + " cmp %[tid], %[val] \n"
>> + " bne " LABEL(end, 64)" \n"
>> + " stlxr %w[result], xzr, %[lock] \n"
>> + " cbnz %w[result], " LABEL(start, 64)" \n"
>> + LABEL(success, 64)": \n"
>> + " str xzr, %[pop] \n"
>> + LABEL(end, 64)": \n"
>> +
>> + : [val] "=&r" (val), [result] "=r" (result)
>> + : [tid] "r" (tid), [lock] "Q" (*lock), [pop] "Q" (*pop)
>> + : "memory"
>> + );
>
> My clang 22.1.3 chokes on the assembly in this patch.
>

Do you mind sharing the output?

>> +
>> + return val;
>> +}
>> +
>> +#ifdef CONFIG_COMPAT_VDSO
>
> I am wondering about the CONFIG_COMPAT{,_VDSO} dependency here.
> As far as I know the list32 variant is meant to be used by code
> emulators which run 32-bit code on a 64-bit kernel, for example FEX.
> But these emulators don't actually seem to need CONFIG_COMPAT.
> So the dependency does not look correct.
> The space savings also should be irrelevant.

Right, good catch. In the new syscall I had to do something similar[1],
to expose the 32-bit functions to 64-bit kernels as well, and not hide
them behind CONFIG_COMPAT.

[1]
https://lore.kernel.org/lkml/20251122-tonyk-robust_futex-v6-2-05fea005a0fd@igalia.com/

>
> The x86 series from Thomas does the same, maybe he will read this
> comment, otherwise I'll bring it up on his series, too.
>
>> +__u32 __vdso_futex_robust_list32_try_unlock(__u32 *lock, __u32 tid, __u32 *pop)
>> +{
>> + __u32 val, result;
>> +
>> + asm volatile (
>> + GLOBLS(32)
>> + " prfm pstl1strm, %[lock] \n"
>> + LABEL(start, 32)": \n"
>> + " ldxr %w[val], %[lock] \n"
>> + " cmp %w[tid], %w[val] \n"
>> + " bne " LABEL(end, 32)" \n"
>> + " stlxr %w[result], wzr, %w[lock] \n"
>> + " cbnz %w[result], " LABEL(start, 32)" \n"
>> + LABEL(success, 32)": \n"
>> + " str wzr, %w[pop] \n"
>> + LABEL(end, 32)": \n"
>> +
>> + : [val] "=&r" (val), [result] "=r" (result)
>> + : [tid] "r" (tid), [lock] "Q" (*lock), [pop] "Q" (*pop)
>> + : "memory"
>> + );
>> +
>> + return val;
>> +}
>> +#endif