[PATCH liburing 0/7] run tests in parallel

[PATCH liburing 0/7] run tests in parallel

[Dylan Yudaken]

This series allows tests to be run in parallel, which speeds up
iterating. Rather than build this functionality into the shell scripts, it
seemed much easier to use make's parallel execution to do this.

My bash/make skills are not top notch, so I might have missed something
obvious, however it does seem to work locally very nicely.


Patch #1 is a bug that seems to trigger quite often when running in parallel
Patch #2-5 fix bugs that prevent tests running in parallel
Patch #6 adds a make target that depends on running each test
Patch #7 Is not related to parallel tests. It is a prep for a later series to use nop to test IOPOLL.

Dylan Yudaken (7):
  test: handle mmap return failures in pollfree test
  test: use unique path for socket
  test: use unique ports
  test: use unique filenames
  test: mkdir -p output folder
  test: add make targets for each test
  test: use remove_buffers instead of nop to generate error codes

 test/232c93d07b74.c    |  2 +-
 test/Makefile          | 10 +++++++++-
 test/accept-test.c     |  2 +-
 test/defer.c           | 28 ++++++++++++++++++++--------
 test/link.c            |  6 +++---
 test/openat2.c         |  6 +++---
 test/pollfree.c        |  4 ++--
 test/recv-msgall.c     |  2 +-
 test/runtests-quiet.sh | 10 ++++++++++
 test/runtests.sh       |  2 +-
 test/send_recv.c       |  2 +-
 test/send_recvmsg.c    |  2 +-
 test/sq-poll-dup.c     |  2 +-
 test/sq-poll-share.c   |  2 +-
 14 files changed, 55 insertions(+), 25 deletions(-)
 create mode 100755 test/runtests-quiet.sh


base-commit: b7d8dd8bbf5b8550c8a0c1ed70431cd8050709f0
-- 
2.30.2

[PATCH liburing 1/7] test: handle mmap return failures in pollfree test

[Dylan Yudaken]

Sometimes these mmap's fail, and it causes SEGFAULTS.
I assume this was accidentally left off when this was originally landed.

Signed-off-by: Dylan Yudaken <dylany@fb.com>
---
 test/pollfree.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/pollfree.c b/test/pollfree.c
index e2511df..d753ffe 100644
--- a/test/pollfree.c
+++ b/test/pollfree.c
@@ -406,10 +406,10 @@ int main(int argc, char *argv[])
   ret = mmap((void *)0x1ffff000ul, 0x1000ul, 0ul, 0x32ul, -1, 0ul);
   if (ret == MAP_FAILED)
     return 0;
-  mmap((void *)0x20000000ul, 0x1000000ul, 7ul, 0x32ul, -1, 0ul);
+  ret = mmap((void *)0x20000000ul, 0x1000000ul, 7ul, 0x32ul, -1, 0ul);
   if (ret == MAP_FAILED)
     return 0;
-  mmap((void *)0x21000000ul, 0x1000ul, 0ul, 0x32ul, -1, 0ul);
+  ret = mmap((void *)0x21000000ul, 0x1000ul, 0ul, 0x32ul, -1, 0ul);
   if (ret == MAP_FAILED)
     return 0;
   loop();
-- 
2.30.2

[PATCH liburing 2/7] test: use unique path for socket

[Dylan Yudaken]

Do not collide if multiple tests are running at once.

Signed-off-by: Dylan Yudaken <dylany@fb.com>
---
 test/accept-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/accept-test.c b/test/accept-test.c
index 71d9d80..4a904e4 100644
--- a/test/accept-test.c
+++ b/test/accept-test.c
@@ -35,7 +35,7 @@ int main(int argc, char *argv[])
 
 	memset(&addr, 0, sizeof(addr));
 	addr.sun_family = AF_UNIX;
-	memcpy(addr.sun_path, "\0sock", 6);
+	memcpy(addr.sun_path, "\0sock2", 7);
 
 	ret = bind(fd, (struct sockaddr *)&addr, addrlen);
 	assert(ret != -1);
-- 
2.30.2

[PATCH liburing 3/7] test: use unique ports

[Dylan Yudaken]

In preparation for running tests in parallel remove some collisions in
ports. In the future this should probably use ephemeral ports, but for now
there are not too many places to change.

