[PATCH liburing v1 0/9] aarch64 nolibc support

[PATCH liburing v1 0/9] aarch64 nolibc support

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Hi Jens,

This series contains nolibc support for aarch64 and one extra irrelevant
cleanup (patch #1). The missing bit from aarch64 is get_page_size()
which is a bit complicated to implement without libc.

aarch64 supports three values of page size: 4K, 16K, and 64K which are
selected at kernel compilation time. Therefore, we can't hard code the
page size for this arch. In this series we utilize open(), read() and
close() syscall to find the page size from /proc/self/auxv.

The auxiliary vector contains information about the page size, it is
located at `AT_PAGESZ` keyval pair.

For more details about the auxv data structure, check the link below.

Link: https://github.com/torvalds/linux/blob/v5.19-rc4/fs/binfmt_elf.c#L260
Link: https://lwn.net/Articles/631631/

There are 9 patches in this series.

- Patch 1 is just a trivial changelog fix.
- Patch 2 is to handle get_page_size() error that may happen in the
  later patches.
- Patch 3 and 4 are to add open() and read() syscall. We will need them
  to get the page size on aarch64.
- Patch 5 is to rename aarch64 directory to arm64.
- Patch 6 is to remove __INTERNAL__LIBURING_SYSCALL_H checks.
- Patch 7 is to add get_page_size() function.
- Patch 8 is to enable the nolibc support for aarch64.
- Patch 9 is for GitHub bot build.

I have built it with GitHub bot and it compiles just fine. But I don't
have an aarch64 machine to run the tests. Since you are using aarch64,
I can rely on you to test it.

How to test this?

  make clean;
  ./configure --nolibc;
  make -j8;
  make runtests;

Please give it a test...

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

Ammar Faizi (9):
  CHANGELOG: Fixup missing space
  setup: Handle `get_page_size()` failure (for aarch64 nolibc support)
  arch: syscall: Add `__sys_open()` syscall
  arch: syscall: Add `__sys_read()` syscall
  arch/arm64: Rename aarch64 directory to arm64
  arch: syscall: Remove `__INTERNAL__LIBURING_SYSCALL_H` checks
  arch/arm64: Add `get_page_size()` function
  arch: Enable nolibc support for arm64
  .github: Enable aarch64 nolibc build for GitHub bot

 .github/workflows/build.yml           |  2 +-
 CHANGELOG                             |  2 +-
 src/arch/arm64/lib.h                  | 44 +++++++++++++++++++++++++++
 src/arch/{aarch64 => arm64}/syscall.h | 14 ++++-----
 src/arch/generic/lib.h                |  4 ---
 src/arch/generic/syscall.h            | 20 +++++++++---
 src/arch/syscall-defs.h               | 12 ++++++++
 src/arch/x86/lib.h                    |  4 ---
 src/arch/x86/syscall.h                |  4 ---
 src/lib.h                             | 21 +++++++------
 src/setup.c                           |  3 ++
 src/syscall.h                         |  4 +--
 12 files changed, 97 insertions(+), 37 deletions(-)
 create mode 100644 src/arch/arm64/lib.h
 rename src/arch/{aarch64 => arm64}/syscall.h (91%)


base-commit: 68103b731c34a9f83c181cb33eb424f46f3dcb94
-- 
Ammar Faizi

[PATCH liburing v1 1/9] CHANGELOG: Fixup missing space

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

  s/reducingthe/reducing the/

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

diff --git a/CHANGELOG b/CHANGELOG
index 01cb677..efb3ff3 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -6,7 +6,7 @@ liburing-2.2 release
 - Add support for multishot accept.
 - io_uring_register_files() will set RLIMIT_NOFILE if necessary.
 - Add support for registered ring fds, io_uring_register_ring_fd(),
-  reducingthe overhead of an io_uring_enter() system call.
+  reducing the overhead of an io_uring_enter() system call.
 - Add support for the message ring opcode.
 - Add support for newer request cancelation features.
 - Add support for IORING_SETUP_COOP_TASKRUN, which can help reduce the
-- 
Ammar Faizi

[PATCH liburing v1 2/9] setup: Handle `get_page_size()` failure (for aarch64 nolibc support)

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

This is a preparation patch to add aarch64 nolibc support.

aarch64 supports three values of page size: 4K, 16K, and 64K which are
selected at kernel compilation time. Therefore, we can't hard code the
page size for this arch. We will utilize open(), read() and close()
syscall to find the page size from /proc/self/auxv. Since syscall may
fail, we may also fail to get the page size here. Handle the failure.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 src/setup.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/setup.c b/src/setup.c
index d2adc7f..ca9d30d 100644
--- a/src/setup.c
+++ b/src/setup.c
@@ -336,6 +336,9 @@ ssize_t io_uring_mlock_size_params(unsigned entries, struct io_uring_params *p)
 	}
 
 	page_size = get_page_size();
