Re: futex+io_uring: futex_q::task can maybe be dangling (but is not actually accessed, so it's fine)

[Jens Axboe <axboe@kernel.dk>]

On 1/15/25 3:20 AM, Thomas Gleixner wrote:
> On Mon, Jan 13 2025 at 15:38, Peter Zijlstra wrote:
>> On Fri, Jan 10, 2025 at 08:33:34PM -0700, Jens Axboe wrote:
>>
>>> @@ -548,7 +549,7 @@ void __futex_queue(struct futex_q *q, struct futex_hash_bucket *hb)
>>>  
>>>  	plist_node_init(&q->list, prio);
>>>  	plist_add(&q->list, &hb->chain);
>>> -	q->task = current;
>>> +	q->task = task;
>>>  }
>>>  
>>>  /**
>>
>> The alternative is, I suppose, to move the q->task assignment out to
>> these two callsites instead. Thomas, any opinions?
> 
> That's fine as long as hb->lock is held, but the explicit argument makes
> all of this simpler to understand.
> 
> Though I'm not really a fan of this part:
> 
>> +		__futex_queue(&ifd->q, hb, NULL);
>> +		spin_unlock(&hb->lock);
> 
> Can we please add that @task argument to futex_queue() and keep the
> internals in the futex code instead of pulling more stuff into io_uring?

Sure, was trying to keep the change more minimal, but we can certainly
add it to futex_queue() instead rather than needing to work around it on
the io_uring side.

I'll be happy to send out a patch for that.

-- 
Jens Axboe