Signed-off-by: Dylan Yudaken <dylany@fb.com>
---
 test/232c93d07b74.c | 2 +-
 test/recv-msgall.c  | 2 +-
 test/send_recv.c    | 2 +-
 test/send_recvmsg.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/test/232c93d07b74.c b/test/232c93d07b74.c
index 4153aef..8a7810b 100644
--- a/test/232c93d07b74.c
+++ b/test/232c93d07b74.c
@@ -26,7 +26,7 @@
 #define RECV_BUFF_SIZE 2
 #define SEND_BUFF_SIZE 3
 
-#define PORT	0x1235
+#define PORT	0x1234
 
 struct params {
 	int tcp;
diff --git a/test/recv-msgall.c b/test/recv-msgall.c
index 5f202b4..a6f7cfc 100644
--- a/test/recv-msgall.c
+++ b/test/recv-msgall.c
@@ -17,7 +17,7 @@
 
 #define MAX_MSG	128
 
-#define PORT	10200
+#define PORT	10201
 #define HOST	"127.0.0.1"
 
 static int recv_prep(struct io_uring *ring, struct iovec *iov, int *sock,
diff --git a/test/send_recv.c b/test/send_recv.c
index ad8ea0e..a7b001a 100644
--- a/test/send_recv.c
+++ b/test/send_recv.c
@@ -19,7 +19,7 @@ static char str[] = "This is a test of send and recv over io_uring!";
 
 #define MAX_MSG	128
 
-#define PORT	10200
+#define PORT	10202
 #define HOST	"127.0.0.1"
 
 static int recv_prep(struct io_uring *ring, struct iovec *iov, int *sock,
diff --git a/test/send_recvmsg.c b/test/send_recvmsg.c
index 43121f0..f607b5c 100644
--- a/test/send_recvmsg.c
+++ b/test/send_recvmsg.c
@@ -19,7 +19,7 @@ static char str[] = "This is a test of sendmsg and recvmsg over io_uring!";
 
 #define MAX_MSG	128
 
-#define PORT	10200
+#define PORT	10203
 #define HOST	"127.0.0.1"
 
 #define BUF_BGID	10
-- 
2.30.2

[PATCH liburing 4/7] test: use unique filenames

[Dylan Yudaken]

Allow tests to be run in parallel

Signed-off-by: Dylan Yudaken <dylany@fb.com>
---
 test/openat2.c       | 6 +++---
 test/sq-poll-dup.c   | 2 +-
 test/sq-poll-share.c | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/test/openat2.c b/test/openat2.c
index 379c61e..34c0f85 100644
--- a/test/openat2.c
+++ b/test/openat2.c
@@ -246,12 +246,12 @@ int main(int argc, char *argv[])
 	}
 
 	if (argc > 1) {
-		path = "/tmp/.open.close";
+		path = "/tmp/.open.at2";
 		path_rel = argv[1];
 		do_unlink = 0;
 	} else {
-		path = "/tmp/.open.close";
-		path_rel = ".open.close";
+		path = "/tmp/.open.at2";
+		path_rel = ".open.at2";
 		do_unlink = 1;
 	}
 
diff --git a/test/sq-poll-dup.c b/test/sq-poll-dup.c
index 0076a31..6a72b82 100644
--- a/test/sq-poll-dup.c
+++ b/test/sq-poll-dup.c
@@ -164,7 +164,7 @@ int main(int argc, char *argv[])
 	if (argc > 1) {
 		fname = argv[1];
 	} else {
-		fname = ".basic-rw";
+		fname = ".basic-rw-poll-dup";
 		t_create_file(fname, FILE_SIZE);
 	}
 
diff --git a/test/sq-poll-share.c b/test/sq-poll-share.c
index 2f1c1dd..7bb7626 100644
--- a/test/sq-poll-share.c
+++ b/test/sq-poll-share.c
@@ -82,7 +82,7 @@ int main(int argc, char *argv[])
 	if (argc > 1) {
 		fname = argv[1];
 	} else {
-		fname = ".basic-rw";
+		fname = ".basic-rw-poll-share";
 		t_create_file(fname, FILE_SIZE);
 	}
 
-- 
2.30.2

[PATCH liburing 5/7] test: mkdir -p output folder

[Dylan Yudaken]

In case multiple mkdir are running at once, do not log an error if it
exists due to a race condition.

Signed-off-by: Dylan Yudaken <dylany@fb.com>
---
 test/runtests.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/runtests.sh b/test/runtests.sh
index 122e482..6d8f7af 100755
--- a/test/runtests.sh
+++ b/test/runtests.sh
@@ -139,7 +139,7 @@ run_test()
 # Run all specified tests
 for tst in "${TESTS[@]}"; do
 	if [ ! -d output ]; then
-		mkdir output
+		mkdir -p output
 	fi
 	if [ -z "${TEST_MAP[$tst]}" ]; then
 		run_test "$tst"
-- 
2.30.2

[PATCH liburing 6/7] test: add make targets for each test

[Dylan Yudaken]

Add a make target runtests-parallel which can run tests in parallel.
This is very useful to quickly run all the tests locally with
  $ make -j runtests-parallel

Signed-off-by: Dylan Yudaken <dylany@fb.com>
---
 test/Makefile          | 10 +++++++++-
 test/runtests-quiet.sh | 10 ++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)
 create mode 100755 test/runtests-quiet.sh

diff --git a/test/Makefile b/test/Makefile
index cb7e15e..fe35ff9 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -185,6 +185,7 @@ all_targets += sq-full-cpp.t
 
 test_targets := $(patsubst %.c,%,$(test_srcs))
 test_targets := $(patsubst %.cc,%,$(test_targets))
+run_test_targets := $(patsubst %,%.run_test,$(test_targets))
 test_targets := $(patsubst %,%.t,$(test_targets))
 all_targets += $(test_targets)
 
@@ -229,4 +230,11 @@ runtests: all
 runtests-loop: all
 	@./runtests-loop.sh $(test_targets)
 
-.PHONY: all install clean runtests runtests-loop
+%.run_test: %.t
+	@./runtests-quiet.sh $<
+
+runtests-parallel: $(run_test_targets)
+	@echo "All tests passed"
+
+.PHONY: all install clean runtests runtests-loop runtests-parallel
+.PHONY += $(run_test_targets)
diff --git a/test/runtests-quiet.sh b/test/runtests-quiet.sh
new file mode 100755
index 0000000..ba9fe2b
--- /dev/null
+++ b/test/runtests-quiet.sh
@@ -0,0 +1,10 @@
+#!/usr/bin/env bash
+
+TESTS=("$@")
+RESULT_FILE=$(mktemp)
+./runtests.sh "${TESTS[@]}" 2>&1 > $RESULT_FILE
+RET="$?"
+if [ "${RET}" -ne 0 ]; then
+    cat $RESULT_FILE
+fi
+rm $RESULT_FILE
-- 
2.30.2

[PATCH liburing 7/7] test: use remove_buffers instead of nop to generate error codes

[Dylan Yudaken]

This is in prep for allwoing NOP to be used in IOPOLL mode. remove_buffers
will consistently return ENOENT if asked to remove buffers from a
nonexistent group, and so this is a suitable replacement. Other opcodes
return -EINVAL in IOPOLL from the prep stage, which has slightly different
behaviour.

Signed-off-by: Dylan Yudaken <dylany@fb.com>
---
 test/defer.c | 28 ++++++++++++++++++++--------
 test/link.c  |  6 +++---
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/test/defer.c b/test/defer.c
index 825b69f..68ee4b4 100644
--- a/test/defer.c
+++ b/test/defer.c
@@ -12,6 +12,10 @@
 #include "liburing.h"
 
 #define RING_SIZE 128
+enum {
+	OP_NOP,
+	OP_REMOVE_BUFFERS
+};
 
 struct test_context {
 	struct io_uring *ring;
@@ -27,7 +31,8 @@ static void free_context(struct test_context *ctx)
 	memset(ctx, 0, sizeof(*ctx));
 }
 
-static int init_context(struct test_context *ctx, struct io_uring *ring, int nr)
+static int init_context(struct test_context *ctx, struct io_uring *ring, int nr,
+			int op)
 {
 	struct io_uring_sqe *sqe;
 	int i;
@@ -45,7 +50,14 @@ static int init_context(struct test_context *ctx, struct io_uring *ring, int nr)
 		sqe = io_uring_get_sqe(ring);
 		if (!sqe)
 			goto err;
-		io_uring_prep_nop(sqe);
+		switch (op) {
+		case OP_NOP:
+			io_uring_prep_nop(sqe);
+			break;
+		case OP_REMOVE_BUFFERS:
+			io_uring_prep_remove_buffers(sqe, 10, 1);
+			break;
+		};
 		sqe->user_data = i;
 		ctx->sqes[i] = sqe;
 	}
@@ -81,7 +93,7 @@ static int test_cancelled_userdata(struct io_uring *ring)
 	struct test_context ctx;
 	int ret, i, nr = 100;
 
-	if (init_context(&ctx, ring, nr))
+	if (init_context(&ctx, ring, nr, OP_NOP))
 		return 1;
 
 	for (i = 0; i < nr; i++)
@@ -115,7 +127,7 @@ static int test_thread_link_cancel(struct io_uring *ring)
 	struct test_context ctx;
 	int ret, i, nr = 100;
 
-	if (init_context(&ctx, ring, nr))
+	if (init_context(&ctx, ring, nr, OP_REMOVE_BUFFERS))
 		return 1;
 
 	for (i = 0; i < nr; i++)
@@ -134,12 +146,12 @@ static int test_thread_link_cancel(struct io_uring *ring)
 		bool fail = false;
 
 		if (i == 0)
-			fail = (ctx.cqes[i].res != -EINVAL);
+			fail = (ctx.cqes[i].res != -ENOENT);
 		else
 			fail = (ctx.cqes[i].res != -ECANCELED);
 
 		if (fail) {
-			printf("invalid status\n");
+			printf("invalid status %d\n", ctx.cqes[i].res);
 			goto err;
 		}
 	}