+	if (page_size < 0)
+		return page_size;
+
 	return rings_size(p, entries, cq_entries, page_size);
 }
 
-- 
Ammar Faizi

[PATCH liburing v1 3/9] arch: syscall: Add `__sys_open()` syscall

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

A prep patch to support aarch64 nolibc. We will use this to get the
page size by reading /proc/self/auxv. For some reason __NR_open is
not defined, so also define it in aarch64 syscall specific file.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 src/arch/aarch64/syscall.h | 4 ++++
 src/arch/generic/syscall.h | 9 +++++++++
 src/arch/syscall-defs.h    | 7 +++++++
 3 files changed, 20 insertions(+)

diff --git a/src/arch/aarch64/syscall.h b/src/arch/aarch64/syscall.h
index c0ab7e2..5e26714 100644
--- a/src/arch/aarch64/syscall.h
+++ b/src/arch/aarch64/syscall.h
@@ -84,6 +84,10 @@
 			"r" (x4), "r"(x5));				\
 })
 
+#ifndef __NR_open
+#define __NR_open 	0x400
+#endif
+
 #include "../syscall-defs.h"
 
 #else /* #if defined(__aarch64__) */
diff --git a/src/arch/generic/syscall.h b/src/arch/generic/syscall.h
index fa93064..71b2234 100644
--- a/src/arch/generic/syscall.h
+++ b/src/arch/generic/syscall.h
@@ -7,6 +7,8 @@
 #ifndef LIBURING_ARCH_GENERIC_SYSCALL_H
 #define LIBURING_ARCH_GENERIC_SYSCALL_H
 
+#include <fcntl.h>
+
 static inline int ____sys_io_uring_register(int fd, unsigned opcode,
 					    const void *arg, unsigned nr_args)
 {
@@ -41,6 +43,13 @@ static inline int ____sys_io_uring_enter(int fd, unsigned to_submit,
 				       _NSIG / 8);
 }
 
