Re: [PATCH] io_uring/rw: transform single vector readv/writev into ubuf

[Jens Axboe <axboe@kernel.dk>]

On 3/24/23 4:41?PM, Ming Lei wrote:
> On Fri, Mar 24, 2023 at 08:35:38AM -0600, Jens Axboe wrote:
>> It's very common to have applications that use vectored reads or writes,
>> even if they only pass in a single segment. Obviously they should be
>> using read/write at that point, but...
> 
> Yeah, it is like fixing application issue in kernel side, :-)

It totally is, the same thing happens all of the time for readv as well.
No amount of informing or documenting will ever fix that...

Also see:

https://lore.kernel.org/linux-fsdevel/20230324204443.45950-1-axboe@kernel.dk/

with which I think I'll change this one to just be:

	if (iter->iter_type == ITER_UBUF) {
		rw->addr = iter->ubuf;
		rw->len = iter->count;
	/* readv -> read distance is the same as writev -> write */
	BUILD_BUG_ON((IORING_OP_READ - IORING_OP_READV) !=
			(IORING_OP_WRITE - IORING_OP_WRITEV));
		req->opcode += (IORING_OP_READ - IORING_OP_READV);
	}

instead.

We could also just skip it completely and just have liburing do the
right thing if io_uring_prep_readv/writev is called with nr_segs == 1.
Just turn it into a READ/WRITE at that point. If we do that, and with
the above generic change, it's probably Good Enough. If you use
READV/WRITEV and you're using the raw interface, then you're on your
own.

>> +	rw->addr = (unsigned long) iter->iov[0].iov_base;
>> +	rw->len = iter->iov[0].iov_len;
>> +	iov_iter_ubuf(iter, ddir, iter->iov[0].iov_base, rw->len);
>> +	/* readv -> read distance is the same as writev -> write */
>> +	BUILD_BUG_ON((IORING_OP_READ - IORING_OP_READV) !=
>> +			(IORING_OP_WRITE - IORING_OP_WRITEV));
>> +	req->opcode += (IORING_OP_READ - IORING_OP_READV);
> 
> It is a bit fragile to change ->opcode, which may need matched
> callbacks for the two OPs, also cause inconsistent opcode in traces.
> 
> I am wondering why not play the magic in io_prep_rw() from beginning?

It has to be done when importing the vec, we cannot really do it in
prep... Well we could, but that'd be adding a bunch more code and
duplicating part of the vec import.

-- 
Jens Axboe