[PATCH liburing v1 0/6] liburing fixes and cleanups

[PATCH liburing v1 0/6] liburing fixes and cleanups

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Hi Jens,

Small liburing fixes and cleanups this time:

- Patch 1 and 2 are a fix for the recent updates from Dylan.
- Patch 3 and 4 are a Makefile cleanup.
- Patch 5 is an update for the GitHub bot to build x86 32-bit nolibc.
- Patch 6 is a workaround for random SIGSEGV error on test/double-poll-crash.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---

Ammar Faizi (6):
  test/runtests-quiet.sh: Fixup redirection
  Makefile: Make sure we build everything before runtests
  test/Makefile: Remove `.PHONY` variable
  test/Makefile: Sort the test file list alphabetically
  .github/workflows: Run the nolibc build for x86 32-bit
  test/double-poll-crash: Skip this test if the `mmap()` fails

 .github/workflows/build.yml |  2 +-
 Makefile                    |  4 ++--
 test/Makefile               |  4 ++--
 test/double-poll-crash.c    | 13 +++++++++++--
 test/runtests-quiet.sh      |  2 +-
 5 files changed, 17 insertions(+), 8 deletions(-)


base-commit: 770efd14e8b17ccf23a45c95ecc9d38de4e17011
-- 
Ammar Faizi

[PATCH liburing v1 1/6] test/runtests-quiet.sh: Fixup redirection

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

1) command > file.txt

will redirect the stdout to file.txt.

2) command > file.txt 2>&1

will redirect the stdout and stderr to file.txt.

What we want is (2). Previous commits placed the "2>&1" wrong.

Cc: Dylan Yudaken <dylany@fb.com>
Fixes: 770efd14e8b17ccf23a45c95ecc9d38de4e17011 ("test/runtests-quiet.sh: fixup redirection")
Fixes: 6480f692d62afbebb088febc369b30a63dbc2ea7 ("test: add make targets for each test")
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 test/runtests-quiet.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/runtests-quiet.sh b/test/runtests-quiet.sh
index 2b84b9f..2bc7da0 100755
--- a/test/runtests-quiet.sh
+++ b/test/runtests-quiet.sh
@@ -2,7 +2,7 @@
 
 TESTS=("$@")
 RESULT_FILE=$(mktemp)
-{ ./runtests.sh "${TESTS[@]}"; } 2>&1 > "$RESULT_FILE"
+./runtests.sh "${TESTS[@]}" > "$RESULT_FILE" 2>&1
 RET="$?"
 if [ "${RET}" -ne 0 ]; then
     cat "$RESULT_FILE"
-- 
Ammar Faizi

[PATCH liburing v1 2/6] Makefile: Make sure we build everything before runtests

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

A recent commit added `runtests-parallel` target to run the tests in
parallel. But it doesn't have a proper build dependency.

As such, when doing:

  make -j clean;
  make -j runtests-parallel;

we got this error:

```
  make[1]: Entering directory '/home/ammarfaizi2/app/liburing/test'
  make[1]: *** No rule to make target '232c93d07b74.t', needed by '232c93d07b74.run_test'.  Stop.
  make[1]: Leaving directory '/home/ammarfaizi2/app/liburing/test'
  make: *** [Makefile:25: runtests-parallel] Error 2
```

Add `all` target as the dependency of `runtests-parallel`. While in
there, I found the same issue on `runtests-loop` target. Do the same
thing for it too. This way the main Makefile will build everything
first before trying to execute the tests.

Cc: Dylan Yudaken <dylany@fb.com>
Fixes: 4fb3c9e9c737c2cf2d4df4e1972e68d596a626f7 ("Add runtests-loop target")
Fixes: 6480f692d62afbebb088febc369b30a63dbc2ea7 ("test: add make targets for each test")
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index d54551e..686be4f 100644
--- a/Makefile
+++ b/Makefile
@@ -19,9 +19,9 @@ partcheck: all
 
 runtests: all
 	@$(MAKE) -C test runtests
-runtests-loop:
+runtests-loop: all
 	@$(MAKE) -C test runtests-loop
-runtests-parallel:
+runtests-parallel: all
 	@$(MAKE) -C test runtests-parallel
 
 config-host.mak: configure
-- 
Ammar Faizi

[PATCH liburing v1 3/6] test/Makefile: Remove `.PHONY` variable

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

What we want is a .PHONY build target that depends on runtests-parallel,
not a .PHONY variable. This variable doesn't do anything. Remove it.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 test/Makefile | 1 -
 1 file changed, 1 deletion(-)

diff --git a/test/Makefile b/test/Makefile
index fe35ff9..444c749 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -237,4 +237,3 @@ runtests-parallel: $(run_test_targets)
 	@echo "All tests passed"
 
 .PHONY: all install clean runtests runtests-loop runtests-parallel
-.PHONY += $(run_test_targets)
-- 
Ammar Faizi

[PATCH liburing v1 4/6] test/Makefile: Sort the test file list alphabetically

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Make it easy to manage and find by sorting it alphabetically. Also, add
a comment to remind us to keep it sorted alphabetically.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 test/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/test/Makefile b/test/Makefile
index 444c749..923f984 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -33,6 +33,7 @@ override CXXFLAGS += $(XCFLAGS) -std=c++11 -DLIBURING_BUILD_TEST
 LDFLAGS ?=
 override LDFLAGS += -L../src/ -luring -lpthread
 
