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

[André Almeida]

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