+static inline int __sys_open(const char *pathname, int flags, mode_t mode)
+{
+	int ret;
+	ret = open(pathname, flags, mode);
+	return (ret < 0) ? -errno : ret;
+}
+
 static inline void *__sys_mmap(void *addr, size_t length, int prot, int flags,
 			       int fd, off_t offset)
 {
diff --git a/src/arch/syscall-defs.h b/src/arch/syscall-defs.h
index 1e8ae1b..0f67446 100644
--- a/src/arch/syscall-defs.h
+++ b/src/arch/syscall-defs.h
@@ -3,6 +3,13 @@
 #ifndef LIBURING_ARCH_SYSCALL_DEFS_H
 #define LIBURING_ARCH_SYSCALL_DEFS_H
 
+#include <fcntl.h>
+
+static inline int __sys_open(const char *pathname, int flags, mode_t mode)
+{
+	return (int) __do_syscall3(__NR_open, pathname, flags, mode);
+}
+
 static inline void *__sys_mmap(void *addr, size_t length, int prot, int flags,
 			       int fd, off_t offset)
 {
-- 
Ammar Faizi

[PATCH liburing v1 4/9] arch: syscall: Add `__sys_read()` syscall

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

A prep patch to support aarch64 nolibc. We will use this to get the
page size by reading /proc/self/auxv.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 src/arch/generic/syscall.h | 7 +++++++
 src/arch/syscall-defs.h    | 5 +++++
 2 files changed, 12 insertions(+)

diff --git a/src/arch/generic/syscall.h b/src/arch/generic/syscall.h
index 71b2234..22252a1 100644
--- a/src/arch/generic/syscall.h
+++ b/src/arch/generic/syscall.h
@@ -50,6 +50,13 @@ static inline int __sys_open(const char *pathname, int flags, mode_t mode)
 	return (ret < 0) ? -errno : ret;
 }
 
+static inline ssize_t __sys_read(int fd, void *buffer, size_t size)
+{
+	ssize_t ret;
+	ret = read(fd, buffer, size);
+	return (ret < 0) ? -errno : ret;
+}
+
 static inline void *__sys_mmap(void *addr, size_t length, int prot, int flags,
 			       int fd, off_t offset)
 {
diff --git a/src/arch/syscall-defs.h b/src/arch/syscall-defs.h
index 0f67446..4b5ba40 100644
--- a/src/arch/syscall-defs.h
+++ b/src/arch/syscall-defs.h
@@ -10,6 +10,11 @@ static inline int __sys_open(const char *pathname, int flags, mode_t mode)
 	return (int) __do_syscall3(__NR_open, pathname, flags, mode);
 }
 
+static inline int __sys_read(int fd, void *buffer, size_t size)
+{
+	return (int) __do_syscall3(__NR_read, fd, buffer, size);
+}
+
 static inline void *__sys_mmap(void *addr, size_t length, int prot, int flags,
 			       int fd, off_t offset)
 {
-- 
Ammar Faizi

[PATCH liburing v1 5/9] arch/arm64: Rename aarch64 directory to arm64

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

In the Linux kernel tree, we use `arm64` instead of `aarch64` to name
the directory that saves this arch specific code. Follow this naming
in liburing too.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 src/arch/{aarch64 => arm64}/syscall.h | 6 +++---
 src/syscall.h                         | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)
 rename src/arch/{aarch64 => arm64}/syscall.h (95%)

diff --git a/src/arch/aarch64/syscall.h b/src/arch/arm64/syscall.h
similarity index 95%
rename from src/arch/aarch64/syscall.h
rename to src/arch/arm64/syscall.h
index 5e26714..732ada0 100644
--- a/src/arch/aarch64/syscall.h
+++ b/src/arch/arm64/syscall.h
@@ -4,8 +4,8 @@
 	#error "This file should be included from src/syscall.h (liburing)"
 #endif
 
-#ifndef LIBURING_ARCH_AARCH64_SYSCALL_H
-#define LIBURING_ARCH_AARCH64_SYSCALL_H
+#ifndef LIBURING_ARCH_ARM64_SYSCALL_H
+#define LIBURING_ARCH_ARM64_SYSCALL_H
 
 #if defined(__aarch64__)
 
@@ -96,4 +96,4 @@
 
 #endif /* #if defined(__aarch64__) */
 
-#endif /* #ifndef LIBURING_ARCH_AARCH64_SYSCALL_H */
+#endif /* #ifndef LIBURING_ARCH_ARM64_SYSCALL_H */
diff --git a/src/syscall.h b/src/syscall.h
index 214789d..9e72e6f 100644
--- a/src/syscall.h
+++ b/src/syscall.h
@@ -74,7 +74,7 @@ static inline bool IS_ERR(const void *ptr)
 #if defined(__x86_64__) || defined(__i386__)
 #include "arch/x86/syscall.h"
 #elif defined(__aarch64__)
-#include "arch/aarch64/syscall.h"
+#include "arch/arm64/syscall.h"
 #else
 /*
  * We don't have native syscall wrappers
-- 
Ammar Faizi

[PATCH liburing v1 6/9] arch: syscall: Remove `__INTERNAL__LIBURING_SYSCALL_H` checks

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

We will include the syscall.h from another place as well. This check
was added by me when refactoring the x86 syscall. For aarch64 we will
include this header from lib.h but we are restricted by this check.

Just remove it for all archs. User shouldn't touch this code directly
anyway.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 src/arch/arm64/syscall.h   | 4 ----
 src/arch/generic/lib.h     | 4 ----
 src/arch/generic/syscall.h | 4 ----
 src/arch/x86/lib.h         | 4 ----
 src/arch/x86/syscall.h     | 4 ----
 src/syscall.h              | 2 --
 6 files changed, 22 deletions(-)

diff --git a/src/arch/arm64/syscall.h b/src/arch/arm64/syscall.h
index 732ada0..69a36a3 100644
--- a/src/arch/arm64/syscall.h
+++ b/src/arch/arm64/syscall.h
@@ -1,9 +1,5 @@
 /* SPDX-License-Identifier: MIT */
 
-#ifndef __INTERNAL__LIBURING_SYSCALL_H
-	#error "This file should be included from src/syscall.h (liburing)"
-#endif
-
 #ifndef LIBURING_ARCH_ARM64_SYSCALL_H
 #define LIBURING_ARCH_ARM64_SYSCALL_H
 
diff --git a/src/arch/generic/lib.h b/src/arch/generic/lib.h
index 737e795..6b006c6 100644
--- a/src/arch/generic/lib.h
+++ b/src/arch/generic/lib.h
@@ -1,9 +1,5 @@
 /* SPDX-License-Identifier: MIT */
 
-#ifndef __INTERNAL__LIBURING_LIB_H
-	#error "This file should be included from src/lib.h (liburing)"
-#endif
-
 #ifndef LIBURING_ARCH_GENERIC_LIB_H
 #define LIBURING_ARCH_GENERIC_LIB_H
 
diff --git a/src/arch/generic/syscall.h b/src/arch/generic/syscall.h
index 22252a1..e637890 100644
--- a/src/arch/generic/syscall.h
+++ b/src/arch/generic/syscall.h
@@ -1,9 +1,5 @@
 /* SPDX-License-Identifier: MIT */
 
-#ifndef __INTERNAL__LIBURING_SYSCALL_H
-	#error "This file should be included from src/syscall.h (liburing)"
-#endif
-
 #ifndef LIBURING_ARCH_GENERIC_SYSCALL_H
 #define LIBURING_ARCH_GENERIC_SYSCALL_H
 
diff --git a/src/arch/x86/lib.h b/src/arch/x86/lib.h
index e6a74f3..6ece2d4 100644
--- a/src/arch/x86/lib.h
+++ b/src/arch/x86/lib.h
@@ -1,9 +1,5 @@
 /* SPDX-License-Identifier: MIT */
 
-#ifndef __INTERNAL__LIBURING_LIB_H
-	#error "This file should be included from src/lib.h (liburing)"
-#endif
-
 #ifndef LIBURING_ARCH_X86_LIB_H
 #define LIBURING_ARCH_X86_LIB_H
 
diff --git a/src/arch/x86/syscall.h b/src/arch/x86/syscall.h
index 43c576b..cb8fb91 100644
--- a/src/arch/x86/syscall.h
+++ b/src/arch/x86/syscall.h
@@ -1,9 +1,5 @@
 /* SPDX-License-Identifier: MIT */
 
-#ifndef __INTERNAL__LIBURING_SYSCALL_H
-	#error "This file should be included from src/syscall.h (liburing)"
-#endif
-
 #ifndef LIBURING_ARCH_X86_SYSCALL_H
 #define LIBURING_ARCH_X86_SYSCALL_H
 
diff --git a/src/syscall.h b/src/syscall.h
index 9e72e6f..39a18df 100644
--- a/src/syscall.h
+++ b/src/syscall.h
@@ -70,7 +70,6 @@ static inline bool IS_ERR(const void *ptr)
 	return uring_unlikely((uintptr_t) ptr >= (uintptr_t) -4095UL);
 }
 