+# Please keep this list sorted alphabetically.
 test_srcs := \
 	232c93d07b74.c \
 	35fa71a030ca.c \
@@ -133,6 +134,7 @@ test_srcs := \
 	short-read.c \
 	shutdown.c \
 	sigfd-deadlock.c \
+	skip-cqe.c \
 	socket-rw.c \
 	socket-rw-eagain.c \
 	socket-rw-offset.c \
@@ -159,7 +161,6 @@ test_srcs := \
 	tty-write-dpoll.c \
 	unlink.c \
 	wakeup-hang.c \
-	skip-cqe.c \
 	# EOL
 
 
-- 
Ammar Faizi

[PATCH liburing v1 5/6] .github/workflows: Run the nolibc build for x86 32-bit

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Since commit:

  b7d8dd8bbf5b ("arch/x86/syscall: Add x86 32-bit native syscall support")

liburing supports nolibc build for x86 32-bit. Run the nolibc build for
this arch on the GitHub bot too.

Currently, liburing nolibc is only available for:
 - x86-64.
 - x86 (32-bit).

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 .github/workflows/build.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index 30a2fc7..39f4314 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -72,7 +72,7 @@ jobs:
 
     - name: Build nolibc
       run: |
-        if [[ "${{matrix.arch}}" == "x86_64" ]]; then \
+        if [[ "${{matrix.arch}}" == "x86_64" || "${{matrix.arch}}" == "i686" ]]; then \
             make clean; \
             ./configure --cc=${{matrix.cc}} --cxx=${{matrix.cxx}} --nolibc; \
             make -j$(nproc) V=1 CPPFLAGS="-Werror" CFLAGS="$FLAGS" CXXFLAGS="$FLAGS"; \
-- 
Ammar Faizi

[PATCH liburing v1 6/6] test/double-poll-crash: Skip this test if the `mmap()` fails

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

This test is very prone to hiting a SIGSEGV signal due to missing error
handling on mmap() calls. This especially often happens when executing
`runtests-parallel`. Let's just skip this test if those calls fail
instead of having a random segfault with no clear resolution.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 test/double-poll-crash.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/test/double-poll-crash.c b/test/double-poll-crash.c
index 5bfce09..231c7da 100644
--- a/test/double-poll-crash.c
+++ b/test/double-poll-crash.c
@@ -52,10 +52,14 @@ static long syz_io_uring_setup(volatile long a0, volatile long a1,
   *ring_ptr_out = mmap(vma1, ring_sz, PROT_READ | PROT_WRITE,
                        MAP_SHARED | MAP_POPULATE | MAP_FIXED, fd_io_uring,
                        IORING_OFF_SQ_RING);
+  if (*ring_ptr_out == MAP_FAILED)
+    exit(0);
   uint32_t sqes_sz = setup_params->sq_entries * SIZEOF_IO_URING_SQE;
   *sqes_ptr_out =
       mmap(vma2, sqes_sz, PROT_READ | PROT_WRITE,
            MAP_SHARED | MAP_POPULATE | MAP_FIXED, fd_io_uring, IORING_OFF_SQES);
+  if (*sqes_ptr_out == MAP_FAILED)
+    exit(0);
   return fd_io_uring;
 }
 
@@ -108,6 +112,7 @@ uint64_t r[4] = {0xffffffffffffffff, 0x0, 0x0, 0xffffffffffffffff};
 
 int main(int argc, char *argv[])
 {
+  void *mmap_ret;
 #if !defined(__i386) && !defined(__x86_64__)
   return 0;
 #endif
@@ -115,8 +120,12 @@ int main(int argc, char *argv[])
   if (argc > 1)
     return 0;
 
-  mmap((void *)0x20000000ul, 0x1000000ul, 7ul, 0x32ul, -1, 0ul);
-  mmap((void *)0x21000000ul, 0x1000ul, 0ul, 0x32ul, -1, 0ul);
+  mmap_ret = mmap((void *)0x20000000ul, 0x1000000ul, 7ul, 0x32ul, -1, 0ul);
+  if (mmap_ret == MAP_FAILED)
+    return 0;
+  mmap_ret = mmap((void *)0x21000000ul, 0x1000ul, 0ul, 0x32ul, -1, 0ul);
+  if (mmap_ret == MAP_FAILED)
+    return 0;
   intptr_t res = 0;
   *(uint32_t*)0x20000484 = 0;
   *(uint32_t*)0x20000488 = 0;
-- 
Ammar Faizi

Re: [PATCH liburing v1 0/6] liburing fixes and cleanups

[Jens Axboe]

On 4/22/22 2:35 PM, Ammar Faizi wrote:
> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
> 
> Hi Jens,
> 
> Small liburing fixes and cleanups this time:
> 
> - Patch 1 and 2 are a fix for the recent updates from Dylan.
> - Patch 3 and 4 are a Makefile cleanup.
> - Patch 5 is an update for the GitHub bot to build x86 32-bit nolibc.
> - Patch 6 is a workaround for random SIGSEGV error on test/double-poll-crash.

Thanks - applied these manually, as email is being a pain in the...
again.

-- 
Jens Axboe