Re: [RFC PATCH] futex: Introduce __vdso_robust_futex_unlock

[Thomas Gleixner <tglx@kernel.org>]

On Wed, Mar 11 2026 at 14:54, Mathieu Desnoyers wrote:
> +u32 __vdso_robust_futex_unlock(u32 *uaddr, uintptr_t *op_pending_addr)
> +{
> +	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_WAKEUP) 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_WAKEUP) 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" (*op_pending_addr)
> +			: "memory");

TBH, all of this is completely overengineered and tasteless bloat.

The exactly same thing can be achieved by doing the obvious:

struct robust_list_head2 {
	struct robust_list_head		rhead;
        u32				unlock_val;
};

// User space
unlock(futex)
{
        struct robust_list_head2 *h = ....;

        h->unlock_val = 0;
        h->rhead.list_op_pending = .... | FUTEX_ROBUST_UNLOCK;

        xchg(futex->uval, h->unlock_val);

        if (h->unlock_val & FUTEX_WAITERS)
        	syscall(FUTEX, &futex->uval, FUTEX_WAKE, ....);

	h->rhead.list_op_pending = NULL;
}

And then the kernel robust list code does:

    	if (fetch_robust_entry(&pending, &head->list_op_pending, &pip))
        	return;

        if (pending & FUTEX_ROBUST_UNLOCK_PENDING) {
        	if (get_user(unlock_val, &head_v2->unlock_val))
                	return;
        }

        .....

        if (!pending)
        	return;

        /*
         * If userspace unlocked the futex already, but did not manage
         * to clear the pending pointer, then the futex is not longer
         * owned by the task and might have been freed already.
         *
         * As the dying task it not the owner anymore there is no need
         * to access the futex and to set the OWNERDEAD bit, just wake
         * up a waiter in case the task died before doing so.
         *
         * That wakeup might be spurious, but that's harmless as all
         * futex users must be able to handle spurious wake ups
         * correctly.
         */
        if (unlock_val) {
         	if (unlock_val & FUTEX_WAITERS)
                	futex_wake(pending + offset,....);
		return;
        }

No?

If you do it clever you can extend the existing code with minimally
intrusive changes.

But yeah, no ASM, no VDSO, no signal magic, no architecture EXTABLE
mess, no architecture specific hackery, too generic and not convoluted
enough, seriously?

And replying to your other mail right here:

> My aim is to use this vDSO as a replacement for atomic xchg and atomic
> cmpxchg within library code. I am trying to make the transition as
> straightforward as possible considering that this is a design bug
> fix.

Absolutely not for the price of creating a completely incomprehensible
and unjustified mess in the kernel when it can be done with a trivial
new interface, which just extends the existing one by the missing
functionality in a generic way.

Thanks,

        tglx