-#define __INTERNAL__LIBURING_SYSCALL_H
 #if defined(__x86_64__) || defined(__i386__)
 #include "arch/x86/syscall.h"
 #elif defined(__aarch64__)
@@ -86,7 +85,6 @@ static inline bool IS_ERR(const void *ptr)
 /* libc syscall wrappers. */
 #include "arch/generic/syscall.h"
 #endif
-#undef __INTERNAL__LIBURING_SYSCALL_H
 
 /*
  * For backward compatibility.
-- 
Ammar Faizi

[PATCH liburing v1 7/9] arch/arm64: Add `get_page_size()` function

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

This is a preparation patch to add aarch64 nolibc support.

aarch64 supports three values of page size: 4K, 16K, and 64K which are
selected at kernel compilation time. Therefore, we can't hard code the
page size for this arch. Utilize open(), read() and close() syscall to
find the page size from /proc/self/auxv. For more details about the
auxv data structure, check the link below.

Link: https://github.com/torvalds/linux/blob/v5.19-rc4/fs/binfmt_elf.c#L260
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 src/arch/arm64/lib.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 src/arch/arm64/lib.h

diff --git a/src/arch/arm64/lib.h b/src/arch/arm64/lib.h
new file mode 100644
index 0000000..4dc39a8
--- /dev/null
+++ b/src/arch/arm64/lib.h
@@ -0,0 +1,44 @@
+/* SPDX-License-Identifier: MIT */
+
+#ifndef LIBURING_ARCH_ARM64_LIB_H
+#define LIBURING_ARCH_ARM64_LIB_H
+
+#include <elf.h>
+#include <sys/auxv.h>
+#include "../../syscall.h"
+
+static inline long get_page_size(void)
+{
+	Elf64_Off buf[2];
+	long page_size;
+	int fd;
+
+	fd = __sys_open("/proc/self/auxv", O_RDONLY, 0);
+	if (fd < 0)
+		return fd;
+
+	while (1) {
+		ssize_t ret;
+
+		ret = __sys_read(fd, buf, sizeof(buf));
+		if (ret < 0) {
+			page_size = -errno;
+			break;
+		}
+
+		if (ret < sizeof(buf)) {
+			page_size = -ENOENT;
+			break;
+		}
+
+		if (buf[0] == AT_PAGESZ) {
+			page_size = buf[1];
+			break;
+		}
+	}
+
+	__sys_close(fd);
+	return page_size;
+}
+
+#endif /* #ifndef LIBURING_ARCH_ARM64_LIB_H */
-- 
Ammar Faizi

