[PATCH v5 0/2] splice helpers + tests

[PATCH v5 0/2] splice helpers + tests

[Pavel Begunkov]

Add splice prep helpers and some basic tests.

P.S. I haven't tried the skip part with splice-less kernel

v2: minor: fix comment, add inline, remove newline

v3: type changes (Stefan Metzmacher)
    update io_uring.h

v4: rebase

v5: skip if not supported (Jens)
    print into stderr (Jens)

Pavel Begunkov (2):
  splice: add splice(2) helpers
  test/splice: add basic splice tests

 src/include/liburing.h          |  12 +++
 src/include/liburing/io_uring.h |  14 ++-
 test/Makefile                   |   5 +-
 test/splice.c                   | 148 ++++++++++++++++++++++++++++++++
 4 files changed, 176 insertions(+), 3 deletions(-)
 create mode 100644 test/splice.c

-- 
2.24.0

[PATCH liburing v5 1/2] splice: add splice(2) helpers

[Pavel Begunkov]

Add splice helpers and update io_uring.h

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 src/include/liburing.h          | 12 ++++++++++++
 src/include/liburing/io_uring.h | 14 +++++++++++++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/src/include/liburing.h b/src/include/liburing.h
index b8bcdf5..7edd4f1 100644
--- a/src/include/liburing.h
+++ b/src/include/liburing.h
@@ -191,6 +191,18 @@ static inline void io_uring_prep_rw(int op, struct io_uring_sqe *sqe, int fd,
 	sqe->__pad2[0] = sqe->__pad2[1] = sqe->__pad2[2] = 0;
 }
 
+static inline void io_uring_prep_splice(struct io_uring_sqe *sqe,
+					int fd_in, loff_t off_in,
+					int fd_out, loff_t off_out,
+					unsigned int nbytes,
+					unsigned int splice_flags)
+{
+	io_uring_prep_rw(IORING_OP_SPLICE, sqe, fd_out, (void *)off_in,
+			 nbytes, off_out);
+	sqe->splice_fd_in = fd_in;
+	sqe->splice_flags = splice_flags;
+}
+
 static inline void io_uring_prep_readv(struct io_uring_sqe *sqe, int fd,
 				       const struct iovec *iovecs,
 				       unsigned nr_vecs, off_t offset)
diff --git a/src/include/liburing/io_uring.h b/src/include/liburing/io_uring.h
index 7e7c2d9..a3d5e25 100644
--- a/src/include/liburing/io_uring.h
+++ b/src/include/liburing/io_uring.h
@@ -23,7 +23,10 @@ struct io_uring_sqe {
 		__u64	off;	/* offset into file */
 		__u64	addr2;
 	};
