Usama!
On Thu, Aug 20 2026 at 16:19, Usama Arif wrote:
> On 14/08/2026 17:02, Thomas Gleixner wrote:
>> --- 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);
>>
> 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.
Correct.
> Is it alright if I send v2 of the patch with your diff above? Or would you
> like to send it?
Go wild.
It lacks an __acquire(q->lock_ptr) annotation and a comment for this
trylock muck and the open coded hb_waiter_inc(). The latter can just
refer to futex_q_lock().
Also the exit condition should be simplified into one "if (ret || uval !=
val)", which removes four lines of code.
> Please let me know how you would like it attributed if I send it.
Suggested-by is fine.
Thanks,
tglx