[PATCH liburing v1 8/9] arch: Enable nolibc support for arm64

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Support nolibc build for arm64.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 src/lib.h | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/src/lib.h b/src/lib.h
index 6672cc5..e1419f8 100644
--- a/src/lib.h
+++ b/src/lib.h
@@ -8,16 +8,19 @@
 
 #define __INTERNAL__LIBURING_LIB_H
 #if defined(__x86_64__) || defined(__i386__)
-	#include "arch/x86/lib.h"
+#include "arch/x86/lib.h"
+#elif defined(__aarch64__)
+#include "arch/arm64/lib.h"
 #else
-	/*
-	 * We don't have nolibc support for this arch. Must use libc!
-	 */
-	#ifdef CONFIG_NOLIBC
-		#error "This arch doesn't support building liburing without libc"
-	#endif
-	/* libc wrappers. */
-	#include "arch/generic/lib.h"
+/*
+ * We don't have nolibc support for this arch. Must use libc!
+ */
+#ifdef CONFIG_NOLIBC
+#error "This arch doesn't support building liburing without libc"
+#endif
+
+/* libc wrappers. */
+#include "arch/generic/lib.h"
 #endif
 #undef __INTERNAL__LIBURING_LIB_H
 
-- 
Ammar Faizi

[PATCH liburing v1 9/9] .github: Enable aarch64 nolibc build for GitHub bot

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

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 88192ff..fc119cb 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -114,7 +114,7 @@ jobs:
 
     - name: Build nolibc
       run: |
-        if [[ "${{matrix.arch}}" == "x86_64" || "${{matrix.arch}}" == "i686" ]]; then \
+        if [[ "${{matrix.arch}}" == "x86_64" || "${{matrix.arch}}" == "i686" || "${{matrix.arch}}" == "aarch64" ]]; then \
             make clean; \
             ./configure --cc=${{matrix.cc}} --cxx=${{matrix.cxx}} --nolibc; \
             make -j$(nproc) V=1 CPPFLAGS="-Werror" CFLAGS="$FLAGS" CXXFLAGS="$FLAGS"; \
