Re: [PATCH RFC v2 1/2] arm64: vdso: Prepare for robust futex unlock support

André Almeida
2026-04-27T16:21:46Z

Em 26/04/2026 15:07, Thomas Weißschuh escreveu:
> Hi André,
>
> Some more comments, after doing an actual proper review.
>
> On 2026-04-24 15:56:00-0300, André Almeida wrote:
>> There will be a VDSO function to unlock non-contended robust futexes in
>> user space. The unlock sequence is racy vs. clearing the list_pending_op
>> pointer in the task's robust list head. To plug this race the kernel needs
>> to know the critical section window so it can clear the pointer when the
>> task is interrupted within that race window. The window is determined by
>> labels in the inline assembly.
>>
>> Signed-off-by: André Almeida <andrealmeid@igalia.com>
>> ---
>> Changes from v1:
>> - Fixed linker not finding VDSO symbols
>> ---
>> arch/arm64/kernel/vdso.c | 30 ++++++++++++++++++++++++++++++
>> arch/arm64/kernel/vdso/vdso.lds.S | 7 +++++++
>> 2 files changed, 37 insertions(+)
>
> What is the reason for splitting the series into two patches?
> To me it looks like it should be one patch.

I've followed how tglx split his series ("x86/vdso: Prepare for robust
futex unlock support", "x86/vdso: Implement
__vdso_futex_robust_try_unlock()"), but I don't have a strong opinion on
this matter, both options seems fine to me.

>
>> diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
>> index 592dd8668de4..f9c520a1c942 100644
>> --- a/arch/arm64/kernel/vdso.c
>> +++ b/arch/arm64/kernel/vdso.c
>> @@ -11,6 +11,7 @@
>> #include <linux/clocksource.h>
>> #include <linux/elf.h>
>> #include <linux/err.h>
>> +#include <linux/futex.h>
>> #include <linux/errno.h>
>> #include <linux/gfp.h>
>> #include <linux/kernel.h>
>> @@ -57,6 +58,33 @@ static struct vdso_abi_info vdso_info[] __ro_after_init = {
>> #endif /* CONFIG_COMPAT_VDSO */
>> };
>>
>> +#ifdef CONFIG_FUTEX_ROBUST_UNLOCK
>> +static void vdso_futex_robust_unlock_update_ips(enum vdso_abi abi, struct mm_struct *mm)
>> +{
>> + unsigned long vdso = (unsigned long) mm->context.vdso;
>> + struct futex_mm_data *fd = &mm->futex;
>> + uintptr_t success, end;
>> +
>> + if (abi == VDSO_ABI_AA64) {
>> + success = (uintptr_t) VDSO_SYMBOL(vdso, futex_list64_try_unlock_cs_success);
>> + end = (uintptr_t) VDSO_SYMBOL(vdso, futex_list64_try_unlock_cs_end);
>> +
>> + futex_set_vdso_cs_range(fd, 0, vdso, success, end, false);
>
> Both VDSO_SYMBOL() and futex_set_vdso_cs_range() add the vdso base
> address to the symbol offsets. The value stored in .start_ip will be
> wrong. The fact that futex_set_vdso_cs_range() does the addition looks
> like an artifact of it being written for x86 first. IMO its interface
> should be changed not to do the addition internally.
>

Got it, so for x86 we would need to explicitly add the base address on
the caller and remove from futex_set_vdso_cs_range()

>> + }
>> +
>> +#ifdef CONFIG_COMPAT_VDSO
>> + if (abi == VDSO_ABI_AA32) {
>> + success = (uintptr_t) VDSO_SYMBOL(vdso, futex_list32_try_unlock_cs_success);
>> + end = (uintptr_t) VDSO_SYMBOL(vdso, futex_list32_try_unlock_cs_end);
>> +
>> + futex_set_vdso_cs_range(fd, 1, vdso, success, end, true);
>> + }
>> +#endif
>> +}
>> +#else
>> +static inline void vdso_futex_robust_unlock_update_ips(enum vdso_abi abi, struct mm_struct *mm) { }
>> +#endif /* CONFIG_FUTEX_ROBUST_UNLOCK */
>> +
>> static int vdso_mremap(const struct vm_special_mapping *sm,
>> struct vm_area_struct *new_vma)
>> {
>
> (...)