Re: [PATCH liburing v5 2/2] test/splice: add basic splice tests

[Jens Axboe <axboe@kernel.dk>]

On 2/24/20 11:39 AM, Pavel Begunkov wrote:
> On 24/02/2020 21:33, Jens Axboe wrote:
>> On 2/24/20 11:26 AM, Pavel Begunkov wrote:
>>> On 24/02/2020 21:23, Jens Axboe wrote:
>>>> On 2/24/20 10:55 AM, Pavel Begunkov wrote:
>>>>> +static int copy_single(struct io_uring *ring,
>>>>> +			int fd_in, loff_t off_in,
>>>>> +			int fd_out, loff_t off_out,
>>>>> +			int pipe_fds[2],
>>>>> +			unsigned int len,
>>>>> +			unsigned flags1, unsigned flags2)
>>>>> +{
>>>>> +	struct io_uring_cqe *cqe;
>>>>> +	struct io_uring_sqe *sqe;
>>>>> +	int i, ret = -1;
>>>>> +
>>>>> +	sqe = io_uring_get_sqe(ring);
>>>>> +	if (!sqe) {
>>>>> +		fprintf(stderr, "get sqe failed\n");
>>>>> +		return -1;
>>>>> +	}
>>>>> +	io_uring_prep_splice(sqe, fd_in, off_in, pipe_fds[1], -1,
>>>>> +			     len, flags1);
>>>>> +	sqe->flags = IOSQE_IO_LINK;
>>>>> +
>>>>> +	sqe = io_uring_get_sqe(ring);
>>>>> +	if (!sqe) {
>>>>> +		fprintf(stderr, "get sqe failed\n");
>>>>> +		return -1;
>>>>> +	}
>>>>> +	io_uring_prep_splice(sqe, pipe_fds[0], -1, fd_out, off_out,
>>>>> +			     len, flags2);
>>>>> +
>>>>> +	ret = io_uring_submit(ring);
>>>>> +	if (ret <= 1) {
>>>>> +		fprintf(stderr, "sqe submit failed: %d\n", ret);
>>>>> +		return -1;
>>>>> +	}
>>>>
>>>> This seems wrong, you prep one and submit, the right return value would
>>>> be 1. This check should be < 1, not <= 1. I'll make the change, rest
>>>> looks good to me. Thanks!
>>>>
>>>
>>> There are 2 sqes, "fd_in -> pipe" and "pipe -> fd_out".
>>
>> I must be blind... I guess the failure case is that it doesn't work so well
>> on the kernels not supporting splice, as we'll return 1 there as the first
> 
> I've wanted for long to kill this weird behaviour, it should consume the whole
> link. Can't imagine any userspace app handling all edge-case errors right...

Yeah, for links it makes sense to error the chain, which would consume
the whole chain too.

>> submit fails. I'll clean up that bit.
> 
> ...I should have tested better. Thanks!

No worries, just trying to do better than we have in the best so we can
have some vague hope of having the test suite pass on older stable
kernels.

-- 
Jens Axboe