-- 
Ammar Faizi

Re: [PATCH liburing v1 7/9] arch/arm64: Add `get_page_size()` function

[Ammar Faizi]

On 6/29/22 7:27 AM, Ammar Faizi wrote:
> +	while (1) {
> +		ssize_t ret;
> +
> +		ret = __sys_read(fd, buf, sizeof(buf));
> +		if (ret < 0) {
> +			page_size = -errno;
> +			break;
> +		}

Oops, this is wrong, I shouldn't use errno here, it should be:
    
    page_size = ret;

Should I resend? Or you can fix it?

-- 
Ammar Faizi

Re: [PATCH liburing v1 2/9] setup: Handle `get_page_size()` failure (for aarch64 nolibc support)

[Alviro Iskandar Setiawan]

On Wed, Jun 29, 2022 at 7:28 AM Ammar Faizi wrote:
>         page_size = get_page_size();
> +       if (page_size < 0)
> +               return page_size;
> +
>         return rings_size(p, entries, cq_entries, page_size);
>  }

the current error handling fallback to 4K if fail on sysconf(_SC_PAGESIZE):
https://github.com/axboe/liburing/blob/68103b731c34a9f83c181cb33eb424f46f3dcb94/src/arch/generic/lib.h#L10-L19
with this patch, get_page_size() is only possible to return negative
value on aarch64.

i don't understand why the current master branch code fallback to 4K when fail?

tq

-- Viro

Re: [PATCH liburing v1 2/9] setup: Handle `get_page_size()` failure (for aarch64 nolibc support)

[Ammar Faizi]

On 6/29/22 7:50 AM, Alviro Iskandar Setiawan wrote:
> On Wed, Jun 29, 2022 at 7:28 AM Ammar Faizi wrote:
>>          page_size = get_page_size();
>> +       if (page_size < 0)
>> +               return page_size;
>> +
>>          return rings_size(p, entries, cq_entries, page_size);
>>   }
> 
> the current error handling fallback to 4K if fail on sysconf(_SC_PAGESIZE):
> https://github.com/axboe/liburing/blob/68103b731c34a9f83c181cb33eb424f46f3dcb94/src/arch/generic/lib.h#L10-L19
> with this patch, get_page_size() is only possible to return negative
> value on aarch64.

Ah right, this one needs a revision. Either we fallback to 4K, or
return error if we fail.

> i don't understand why the current master branch code fallback to 4K when fail?

Neither do I. Maybe because 4K is widely used page size?

Jens, can you shed some light on this?

   The current upstream does this:

      - For x86/x86-64, it's hard-coded to 4K. So it can't fail.
      - For other archs, if sysconf(_SC_PAGESIZE) fails, we fallback to 4K.

Now we are going to add aarch64, it uses a group of syscalls to get the page
size. So it may fail. What should we do when we fail?

Fallback to 4K? Or return error code from syscall?
       
-- 
Ammar Faizi

Re: [PATCH liburing v1 5/9] arch/arm64: Rename aarch64 directory to arm64

[Jens Axboe]

On 6/28/22 6:27 PM, Ammar Faizi wrote:
> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
> 
> In the Linux kernel tree, we use `arm64` instead of `aarch64` to name
> the directory that saves this arch specific code. Follow this naming
> in liburing too.

I don't feel too strongly about this, though I do think the linux
kernel is wrong in this regard and liburing is doing it right :-)

-- 
Jens Axboe

Re: [PATCH liburing v1 7/9] arch/arm64: Add `get_page_size()` function

[Jens Axboe]

On 6/28/22 6:27 PM, Ammar Faizi wrote:
> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
> 
> This is a preparation patch to add aarch64 nolibc support.
> 
> aarch64 supports three values of page size: 4K, 16K, and 64K which are
> selected at kernel compilation time. Therefore, we can't hard code the
> page size for this arch. Utilize open(), read() and close() syscall to
> find the page size from /proc/self/auxv. For more details about the
> auxv data structure, check the link below.

