Re: [RFC PATCH v4] futex: Introduce __vdso_robust_futex_unlock and __vdso_robust_pi_futex_try_unlock

[Mathieu Desnoyers <mathieu.desnoyers@efficios.com>]

On 2026-03-13 10:24, Thomas Weißschuh wrote:
> Hi Mathieu,
> 
> some small remarks around the vDSO code.
> 
> On Fri, Mar 13, 2026 at 09:39:03AM -0400, Mathieu Desnoyers wrote:
> 
> (...)
> 
>> The approach taken to fix this is to introduce two vDSO and extend the
>> x86 vDSO exception table to track the relevant ip ranges: one for non-PI
>> robust futexes, and one for PI robust futexes.
> 
> One of the central points behind the vDSO so far was that it is only a
> performance optimization. It is never required for correctness.
> What are applications supposed to do when the vDSO is disabled?

Good point!

For non-PI futexes, we'd need to introduce a new robust_futex_unlock
system call to handle the scenario where vDSO is unavailable. This
system call would perform the equivalent of what is done within the
vDSO, but from within the kernel. This comes with a significant
performance overhead, as this would be called on _every_ robust
futex unlock.

For PI futexes, applications could either directly invoke the
futex_unlock_pi system call when the vDSO is not available, or
we could provide a new system call which does the equivalent of
the vDSO within the kernel. The advantage of the new system call
is that we would not hit the hb->lock in the uncontended case.
Another alternative is that we tweak the existing futex_unlock_pi
to handle the case where it is called when there are no waiters more
efficiently.

> 
> (...)
> 
>> diff --git a/arch/x86/entry/vdso/common/vfutex.c b/arch/x86/entry/vdso/common/vfutex.c
>> new file mode 100644
>> index 000000000000..336095b04952
>> --- /dev/null
>> +++ b/arch/x86/entry/vdso/common/vfutex.c
>> @@ -0,0 +1,90 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (C) 2026 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> + */
>> +#include <linux/types.h>
>> +#include <linux/futex.h>
> 
> This should be uapi/linux/futex.h. Headers from the linux/ namespace should
> not be used in vDSO code. The definitions from them may end up being wrong
> in the compat vDSO. Either use uapi/ or vdso/ headers. (linux/types.h is a bit
> of an exception for historic reasons, it could be replaced by uapi/linux/types.h)

OK, will fix.

> 
>> +#include <vdso/futex.h>
>> +#include "extable.h"
> 
>> +
>> +#ifdef CONFIG_X86_64
> 
> This only works because of the ugly hacks in fake_32bit_build.h.
> Testing for '#ifdef __x86_64__' is simpler and nicer to read.

OK, will fix.

> 
>> +# define ASM_PTR_BIT_SET	"btsq "
>> +# define ASM_PTR_SET		"movq "
>> +#else
>> +# define ASM_PTR_BIT_SET	"btsl "
>> +# define ASM_PTR_SET		"movl "
>> +#endif
>> +
>> +u32 __vdso_robust_futex_unlock(u32 *uaddr, struct robust_list_head *robust_list_head)
>> +{
>> +	u32 val = 0;
>> +
>> +	/*
>> +	 * Within the ip range identified by the futex exception table,
>> +	 * the register "eax" contains the value loaded by xchg. This is
>> +	 * expected by futex_vdso_exception() to check whether waiters
>> +	 * need to be woken up. This register state is transferred to
>> +	 * bit 1 (NEED_ACTION) of *op_pending_addr before the ip range
>> +	 * ends.
>> +	 */
>> +	asm volatile (
>> +		_ASM_VDSO_EXTABLE_FUTEX_HANDLE(1f, 3f)
>> +		/* Exchange uaddr (store-release). */
>> +		"xchg %[uaddr], %[val]\n\t"
>> +		"1:\n\t"
>> +		/* Test if FUTEX_WAITERS (0x80000000) is set. */
>> +		"test %[val], %[val]\n\t"
>> +		"js 2f\n\t"
>> +		/* Clear *op_pending_addr if there are no waiters. */
>> +		ASM_PTR_SET "$0, %[op_pending_addr]\n\t"
>> +		"jmp 3f\n\t"
>> +		"2:\n\t"
>> +		/* Set bit 1 (NEED_ACTION) in *op_pending_addr. */
>> +		ASM_PTR_BIT_SET "$1, %[op_pending_addr]\n\t"
>> +		"3:\n\t"
>> +		: [val] "+a" (val),
>> +		  [uaddr] "+m" (*uaddr)
>> +		: [op_pending_addr] "m" (robust_list_head->list_op_pending)
>> +		: "memory"
>> +	);
>> +	return val;
>> +}
>> +
>> +u32 robust_futex_unlock(u32 *, struct robust_list_head *)
>> +	__attribute__((weak, alias("__vdso_robust_futex_unlock")));
> 
> __weak and __alias() as per checkpatch.pl.

OK, will fix.

> 
> The entries in the linkerscripts are missing.

Good catch, will fix for:

vdso/vdso64/vdsox32.lds.S
vdso/vdso64/vdso64.lds.S
vdso/vdso32/vdso32.lds.S

> 
> (...)
> 
>> --- /dev/null
>> +++ b/include/vdso/futex.h
>> @@ -0,0 +1,72 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Copyright (C) 2026 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> + */
>> +
>> +#ifndef _VDSO_FUTEX_H
>> +#define _VDSO_FUTEX_H
>> +
>> +#include <linux/types.h>
>> +#include <linux/futex.h>
> 
> Same remarks about the linux/ namespace as before.

OK, fixed.

> 
>> +/**
>> + * __vdso_robust_futex_unlock - Architecture-specific vDSO implementation of robust futex unlock.
>> + * @uaddr:		Lock address (points to a 32-bit unsigned integer type).
>> + * @robust_list_head:	The thread-specific robust list that has been registered with set_robust_list.
>> + *
>> + * This vDSO unlocks the robust futex by exchanging the content of
>> + * *@uaddr with 0 with a store-release semantic. If the futex has
>> + * waiters, it sets bit 1 of *@robust_list_head->list_op_pending, else
>> + * it clears *@robust_list_head->list_op_pending. Those operations are
>> + * within a code region known by the kernel, making them safe with
>> + * respect to asynchronous program termination either from thread
>> + * context or from a nested signal handler.
>> + *
>> + * Returns:	The old value present at *@uaddr.
>> + *
>> + * Expected use of this vDSO:
>> + *
>> + * robust_list_head is the thread-specific robust list that has been
>> + * registered with set_robust_list.
>> + *
>> + * if ((__vdso_robust_futex_unlock((u32 *) &mutex->__data.__lock, robust_list_head)
>> + *     & FUTEX_WAITERS) != 0)
>> + *         futex_wake((u32 *) &mutex->__data.__lock, 1, private);
>> + * WRITE_ONCE(robust_list_head->list_op_pending, 0);
>> + */
>> +extern u32 __vdso_robust_futex_unlock(u32 *uaddr, struct robust_list_head *robust_list_head);
> 
> Drop the extern.

OK, fixed.

Thanks,

Mathieu

> 
> (...)
> 
>> +#endif /* _VDSO_FUTEX_H */
> 
> (...)


-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com