Re: [PATCH] futex: Avoid hash-bucket locking for mismatched waits

On 14/08/2026 17:02, Thomas Gleixner wrote:
> On Fri, Aug 14 2026 at 18:01, Thomas Gleixner wrote:
>> On Mon, Aug 10 2026 at 13:17, Usama Arif wrote:
>>> On 07/08/2026 16:42, Thomas Gleixner wrote:
>>>> On Wed, Aug 05 2026 at 06:28, Usama Arif wrote:
>>>>> On Tue, 4 Aug 2026 17:07:59 +0000 Dmitry Ilvokhin <[email protected]> wrote:
>>>>> The above data shows the significance of the patch.
>>>>> It provides a very meaningful improvement (22.4% of time spent in futex_q_lock()
>>>>> will be significantly optimized and will also deliver second-order effects)
>>>>> and has no measurable impact on latency in the matching path.
>>>>> IMHO, this patch is a free lunch.
>>>>
>>>> Not really free. The user space access is not exactly cheap either
>>>> because CLAC/STAC are memory fencing to meet the SMAP guarantees.
>>>
>>> My understanding from 86e6b1547b3d is that STAC/CLAC “end up serializing
>>> execution on older Zen,” while Zen 5’s AC renaming “improves performance
>>> of STAC/CLAC a lot a lot.”  Architecturally, they only change the AC bit.
>>> They are not memory-ordering instructions like LFENCE.
>>
>> It's not a memory ordering instruction, but it has to guarantee that the
>> AC change is effective when the subsequent permission check
>> happens. That's true for both STAC and CLAC.
>>
>> So it _cannot_ be free by definition and the penalty depends on the
>> micro architecture.
>>
>>> I am currently testing on Zen5 which could be why I didn't see any
>>> wall-time regression in futex_wait_timeout.c from [1].
>>
>> It's not relevant whether your ZEN5 works fine or not. We are not
>> optimizing for a particular machine.
>>
>> A trivial futex bouncing test case with two threads degrades on a ZEN3
>> by ~20% and when looking at it with perf top clearly the extra user
>> access stands out very prominently.
>>
>> The below variant does not expose that behavior and actually improves
>> the same test case by ~5% on that machine.
> 
> Bah. Included the broken version. Fixed one is below.
> 
> Thanks,
> 
>         tglx
> ---
>  kernel/futex/waitwake.c |   16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> --- a/kernel/futex/waitwake.c
> +++ b/kernel/futex/waitwake.c
> @@ -857,7 +857,21 @@ int futex_wait_setup(u32 __user *uaddr,
>  		CLASS(hbr, hbr)(&q->key);
>  		auto hb = hbr.hb;
>  
> -		futex_q_lock(q, hb);
> +		futex_hb_waiters_inc(hb);
> +		q->lock_ptr = &hb->lock;
> +
> +		if (!spin_trylock(&hb->lock)) {
> +			ret = get_user_inline(uval, uaddr);
> +			if (ret) {
> +				futex_hb_waiters_dec(hb);
> +				return ret;
> +			}
> +			if (uval != val) {
> +				futex_hb_waiters_dec(hb);
> +				return -EWOULDBLOCK;
> +			}
> +			spin_lock(&hb->lock);
> +		}
>  
>  		ret = futex_get_value_locked(&uval, uaddr);
>  

Hi Thomas,

Sorry for the late reply.

I like this approach; it makes the trade-off much better than my patch.
When spin_trylock() succeeds, the matching path performs only the existing 
single user access, rather than the two accesses in my version.

Is it alright if I send v2 of the patch with your diff above? Or would you
like to send it?
Please let me know how you would like it attributed if I send it.

Thanks!
Usama