@@ -158,7 +170,7 @@ static int test_drain_with_linked_timeout(struct io_uring *ring)
 	struct test_context ctx;
 	int ret, i;
 
-	if (init_context(&ctx, ring, nr * 2))
+	if (init_context(&ctx, ring, nr * 2, OP_NOP))
 		return 1;
 
 	for (i = 0; i < nr; i++) {
@@ -188,7 +200,7 @@ static int run_drained(struct io_uring *ring, int nr)
 	struct test_context ctx;
 	int ret, i;
 
-	if (init_context(&ctx, ring, nr))
+	if (init_context(&ctx, ring, nr, OP_NOP))
 		return 1;
 
 	for (i = 0; i < nr; i++)
diff --git a/test/link.c b/test/link.c
index c89d6b2..41d3899 100644
--- a/test/link.c
+++ b/test/link.c
@@ -178,7 +178,7 @@ static int test_single_link_fail(struct io_uring *ring)
 		goto err;
 	}
 
-	io_uring_prep_nop(sqe);
+	io_uring_prep_remove_buffers(sqe, 10, 1);
 	sqe->flags |= IOSQE_IO_LINK;
 
 	sqe = io_uring_get_sqe(ring);
@@ -205,8 +205,8 @@ static int test_single_link_fail(struct io_uring *ring)
 			printf("failed to get cqe\n");
 			goto err;
 		}
