Re: [PATCH 1/1] io_uring/rw: forbid multishot async reads

[Pavel Begunkov <asml.silence@gmail.com>]

On 2/17/25 15:57, Jens Axboe wrote:
> On 2/17/25 8:33 AM, Pavel Begunkov wrote:
>> On 2/17/25 15:06, Jens Axboe wrote:
>>> On 2/17/25 7:12 AM, Pavel Begunkov wrote:
>>>> On 2/17/25 13:58, Jens Axboe wrote:
>>>>> On 2/17/25 6:37 AM, Pavel Begunkov wrote:
>>>>>> At the moment we can't sanely handle queuing an async request from a
>>>>>> multishot context, so disable them. It shouldn't matter as pollable
>>>>>> files / socekts don't normally do async.
>>>>>
>>>>> Having something pollable that can return -EIOCBQUEUED is odd, but
>>>>> that's just a side comment.
>>>>>
>>>>>
>>>>>> diff --git a/io_uring/rw.c b/io_uring/rw.c
>>>>>> index 96b42c331267..4bda46c5eb20 100644
>>>>>> --- a/io_uring/rw.c
>>>>>> +++ b/io_uring/rw.c
>>>>>> @@ -878,7 +878,15 @@ static int __io_read(struct io_kiocb *req, unsigned int issue_flags)
>>>>>>         if (unlikely(ret))
>>>>>>             return ret;
>>>>>>     -    ret = io_iter_do_read(rw, &io->iter);
>>>>>> +    if (unlikely(req->opcode == IORING_OP_READ_MULTISHOT)) {
>>>>>> +        void *cb_copy = rw->kiocb.ki_complete;
>>>>>> +
>>>>>> +        rw->kiocb.ki_complete = NULL;
>>>>>> +        ret = io_iter_do_read(rw, &io->iter);
>>>>>> +        rw->kiocb.ki_complete = cb_copy;
>>>>>> +    } else {
>>>>>> +        ret = io_iter_do_read(rw, &io->iter);
>>>>>> +    }
>>>>>
>>>>> This looks a bit odd. Why can't io_read_mshot() just clear
>>>>> ->ki_complete?
>>>>
>>>> Forgot about that one, as for restoring it back, io_uring compares
>>>> or calls ->ki_complete in a couple of places, this way the patch
>>>> is more contained. It can definitely be refactored on top.
>>>
>>> I'd be tempted to do that for the fix too, the patch as-is is a
>>> bit of an eye sore... Hmm.
>>
>> It is an eyesore, sure, but I think a simple/concise eyesore is
>> better as a fix than having to change a couple more blocks across
>> rw.c. It probably wouldn't be too many changes, but I can't say
>> I'm concerned about this version too much as long as it can be
>> reshuffled later.
> 
> Sure, as discussed let's do a cleanup series on top. You'll send out
> a v2 with some improved commit message wording?

Yeah, I'll resend it a bit later

-- 
Pavel Begunkov