We should probably cache this value if already read? At least I don't
think we have systems where the page size would differ between
applications.

-- 
Jens Axboe

Re: [PATCH liburing v1 5/9] arch/arm64: Rename aarch64 directory to arm64

[Ammar Faizi]

On 6/29/22 9:48 PM, Jens Axboe wrote:
> On 6/28/22 6:27 PM, Ammar Faizi wrote:
>> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
>>
>> In the Linux kernel tree, we use `arm64` instead of `aarch64` to name
>> the directory that saves this arch specific code. Follow this naming
>> in liburing too.
> 
> I don't feel too strongly about this, though I do think the linux
> kernel is wrong in this regard and liburing is doing it right :-)

OK, will drop this in v2.

-- 
Ammar Faizi

Re: [PATCH liburing v1 7/9] arch/arm64: Add `get_page_size()` function

[Ammar Faizi]

On 6/29/22 9:49 PM, Jens Axboe wrote:
> On 6/28/22 6:27 PM, Ammar Faizi wrote:
>> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
>>
>> This is a preparation patch to add aarch64 nolibc support.
>>
>> aarch64 supports three values of page size: 4K, 16K, and 64K which are
>> selected at kernel compilation time. Therefore, we can't hard code the
>> page size for this arch. Utilize open(), read() and close() syscall to
>> find the page size from /proc/self/auxv. For more details about the
>> auxv data structure, check the link below.
> 
> We should probably cache this value if already read? At least I don't
> think we have systems where the page size would differ between
> applications.

Good idea, will use a static variable to cache it in v2 then...

-- 
Ammar Faizi

Re: [PATCH liburing v1 2/9] setup: Handle `get_page_size()` failure (for aarch64 nolibc support)

[Jens Axboe]

On 6/28/22 7:00 PM, Ammar Faizi wrote:
> On 6/29/22 7:50 AM, Alviro Iskandar Setiawan wrote:
>> On Wed, Jun 29, 2022 at 7:28 AM Ammar Faizi wrote:
>>>          page_size = get_page_size();
>>> +       if (page_size < 0)
>>> +               return page_size;
>>> +
>>>          return rings_size(p, entries, cq_entries, page_size);
>>>   }
>>
>> the current error handling fallback to 4K if fail on sysconf(_SC_PAGESIZE):
>> https://github.com/axboe/liburing/blob/68103b731c34a9f83c181cb33eb424f46f3dcb94/src/arch/generic/lib.h#L10-L19
>> with this patch, get_page_size() is only possible to return negative
>> value on aarch64.
> 
> Ah right, this one needs a revision. Either we fallback to 4K, or
> return error if we fail.
> 
>> i don't understand why the current master branch code fallback to 4K when fail?
> 
> Neither do I. Maybe because 4K is widely used page size?
> 
> Jens, can you shed some light on this?
> 
>   The current upstream does this:
> 
>      - For x86/x86-64, it's hard-coded to 4K. So it can't fail.
>      - For other archs, if sysconf(_SC_PAGESIZE) fails, we fallback to 4K.
> 
> Now we are going to add aarch64, it uses a group of syscalls to get the page
> size. So it may fail. What should we do when we fail?
> 
> Fallback to 4K? Or return error code from syscall?

4k is the most common page size, by far, so makes sense to have that as
a fallback rather than just error out. Perhaps the application will then
fail differently, but there's also a chance that it'll just work.

So I think just falling back to 4k if we fail for whatever reason is the
sanest recourse.

-- 
Jens Axboe

Re: [PATCH liburing v1 2/9] setup: Handle `get_page_size()` failure (for aarch64 nolibc support)

[Ammar Faizi]

On 6/29/22 10:00 PM, Jens Axboe wrote:
> 4k is the most common page size, by far, so makes sense to have that as
> a fallback rather than just error out. Perhaps the application will then
> fail differently, but there's also a chance that it'll just work.
> 
> So I think just falling back to 4k if we fail for whatever reason is the
> sanest recourse.

OK, I'll do that in the v2 revision.

-- 
Ammar Faizi