Re: [PATCH 3/5] io_uring: consider ring dead once the ref is marked dying

[Jens Axboe <axboe@kernel.dk>]

(getting back to this post the merge window)

On 3/21/25 3:22 PM, Pavel Begunkov wrote:
> On 3/21/25 19:24, Jens Axboe wrote:
>> Don't gate this on the task exiting flag. It's generally not a good idea
> 
> Do you refer to tw add and the PF_EXITING logic inside? We can't gate
> it solely on dying refs as it's not sync'ed (and the patch doesn't).
> And task is dying is not same as ring is closed. E.g. a task can
> exit(2) but leave the ring intact to other tasks.

It's not gated solely on dying refs, it's an addition.

>> to gate it on the task PF_EXITING flag anyway. Once the ring is starting
>> to go through ring teardown, the ref is marked as dying. Use that as
>> our fallback/cancel mechanism.
>>
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>> ---
>>   io_uring/io_uring.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
>> index 2b9dae588f04..984db01f5184 100644
>> --- a/io_uring/io_uring.c
>> +++ b/io_uring/io_uring.c
>> @@ -555,7 +555,8 @@ static void io_queue_iowq(struct io_kiocb *req)
>>        * procedure rather than attempt to run this request (or create a new
>>        * worker for it).
>>        */
>> -    if (WARN_ON_ONCE(!same_thread_group(tctx->task, current)))
>> +    if (WARN_ON_ONCE(!same_thread_group(tctx->task, current) &&
>> +             !percpu_ref_is_dying(&req->ctx->refs)))
> 
> Should it be "||"? Otherwise I don't understand the purpose of it.

Yep that's a braino, it should be:

if (!same_thread_group(req->task, current) ||
    io_ring_ref_is_dying(req->ctx))
	atomic_or(IO_WQ_WORK_CANCEL, &req->work.flags);

-- 
Jens Axboe