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

["André Almeida" <andrealmeid@igalia.com>]

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)
>>   {
> 
> (...)