Re: [PATCH 3/3] io_uring/rw: add support for IORING_OP_READ_MULTISHOT

[Jens Axboe <axboe@kernel.dk>]

On 9/11/23 5:57 PM, Gabriel Krisman Bertazi wrote:
>> +int io_read_mshot(struct io_kiocb *req, unsigned int issue_flags)
>> +{
>> +	unsigned int cflags = 0;
>> +	int ret;
>> +
>> +	/*
>> +	 * Multishot MUST be used on a pollable file
>> +	 */
>> +	if (!file_can_poll(req->file))
>> +		return -EBADFD;
> 
> io_uring is pollable, so I think you want to also reject when
> req->file->f_ops == io_uring_fops to avoid the loop where a ring
> monitoring itself will cause a recursive completion? Maybe this can't
> happen here for some reason I miss?

I saw your followup, but we do actually handle that case - if this fd is
an io_uring context, then we track the inflight state of it so we can
appropriately cancel to break that loop.

But yeah, doesn't matter for this case, as you cannot read or write to
an io_uring fd in the first place.

>> +	ret = __io_read(req, issue_flags);
>> +
>> +	/*
>> +	 * If we get -EAGAIN, recycle our buffer and just let normal poll
>> +	 * handling arm it.
>> +	 */
>> +	if (ret == -EAGAIN) {
>> +		io_kbuf_recycle(req, issue_flags);
>> +		return -EAGAIN;
>> +	}
>> +
>> +	/*
>> +	 * Any error will terminate a multishot request
>> +	 */
>> +	if (ret <= 0) {
>> +finish:
>> +		io_req_set_res(req, ret, cflags);
>> +		if (issue_flags & IO_URING_F_MULTISHOT)
>> +			return IOU_STOP_MULTISHOT;
>> +		return IOU_OK;
> 
> Just a style detail, but I'd prefer to unfold this on the end of the function
> instead of jumping backwards here..

Sure, that might look better. I'll make the edit.

-- 
Jens Axboe