Re: [PATCH for-next 1/4] io_uring: if a linked request has REQ_F_FORCE_ASYNC then run it async

[Jens Axboe <axboe@kernel.dk>]

On 1/29/23 3:57 PM, Jens Axboe wrote:
> On 1/27/23 6:52?AM, Dylan Yudaken wrote:
>> REQ_F_FORCE_ASYNC was being ignored for re-queueing linked
>> requests. Instead obey that flag.
>>
>> Signed-off-by: Dylan Yudaken <dylany@meta.com>
>> ---
>>  io_uring/io_uring.c | 8 +++++---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
>> index db623b3185c8..980ba4fda101 100644
>> --- a/io_uring/io_uring.c
>> +++ b/io_uring/io_uring.c
>> @@ -1365,10 +1365,12 @@ void io_req_task_submit(struct io_kiocb *req, bool *locked)
>>  {
>>  	io_tw_lock(req->ctx, locked);
>>  	/* req->task == current here, checking PF_EXITING is safe */
>> -	if (likely(!(req->task->flags & PF_EXITING)))
>> -		io_queue_sqe(req);
>> -	else
>> +	if (unlikely(req->task->flags & PF_EXITING))
>>  		io_req_defer_failed(req, -EFAULT);
>> +	else if (req->flags & REQ_F_FORCE_ASYNC)
>> +		io_queue_iowq(req, locked);
>> +	else
>> +		io_queue_sqe(req);
>>  }
>>  
>>  void io_req_task_queue_fail(struct io_kiocb *req, int ret)
> 
> This one causes a failure for me with test/multicqes_drain.t, which
> doesn't quite make sense to me (just yet), but it is a reliable timeout.

OK, quick look and I think this is a bad assumption in the test case.
It's assuming that a POLL_ADD already succeeded, and hence that a
subsequent POLL_REMOVE will succeed. But now it's getting ENOENT as
we can't find it just yet, which means the cancelation itself isn't
being done. So we just end up waiting for something that doesn't happen.

Or could be an internal race with lookup/issue. In any case, it's
definitely being exposed by this patch.

-- 
Jens Axboe