-	__u64	addr;		/* pointer to buffer or iovecs */
+	union {
+		__u64	addr;	/* pointer to buffer or iovecs */
+		__u64	splice_off_in;
+	};
 	__u32	len;		/* buffer size or number of iovecs */
 	union {
 		__kernel_rwf_t	rw_flags;
@@ -37,6 +40,7 @@ struct io_uring_sqe {
 		__u32		open_flags;
 		__u32		statx_flags;
 		__u32		fadvise_advice;
+		__u32		splice_flags;
 	};
 	__u64	user_data;	/* data to be passed back at completion time */
 	union {
@@ -45,6 +49,7 @@ struct io_uring_sqe {
 			__u16	buf_index;
 			/* personality to use, if used */
 			__u16	personality;
+			__s32	splice_fd_in;
 		};
 		__u64	__pad2[3];
 	};
@@ -113,6 +118,7 @@ enum {
 	IORING_OP_RECV,
 	IORING_OP_OPENAT2,
 	IORING_OP_EPOLL_CTL,
+	IORING_OP_SPLICE,
 
 	/* this goes last, obviously */
 	IORING_OP_LAST,
@@ -128,6 +134,12 @@ enum {
  */
 #define IORING_TIMEOUT_ABS	(1U << 0)
 
+/*
+ * sqe->splice_flags
+ * extends splice(2) flags
+ */
+#define SPLICE_F_FD_IN_FIXED	(1U << 31) /* the last bit of __u32 */
+
 /*
  * IO completion data structure (Completion Queue Entry)
  */
-- 
2.24.0

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

[Pavel Begunkov]

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 test/Makefile |   5 +-
 test/splice.c | 148 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 151 insertions(+), 2 deletions(-)
 create mode 100644 test/splice.c

diff --git a/test/Makefile b/test/Makefile
index 09c7aa2..8437f31 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -20,7 +20,7 @@ all_targets += poll poll-cancel ring-leak fsync io_uring_setup io_uring_register
 		connect 7ad0e4b2f83c-test submit-reuse fallocate open-close \
 		file-update statx accept-reuse poll-v-poll fadvise madvise \
 		short-read openat2 probe shared-wq personality eventfd \
-		send_recv eventfd-ring across-fork sq-poll-kthread
+		send_recv eventfd-ring across-fork sq-poll-kthread splice
 
 include ../Makefile.quiet
 
@@ -47,7 +47,8 @@ test_srcs := poll.c poll-cancel.c ring-leak.c fsync.c io_uring_setup.c \
 	7ad0e4b2f83c-test.c submit-reuse.c fallocate.c open-close.c \
 	file-update.c statx.c accept-reuse.c poll-v-poll.c fadvise.c \
 	madvise.c short-read.c openat2.c probe.c shared-wq.c \
-	personality.c eventfd.c eventfd-ring.c across-fork.c sq-poll-kthread.c
+	personality.c eventfd.c eventfd-ring.c across-fork.c sq-poll-kthread.c \
+	splice.c
 
 test_objs := $(patsubst %.c,%.ol,$(test_srcs))
 
diff --git a/test/splice.c b/test/splice.c
new file mode 100644
index 0000000..1eaa788
--- /dev/null
+++ b/test/splice.c
@@ -0,0 +1,148 @@
+#include <errno.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <string.h>
+#include <fcntl.h>
+#include <sys/mman.h>
+
+#include "liburing.h"
+
+static int no_splice = 0;
+
+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;
+	}
+
+	for (i = 0; i < 2; i++) {
+		ret = io_uring_wait_cqe(ring, &cqe);
+		if (ret < 0) {
+			fprintf(stderr, "wait completion %d\n", cqe->res);
+			return ret;
+		}
+
+		ret = cqe->res;
+		if (ret != len) {
+			fprintf(stderr, "splice: returned %i, expected %i\n",
+				cqe->res, len);
+			return ret < 0 ? ret : -1;
+		}
+		io_uring_cqe_seen(ring, cqe);
+	}
+	return 0;
+}
+
+static int test_splice(struct io_uring *ring)
+{
+	int ret = -1, len = 4 * 4096;
+	int fd_out = -1, fd_in = -1;
+	int pipe_fds[2] = {-1, -1};
+
+	if (pipe(pipe_fds) < 0)
+		goto exit;
+	fd_in = open("/dev/urandom", O_RDONLY);
+	if (fd_in < 0)
+		goto exit;
+	fd_out = memfd_create("splice_test_out_file", 0);
+	if (fd_out < 0)
+		goto exit;
+	if (ftruncate(fd_out, len) == -1)
+		goto exit;
+
+	ret = copy_single(ring, fd_in, -1, fd_out, -1, pipe_fds,
+			  len, SPLICE_F_MOVE | SPLICE_F_MORE, 0);
+	if (ret == -EINVAL) {
+		no_splice = 1;
+		goto exit;
+	}
+	if (ret) {
+		fprintf(stderr, "basic splice-copy failed\n");
+		goto exit;
+	}
+
+	ret = copy_single(ring, fd_in, 0, fd_out, 0, pipe_fds,
+			  len, 0, SPLICE_F_MOVE | SPLICE_F_MORE);
+	if (ret) {
+		fprintf(stderr, "basic splice with offset failed\n");
+		goto exit;
+	}
+
+	ret = io_uring_register_files(ring, &fd_in, 1);
+	if (ret) {
+		fprintf(stderr, "%s: register ret=%d\n", __FUNCTION__, ret);
+		goto exit;
+	}
+
+	ret = copy_single(ring, 0, 0, fd_out, 0, pipe_fds,
+			  len, SPLICE_F_FD_IN_FIXED, 0);
+	if (ret) {
+		fprintf(stderr, "basic splice with reg files failed\n");
+		goto exit;
+	}
+
+	ret = 0;
+exit:
+	if (fd_out >= 0)
+		close(fd_out);
+	if (fd_in >= 0)
+		close(fd_in);
+	if (pipe_fds[0] >= 0) {
+		close(pipe_fds[0]);
+		close(pipe_fds[1]);
+	}
+	return ret;
+}
+
+int main(int argc, char *argv[])
+{
+	struct io_uring ring;
+	int ret;
+
+	ret = io_uring_queue_init(8, &ring, 0);
+	if (ret) {
+		fprintf(stderr, "ring setup failed\n");
+		return 1;
+	}
+
+	ret = test_splice(&ring);
+	if (ret && no_splice) {
+		fprintf(stderr, "skip, doesn't support splice()\n");
+		return 0;
+	}
+	if (ret) {
+		fprintf(stderr, "test_splice failed %i %i\n", ret, errno);
+		return ret;
+	}
+
+	return 0;
+}
-- 
2.24.0

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

[Jens Axboe]

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!

-- 
Jens Axboe

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

[Pavel Begunkov]

[-- Attachment #1.1: Type: text/plain, Size: 1285 bytes --]

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".

-- 
Pavel Begunkov


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Jens Axboe]

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
submit fails. I'll clean up that bit.

-- 
Jens Axboe

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

[Pavel Begunkov]

[-- Attachment #1.1: Type: text/plain, Size: 1862 bytes --]

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...

> submit fails. I'll clean up that bit.

...I should have tested better. Thanks!

-- 
Pavel Begunkov


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Jens Axboe]

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

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

[Pavel Begunkov]

[-- Attachment #1.1: Type: text/plain, Size: 842 bytes --]

On 24/02/2020 21:41, Jens Axboe wrote:
>> 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.

Have you gave a thought to using C++ for testing? It would be really nice to
have some flexible test generator removing boilerplate and allowing
automatically try different flags combinations. It sounds like a lot of pain
doing this in old plain C.

-- 
Pavel Begunkov


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Jens Axboe]

On 2/24/20 11:47 AM, Pavel Begunkov wrote:
> On 24/02/2020 21:41, Jens Axboe wrote:
>>> 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.
> 
> Have you gave a thought to using C++ for testing? It would be really nice to
> have some flexible test generator removing boilerplate and allowing
> automatically try different flags combinations. It sounds like a lot of pain
> doing this in old plain C.

I'm not a c++ guy, but if it's useful, then I'm open to anything.

-- 
Jens Axboe