-		if (i == 0 && cqe->res != -EINVAL) {
-			printf("sqe0 failed with %d, wanted -EINVAL\n", cqe->res);
+		if (i == 0 && cqe->res != -ENOENT) {
+			printf("sqe0 failed with %d, wanted -ENOENT\n", cqe->res);
 			goto err;
 		}
 		if (i == 1 && cqe->res != -ECANCELED) {
-- 
2.30.2

Re: [PATCH liburing 6/7] test: add make targets for each test

[Ammar Faizi]

On 4/22/22 6:48 PM, Dylan Yudaken wrote:
> Add a make target runtests-parallel which can run tests in parallel.
> This is very useful to quickly run all the tests locally with
>    $ make -j runtests-parallel
> 
> Signed-off-by: Dylan Yudaken <dylany@fb.com>

Two comments below...

>   test/Makefile          | 10 +++++++++-
>   test/runtests-quiet.sh | 10 ++++++++++
>   2 files changed, 19 insertions(+), 1 deletion(-)
>   create mode 100755 test/runtests-quiet.sh

I suggest to add the following to the main Makefile:
```
runtests-parallel: all
	+$(MAKE) -C test runtests-parallel
```

So we can do this directly:
```
    make -j runtests-parallel;
```
instead of doing this:
```
    cd test;
    make -j runtests-parallel;
```

> -.PHONY: all install clean runtests runtests-loop
> +%.run_test: %.t
> +	@./runtests-quiet.sh $<
> +
> +runtests-parallel: $(run_test_targets)
> +	@echo "All tests passed"

Note that this parallel thing is doing:

    @./runtests-quiet.sh $THE_TEST_FILE

^ That thing is not a problem. But the ./runtests-quiet.sh exit code is.
> diff --git a/test/runtests-quiet.sh b/test/runtests-quiet.sh
> new file mode 100755
> index 0000000..ba9fe2b
> --- /dev/null
> +++ b/test/runtests-quiet.sh
> @@ -0,0 +1,10 @@
> +#!/usr/bin/env bash
> +
> +TESTS=("$@")
> +RESULT_FILE=$(mktemp)
> +./runtests.sh "${TESTS[@]}" 2>&1 > $RESULT_FILE
> +RET="$?"
> +if [ "${RET}" -ne 0 ]; then
> +    cat $RESULT_FILE
> +fi
> +rm $RESULT_FILE

This script's exit code doesn't necessarily represent the exit code of
the `./runtests.sh "${TESTS[@]}"`, so you have to add `exit $RET` at the
end of the script. Otherwise, the Makefile will always print "All tests
passed" even if we have tests failed.

-- 
Ammar Faizi

Re: [PATCH liburing 6/7] test: add make targets for each test

[Dylan Yudaken]

On Fri, 2022-04-22 at 20:40 +0700, Ammar Faizi wrote:
> On 4/22/22 6:48 PM, Dylan Yudaken wrote:
> > Add a make target runtests-parallel which can run tests in
> > parallel.
> > This is very useful to quickly run all the tests locally with
> >    $ make -j runtests-parallel
> > 
> > Signed-off-by: Dylan Yudaken <dylany@fb.com>
> 
> Two comments below...
> 
> >   test/Makefile          | 10 +++++++++-
> >   test/runtests-quiet.sh | 10 ++++++++++
> >   2 files changed, 19 insertions(+), 1 deletion(-)
> >   create mode 100755 test/runtests-quiet.sh
> 
> I suggest to add the following to the main Makefile:
> ```
> runtests-parallel: all
>         +$(MAKE) -C test runtests-parallel
> ```
> 
> So we can do this directly:
> ```
>     make -j runtests-parallel;
> ```
> instead of doing this:
> ```
>     cd test;
>     make -j runtests-parallel;
> ```
> 
> > -.PHONY: all install clean runtests runtests-loop
> > +%.run_test: %.t
> > +       @./runtests-quiet.sh $<
> > +
> > +runtests-parallel: $(run_test_targets)
> > +       @echo "All tests passed"
> 
> Note that this parallel thing is doing:
> 
>     @./runtests-quiet.sh $THE_TEST_FILE
> 
> ^ That thing is not a problem. But the ./runtests-quiet.sh exit code
> is.
> > diff --git a/test/runtests-quiet.sh b/test/runtests-quiet.sh
> > new file mode 100755
> > index 0000000..ba9fe2b
> > --- /dev/null
> > +++ b/test/runtests-quiet.sh
> > @@ -0,0 +1,10 @@
> > +#!/usr/bin/env bash
> > +
> > +TESTS=("$@")
> > +RESULT_FILE=$(mktemp)
> > +./runtests.sh "${TESTS[@]}" 2>&1 > $RESULT_FILE
> > +RET="$?"
> > +if [ "${RET}" -ne 0 ]; then
> > +    cat $RESULT_FILE
> > +fi
> > +rm $RESULT_FILE
> 
> This script's exit code doesn't necessarily represent the exit code
> of
> the `./runtests.sh "${TESTS[@]}"`, so you have to add `exit $RET` at
> the
> end of the script. Otherwise, the Makefile will always print "All
> tests
> passed" even if we have tests failed.
> 

Both of these are good ideas - thanks! The second one especially is a
great spot. Will respin a v2 with them

Dylan