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

[Jens Axboe <axboe@kernel.dk>]

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?

-- 
Jens Axboe