Tea Inside Mailing List <[email protected]>
 help / color / mirror / Atom feed
* [PATCH liburing v1 0/4] Refactor arch dependent code and x86-64 improvement
@ 2022-02-11 15:57 Ammar Faizi
  2022-02-11 15:57 ` [PATCH liburing v1 1/4] arch/generic: Create arch generic syscall wrappers Ammar Faizi
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Ammar Faizi @ 2022-02-11 15:57 UTC (permalink / raw)
  To: Jens Axboe
  Cc: io-uring Mailing List, GNU/Weeb Mailing List,
	Tea Inside Mailing List, Alviro Iskandar Setiawan,
	Alviro Iskandar Setiawan, Ammar Faizi, Nugra

Hi,

We have many #ifdef/#endif in syscall.h since nolibc support is added.
This series tries to clean them up, separate the definitions into
smaller more manageable pieces.

Also, optimize function call for x86-64. Avoid libc function call for
syscall even if CONFIG_NOLIBC is disabled. If this patchset is applied,
CONFIG_NOLIBC is still meaningful, we may still use libc for malloc(),
free() and memset().


New directory structure for arch 
==================================

1) src/arch/generic
   This contains wrappers based on libc that can be used for all archs.

2) src/arch/x86
   This contains x86 specific code.

In the future, more architecture specific code can be added. Currently,
architectures other than x86-64 will use code from src/arch/generic.


Technical Explanation
======================

There are 4 patches in this series.

Patch 1
#########
This is a preparation for refactoring the syscall wrappers.

This creates a new file src/arch/generic/syscall.h. This file contains
libc syscall wrappers for architectures that don't have arch specific
code. In the next patches, we will include this file from src/syscall.h.

It aims to reduce the usage of #ifdef/#endif that occurs in every
function in src/syscall.h file. Also, it will make the arch specific
code structure cleaner and easier to manage.

Patch 2
#########

There are 3 things in this patch (a, b, c):

a) Remove all ____sys* and uring_* functions from src/syscall.h. We
   will define them in the src/arch/* files, so we can avoid many
   #ifdef/#endif.

b) Rename all __arch_impl_* functions in src/arch/x86/syscall.h with
   ____sys* and uring_* to support point (1).

c) Always use arch specific code for x86-64 syscalls, even with
   CONFIG_NOLIBC disabled. For other archs, currently, will still
   use the libc wrappers (we provided it in src/arch/generic*).

Major changes happen in point (c). We will always use inline assembly
for invoking syscall for x86-64. Reasoning:

1. It reduces function calls.
------------------------------
If we use libc, we need to call syscall(2) function and deal with a
global state via errno macro (errno macro will expand to a
function call too).

If we use inline Assembly, we eliminate many functions calls, we
don't need to use errno or any global state anymore as it will
just directly return error code that we can check with a simple
comparison.

2. Allow the compiler to reuse caller clobbered registers.
-----------------------------------------------------------
By the rule of System V ABI x86-64, a function call clobbers %rax,
%rdi, %rsi, %rdx, %rcx, %r8, %r9, %r10 and %r11. On Linux, syscall
only clobbers %rax, %rcx and %r11. But since libc syscall(2) wrapper
is a function call, the compiler will always miss the opportunity
to reuse those clobbered registers. That means it has to preserve
the life values on the stack if they happen to be in the clobbered
registers (that's also extra memory access).

By inlining the syscall instruction, the compiler has an opportunity
to reuse all registers after invoking syscall, except %rax, %rcx and
%r11.

3. Smaller binary size.
------------------------
Point (1) and (2) will also reduce the data movement, hence smaller
Assembly code, smaller binary size.

4. Reduce %rip round trip to libc.so.
--------------------------------------
Call to a libc function will make the %rip jump to libc.so memory
area. This can have extra overhead and extra icache misses in some
scenario. If we inline the syscall instruction, this overhead can
be removed.

Patch 3
#########
This takes care of lib.h header file, split it into generic and arch
specific.

Patch 4
#########
Change all syscall function name prefix to __sys.

Instead of using uring_mmap, uring_close, uring_madvise, etc. Let's
use __sys_mmap, __sys_close, __sys_madvise, etc. That looks better
convention for syscall function name like what we do with
__sys_io_uring* functions.


Cc: Nugra <[email protected]>
Signed-off-by: Alviro Iskandar Setiawan <[email protected]>
Signed-off-by: Ammar Faizi <[email protected]>
---
Alviro Iskandar Setiawan (2):
  arch/generic: Create arch generic syscall wrappers
  arch/x86, syscall: Refactor arch specific and generic syscall wrappers

Ammar Faizi (2):
  lib.h: Split off lib header for arch specific and generic
  Change all syscall function name prefix to __sys

 src/arch/generic/lib.h     |  21 +++++
 src/arch/generic/syscall.h |  87 +++++++++++++++++++++
 src/arch/x86/lib.h         |  20 +++--
 src/arch/x86/syscall.h     |  57 ++++++++------
 src/lib.h                  |  45 +++++------
 src/nolibc.c               |   4 +-
 src/register.c             |   4 +-
 src/setup.c                |  22 +++---
 src/syscall.h              | 155 ++++++-------------------------------
 9 files changed, 214 insertions(+), 201 deletions(-)
 create mode 100644 src/arch/generic/lib.h
 create mode 100644 src/arch/generic/syscall.h


base-commit: 90a4da4d51f137229a2ef39b25880d81adfcb487
-- 
2.32.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH liburing v1 1/4] arch/generic: Create arch generic syscall wrappers
  2022-02-11 15:57 [PATCH liburing v1 0/4] Refactor arch dependent code and x86-64 improvement Ammar Faizi
@ 2022-02-11 15:57 ` Ammar Faizi
  2022-02-11 15:57 ` [PATCH liburing v1 2/4] arch/x86, syscall: Refactor arch specific and " Ammar Faizi
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Ammar Faizi @ 2022-02-11 15:57 UTC (permalink / raw)
  To: Jens Axboe
  Cc: io-uring Mailing List, GNU/Weeb Mailing List,
	Tea Inside Mailing List, Alviro Iskandar Setiawan,
	Alviro Iskandar Setiawan, Nugra, Ammar Faizi

From: Alviro Iskandar Setiawan <[email protected]>

This is a preparation for refactoring the syscall wrappers.

This creates a new file src/arch/generic/syscall.h. This file
contains libc syscall wrappers for architectures that don't
have arch specific code. In the next patches, we will include
this file from src/syscall.h.

It aims to reduce the usage of #ifdef/#endif that occurs in
every function in src/syscall.h file. Also, it will make the
arch specific code structure cleaner and easier to manage.

Cc: Nugra <[email protected]>
Signed-off-by: Alviro Iskandar Setiawan <[email protected]>
Co-authored-by: Ammar Faizi <[email protected]>
Signed-off-by: Ammar Faizi <[email protected]>
---
 src/arch/generic/syscall.h | 83 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)
 create mode 100644 src/arch/generic/syscall.h

diff --git a/src/arch/generic/syscall.h b/src/arch/generic/syscall.h
new file mode 100644
index 0000000..7136290
--- /dev/null
+++ b/src/arch/generic/syscall.h
@@ -0,0 +1,83 @@
+/* SPDX-License-Identifier: MIT */
+
+#ifndef LIBURING_ARCH_GENERIC_SYSCALL_H
+#define LIBURING_ARCH_GENERIC_SYSCALL_H
+
+static inline int ____sys_io_uring_register(int fd, unsigned opcode,
+					    const void *arg, unsigned nr_args)
+{
+	int ret;
+	ret = syscall(__NR_io_uring_register, fd, opcode, arg, nr_args);
+	return (ret < 0) ? -errno : ret;
+}
+
+static inline int ____sys_io_uring_setup(unsigned entries,
+					 struct io_uring_params *p)
+{
+	int ret;
+	ret = syscall(__NR_io_uring_setup, entries, p);
+	return (ret < 0) ? -errno : ret;
+}
+
+static inline int ____sys_io_uring_enter2(int fd, unsigned to_submit,
+					  unsigned min_complete, unsigned flags,
+					  sigset_t *sig, int sz)
+{
+	int ret;
+	ret = syscall(__NR_io_uring_enter, fd, to_submit, min_complete, flags,
+		      sig, sz);
+	return (ret < 0) ? -errno : ret;
+}
+
+static inline int ____sys_io_uring_enter(int fd, unsigned to_submit,
+					 unsigned min_complete, unsigned flags,
+					 sigset_t *sig)
+{
+	return ____sys_io_uring_enter2(fd, to_submit, min_complete, flags, sig,
+				       _NSIG / 8);
+}
+
+static inline void *uring_mmap(void *addr, size_t length, int prot, int flags,
+			       int fd, off_t offset)
+{
+	void *ret;
+	ret = mmap(addr, length, prot, flags, fd, offset);
+	return (ret == MAP_FAILED) ? ERR_PTR(-errno) : ret;
+}
+
+static inline int uring_munmap(void *addr, size_t length)
+{
+	int ret;
+	ret = munmap(addr, length);
+	return (ret < 0) ? -errno : ret;
+}
+
+static inline int uring_madvise(void *addr, size_t length, int advice)
+{
+	int ret;
+	ret = madvise(addr, length, advice);
+	return (ret < 0) ? -errno : ret;
+}
+
+static inline int uring_getrlimit(int resource, struct rlimit *rlim)
+{
+	int ret;
+	ret = getrlimit(resource, rlim);
+	return (ret < 0) ? -errno : ret;
+}
+
+static inline int uring_setrlimit(int resource, const struct rlimit *rlim)
+{
+	int ret;
+	ret = setrlimit(resource, rlim);
+	return (ret < 0) ? -errno : ret;
+}
+
+static inline int uring_close(int fd)
+{
+	int ret;
+	ret = close(fd);
+	return (ret < 0) ? -errno : ret;
+}
+
+#endif /* #ifndef LIBURING_ARCH_GENERIC_SYSCALL_H */
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH liburing v1 2/4] arch/x86, syscall: Refactor arch specific and generic syscall wrappers
  2022-02-11 15:57 [PATCH liburing v1 0/4] Refactor arch dependent code and x86-64 improvement Ammar Faizi
  2022-02-11 15:57 ` [PATCH liburing v1 1/4] arch/generic: Create arch generic syscall wrappers Ammar Faizi
@ 2022-02-11 15:57 ` Ammar Faizi
  2022-02-11 15:57 ` [PATCH liburing v1 3/4] lib.h: Split off lib header for arch specific and generic Ammar Faizi
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Ammar Faizi @ 2022-02-11 15:57 UTC (permalink / raw)
  To: Jens Axboe
  Cc: io-uring Mailing List, GNU/Weeb Mailing List,
	Tea Inside Mailing List, Alviro Iskandar Setiawan,
	Alviro Iskandar Setiawan, Nugra, Ammar Faizi

From: Alviro Iskandar Setiawan <[email protected]>

In the previous patch, we create a file src/arch/generic/syscall.h.
Let's use it. There are 3 things in this patch (a, b, c):

a) Remove all ____sys* and uring_* functions from src/syscall.h. We
   will define them in the src/arch/* files, so we can avoid many
   #ifdef/#endif.

b) Rename all __arch_impl_* functions in src/arch/x86/syscall.h with
   ____sys* and uring_* to support point (1).

c) Always use arch specific code for x86-64 syscalls, even with
   CONFIG_NOLIBC disabled. For other archs, currently, will still
   use the libc wrappers (we provided it in src/arch/generic*).

Major changes happen in point (c). We will always use inline assembly
for invoking syscall for x86-64. Reasoning:

1. It reduces function calls.
------------------------------
If we use libc, we need to call syscall(2) function and deal with a
global state via `errno` macro (`errno` macro will expand to a
function call too).

If we use inline Assembly, we eliminate many functions calls, we
don't need to use `errno` or any global state anymore as it will
just directly return error code that we can check with a simple
comparison.

2. Allow the compiler to reuse caller clobbered registers.
-----------------------------------------------------------
By the rule of System V ABI x86-64, a function call clobbers %rax,
%rdi, %rsi, %rdx, %rcx, %r8, %r9, %r10 and %r11. On Linux, syscall
only clobbers %rax, %rcx and %r11. But since libc syscall(2) wrapper
is a function call, the compiler will always miss the opportunity
to reuse those clobbered registers. That means it has to preserve
the life values on the stack if they happen to be in the clobbered
registers (that's also extra memory access).

By inlining the syscall instruction, the compiler has an opportunity
to reuse all registers after invoking syscall, except %rax, %rcx and
%r11.

3. Smaller binary size.
------------------------
Point (1) and (2) will also reduce the data movement, hence smaller
Assembly code, smaller binary size.

4. Reduce %rip round trip to libc.so.
--------------------------------------
Call to a libc function will make the %rip jump to libc.so memory
area. This can have extra overhead and extra icache misses in some
scenario. If we inline the syscall instruction, this overhead can
be removed.

No functional change should be visible to user. At this point, we
may stil use libc for malloc(), free() and memset(), so CONFIG_NOLIBC
is still meaningful after these changes.

Cc: Nugra <[email protected]>
Signed-off-by: Alviro Iskandar Setiawan <[email protected]>
Co-authored-by: Ammar Faizi <[email protected]>
Signed-off-by: Ammar Faizi <[email protected]>
---
 src/arch/generic/syscall.h |   4 +
 src/arch/x86/syscall.h     |  57 ++++++++------
 src/syscall.h              | 155 ++++++-------------------------------
 3 files changed, 63 insertions(+), 153 deletions(-)

diff --git a/src/arch/generic/syscall.h b/src/arch/generic/syscall.h
index 7136290..6b10fe3 100644
--- a/src/arch/generic/syscall.h
+++ b/src/arch/generic/syscall.h
@@ -1,5 +1,9 @@
 /* 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/syscall.h b/src/arch/x86/syscall.h
index 2fb3552..2d5642c 100644
--- a/src/arch/x86/syscall.h
+++ b/src/arch/x86/syscall.h
@@ -1,12 +1,12 @@
 /* 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
 
-#ifndef LIBURING_SYSCALL_H
-#  error "This file should be included from src/syscall.h (liburing)"
-#endif
-
 #if defined(__x86_64__)
 /**
  * Note for syscall registers usage (x86-64):
@@ -29,8 +29,8 @@
  *   %r11 == %rflags and %rcx == %rip.
  */
 
-static inline void *__arch_impl_mmap(void *addr, size_t length, int prot,
-				     int flags, int fd, off_t offset)
+static inline void *uring_mmap(void *addr, size_t length, int prot, int flags,
+			       int fd, off_t offset)
 {
 	void *rax;
 	register int r10 __asm__("r10") = flags;
@@ -52,7 +52,7 @@ static inline void *__arch_impl_mmap(void *addr, size_t length, int prot,
 	return rax;
 }
 
-static inline int __arch_impl_munmap(void *addr, size_t length)
+static inline int uring_munmap(void *addr, size_t length)
 {
 	long rax;
 
@@ -67,7 +67,7 @@ static inline int __arch_impl_munmap(void *addr, size_t length)
 	return (int) rax;
 }
 
-static inline int __arch_impl_madvise(void *addr, size_t length, int advice)
+static inline int uring_madvise(void *addr, size_t length, int advice)
 {
 	long rax;
 
@@ -83,7 +83,7 @@ static inline int __arch_impl_madvise(void *addr, size_t length, int advice)
 	return (int) rax;
 }
 
-static inline int __arch_impl_getrlimit(int resource, struct rlimit *rlim)
+static inline int uring_getrlimit(int resource, struct rlimit *rlim)
 {
 	long rax;
 
@@ -98,7 +98,7 @@ static inline int __arch_impl_getrlimit(int resource, struct rlimit *rlim)
 	return (int) rax;
 }
 
-static inline int __arch_impl_setrlimit(int resource, const struct rlimit *rlim)
+static inline int uring_setrlimit(int resource, const struct rlimit *rlim)
 {
 	long rax;
 
@@ -113,7 +113,7 @@ static inline int __arch_impl_setrlimit(int resource, const struct rlimit *rlim)
 	return (int) rax;
 }
 
-static inline int __arch_impl_close(int fd)
+static inline int uring_close(int fd)
 {
 	long rax;
 
@@ -127,9 +127,9 @@ static inline int __arch_impl_close(int fd)
 	return (int) rax;
 }
 
-static inline int __arch_impl_io_uring_register(int fd, unsigned opcode,
-						const void *arg,
-						unsigned nr_args)
+static inline int ____sys_io_uring_register(int fd, unsigned opcode,
+					    const void *arg,
+					    unsigned nr_args)
 {
 	long rax;
 	register unsigned r10 __asm__("r10") = nr_args;
@@ -147,8 +147,8 @@ static inline int __arch_impl_io_uring_register(int fd, unsigned opcode,
 	return (int) rax;
 }
 
-static inline int __arch_impl_io_uring_setup(unsigned entries,
-					     struct io_uring_params *p)
+static inline int ____sys_io_uring_setup(unsigned entries,
+					 struct io_uring_params *p)
 {
 	long rax;
 
@@ -163,10 +163,9 @@ static inline int __arch_impl_io_uring_setup(unsigned entries,
 	return (int) rax;
 }
 
-static inline int __arch_impl_io_uring_enter(int fd, unsigned to_submit,
-					     unsigned min_complete,
-					     unsigned flags, sigset_t *sig,
-					     int sz)
+static inline int ____sys_io_uring_enter2(int fd, unsigned to_submit,
+					  unsigned min_complete, unsigned flags,
+					  sigset_t *sig, int sz)
 {
 	long rax;
 	register unsigned r10 __asm__("r10") = flags;
@@ -188,12 +187,26 @@ static inline int __arch_impl_io_uring_enter(int fd, unsigned to_submit,
 	return (int) rax;
 }
 
+static inline int ____sys_io_uring_enter(int fd, unsigned to_submit,
+					 unsigned min_complete, unsigned flags,
+					 sigset_t *sig)
+{
+	return ____sys_io_uring_enter2(fd, to_submit, min_complete, flags, sig,
+				       _NSIG / 8);
+}
+
 #else /* #if defined(__x86_64__) */
 
 /*
- * TODO: Add x86 (32-bit) support here.
+ * For x86 (32-bit), fallback to libc wrapper.
+ * We can't use CONFIG_NOLIBC for x86 (32-bit) at the moment.
+ *
+ * TODO: Add x86 (32-bit) nolibc support.
  */
-#error "x86 (32-bit) is currently not supported for nolibc builds"
+#ifdef CONFIG_NOLIBC
+	#error "x86 (32-bit) is currently not supported for nolibc builds"
+#endif
+#include "../generic/syscall.h"
 
 #endif /* #if defined(__x86_64__) */
 
diff --git a/src/syscall.h b/src/syscall.h
index 4b336f1..beb357e 100644
--- a/src/syscall.h
+++ b/src/syscall.h
@@ -55,27 +55,6 @@
  */
 struct io_uring_params;
 
-
-#ifdef CONFIG_NOLIBC
-#  if defined(__x86_64__) || defined(__i386__)
-#    include "arch/x86/syscall.h"
-#  else
-#    error "This arch doesn't support building liburing without libc"
-#  endif
-#endif
-
-
-/*
- * System calls
- */
-int __sys_io_uring_setup(unsigned entries, struct io_uring_params *p);
-int __sys_io_uring_enter(int fd, unsigned to_submit, unsigned min_complete,
-			 unsigned flags, sigset_t *sig);
-int __sys_io_uring_enter2(int fd, unsigned to_submit, unsigned min_complete,
-			  unsigned flags, sigset_t *sig, int sz);
-int __sys_io_uring_register(int fd, unsigned int opcode, const void *arg,
-			    unsigned int nr_args);
-
 static inline void *ERR_PTR(intptr_t n)
 {
 	return (void *) n;
@@ -91,118 +70,32 @@ static inline bool IS_ERR(const void *ptr)
 	return uring_unlikely((uintptr_t) ptr >= (uintptr_t) -4095UL);
 }
 
-static inline int ____sys_io_uring_register(int fd, unsigned opcode,
-					    const void *arg, unsigned nr_args)
-{
-#ifdef CONFIG_NOLIBC
-	return __arch_impl_io_uring_register(fd, opcode, arg, nr_args);
-#else
-	int ret;
-	ret = syscall(__NR_io_uring_register, fd, opcode, arg, nr_args);
-	return (ret < 0) ? -errno : ret;
-#endif
-}
-
-static inline int ____sys_io_uring_setup(unsigned entries,
-					 struct io_uring_params *p)
-{
-#ifdef CONFIG_NOLIBC
-	return __arch_impl_io_uring_setup(entries, p);
+#define __INTERNAL__LIBURING_SYSCALL_H
+#if defined(__x86_64__) || defined(__i386__)
+	#include "arch/x86/syscall.h"
 #else
-	int ret;
-	ret = syscall(__NR_io_uring_setup, entries, p);
-	return (ret < 0) ? -errno : ret;
+	/*
+	 * We don't have native syscall wrappers
+	 * for this arch. Must use libc!
+	 */
+	#ifdef CONFIG_NOLIBC
+		#error "This arch doesn't support building liburing without libc"
+	#endif
+	/* libc syscall wrappers. */
+	#include "arch/generic/syscall.h"
 #endif
-}
-
-static inline int ____sys_io_uring_enter2(int fd, unsigned to_submit,
-					  unsigned min_complete, unsigned flags,
-					  sigset_t *sig, int sz)
-{
-#ifdef CONFIG_NOLIBC
-	return __arch_impl_io_uring_enter(fd, to_submit, min_complete, flags,
-					  sig, sz);
-#else
-	int ret;
-	ret = syscall(__NR_io_uring_enter, fd, to_submit, min_complete, flags,
-		      sig, sz);
-	return (ret < 0) ? -errno : ret;
-#endif
-}
-
-static inline int ____sys_io_uring_enter(int fd, unsigned to_submit,
-					 unsigned min_complete, unsigned flags,
-					 sigset_t *sig)
-{
-	return ____sys_io_uring_enter2(fd, to_submit, min_complete, flags, sig,
-				       _NSIG / 8);
-}
-
-static inline void *uring_mmap(void *addr, size_t length, int prot, int flags,
-			       int fd, off_t offset)
-{
-#ifdef CONFIG_NOLIBC
-	return __arch_impl_mmap(addr, length, prot, flags, fd, offset);
-#else
-	void *ret;
-	ret = mmap(addr, length, prot, flags, fd, offset);
-	return (ret == MAP_FAILED) ? ERR_PTR(-errno) : ret;
-#endif
-}
+#undef __INTERNAL__LIBURING_SYSCALL_H
 
-static inline int uring_munmap(void *addr, size_t length)
-{
-#ifdef CONFIG_NOLIBC
-	return __arch_impl_munmap(addr, length);
-#else
-	int ret;
-	ret = munmap(addr, length);
-	return (ret < 0) ? -errno : ret;
-#endif
-}
-
-static inline int uring_madvise(void *addr, size_t length, int advice)
-{
-#ifdef CONFIG_NOLIBC
-	return __arch_impl_madvise(addr, length, advice);
-#else
-	int ret;
-	ret = madvise(addr, length, advice);
-	return (ret < 0) ? -errno : ret;
-#endif
-}
-
-static inline int uring_getrlimit(int resource, struct rlimit *rlim)
-{
-#ifdef CONFIG_NOLIBC
-	return __arch_impl_getrlimit(resource, rlim);
-#else
-	int ret;
-	ret = getrlimit(resource, rlim);
-	return (ret < 0) ? -errno : ret;
-#endif
-}
-
-static inline int uring_setrlimit(int resource, const struct rlimit *rlim)
-{
-#ifdef CONFIG_NOLIBC
-	return __arch_impl_setrlimit(resource, rlim);
-#else
-	int ret;
-	ret = setrlimit(resource, rlim);
-	return (ret < 0) ? -errno : ret;
-#endif
-}
-
-static inline int uring_close(int fd)
-{
-#ifdef CONFIG_NOLIBC
-	return __arch_impl_close(fd);
-#else
-	int ret;
-	ret = close(fd);
-	return (ret < 0) ? -errno : ret;
-#endif
-}
+/*
+ * For backward compatibility.
+ * (these __sys* functions always use libc, see syscall.c)
+ */
+int __sys_io_uring_setup(unsigned entries, struct io_uring_params *p);
+int __sys_io_uring_enter(int fd, unsigned to_submit, unsigned min_complete,
+			 unsigned flags, sigset_t *sig);
+int __sys_io_uring_enter2(int fd, unsigned to_submit, unsigned min_complete,
+			  unsigned flags, sigset_t *sig, int sz);
+int __sys_io_uring_register(int fd, unsigned int opcode, const void *arg,
+			    unsigned int nr_args);
 
 #endif
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH liburing v1 3/4] lib.h: Split off lib header for arch specific and generic
  2022-02-11 15:57 [PATCH liburing v1 0/4] Refactor arch dependent code and x86-64 improvement Ammar Faizi
  2022-02-11 15:57 ` [PATCH liburing v1 1/4] arch/generic: Create arch generic syscall wrappers Ammar Faizi
  2022-02-11 15:57 ` [PATCH liburing v1 2/4] arch/x86, syscall: Refactor arch specific and " Ammar Faizi
@ 2022-02-11 15:57 ` Ammar Faizi
  2022-02-11 15:57 ` [PATCH liburing v1 4/4] Change all syscall function name prefix to __sys Ammar Faizi
  2022-02-11 16:39 ` [PATCH liburing v1 0/4] Refactor arch dependent code and x86-64 improvement Jens Axboe
  4 siblings, 0 replies; 6+ messages in thread
From: Ammar Faizi @ 2022-02-11 15:57 UTC (permalink / raw)
  To: Jens Axboe
  Cc: io-uring Mailing List, GNU/Weeb Mailing List,
	Tea Inside Mailing List, Alviro Iskandar Setiawan,
	Alviro Iskandar Setiawan, Ammar Faizi, Nugra

1) Clean up #ifdef/#endif for get_page_size().

2) Always use arch specific code for x86-64 to reduce libc usage.

3) For other archs, we will use src/arch/generic/lib.h header that
   contains libc wrapper.

At this point, on x86-64, we only use libc for memset(), malloc() and
free().

Cc: Nugra <[email protected]>
Signed-off-by: Ammar Faizi <[email protected]>
---
 src/arch/generic/lib.h | 21 ++++++++++++++++++++
 src/arch/x86/lib.h     | 20 ++++++++++++-------
 src/lib.h              | 45 ++++++++++++++++++------------------------
 3 files changed, 53 insertions(+), 33 deletions(-)
 create mode 100644 src/arch/generic/lib.h

diff --git a/src/arch/generic/lib.h b/src/arch/generic/lib.h
new file mode 100644
index 0000000..737e795
--- /dev/null
+++ b/src/arch/generic/lib.h
@@ -0,0 +1,21 @@
+/* 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
+
+static inline long get_page_size(void)
+{
+	long page_size;
+
+	page_size = sysconf(_SC_PAGESIZE);
+	if (page_size < 0)
+		page_size = 4096;
+
+	return page_size;
+}
+
+#endif /* #ifndef LIBURING_ARCH_GENERIC_LIB_H */
diff --git a/src/arch/x86/lib.h b/src/arch/x86/lib.h
index 65ad396..bacf74e 100644
--- a/src/arch/x86/lib.h
+++ b/src/arch/x86/lib.h
@@ -1,15 +1,15 @@
 /* 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
 
-#ifndef LIBURING_LIB_H
-#  error "This file should be included from src/lib.h (liburing)"
-#endif
-
 #if defined(__x86_64__)
 
-static inline long __arch_impl_get_page_size(void)
+static inline long get_page_size(void)
 {
 	return 4096;
 }
@@ -17,9 +17,15 @@ static inline long __arch_impl_get_page_size(void)
 #else /* #if defined(__x86_64__) */
 
 /*
- * TODO: Add x86 (32-bit) support here.
+ * For x86 (32-bit), fallback to libc wrapper.
+ * We can't use CONFIG_NOLIBC for x86 (32-bit) at the moment.
+ *
+ * TODO: Add x86 (32-bit) nolibc support.
  */
-#error "x86 (32-bit) is currently not supported for nolibc builds"
+#ifdef CONFIG_NOLIBC
+	#error "x86 (32-bit) is currently not supported for nolibc builds"
+#endif
+#include "../generic/lib.h"
 
 #endif /* #if defined(__x86_64__) */
 
diff --git a/src/lib.h b/src/lib.h
index bd02805..6672cc5 100644
--- a/src/lib.h
+++ b/src/lib.h
@@ -6,23 +6,31 @@
 #include <string.h>
 #include <unistd.h>
 
-#ifdef CONFIG_NOLIBC
-#  if defined(__x86_64__) || defined(__i386__)
-#    include "arch/x86/lib.h"
-#  else
-#    error "This arch doesn't support building liburing without libc"
-#  endif
+#define __INTERNAL__LIBURING_LIB_H
+#if defined(__x86_64__) || defined(__i386__)
+	#include "arch/x86/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"
 #endif
+#undef __INTERNAL__LIBURING_LIB_H
+
 
 #ifndef offsetof
-# define offsetof(TYPE, FIELD) ((size_t) &((TYPE *)0)->FIELD)
+	#define offsetof(TYPE, FIELD) ((size_t) &((TYPE *)0)->FIELD)
 #endif
 
 #ifndef container_of
-# define container_of(PTR, TYPE, FIELD) ({				\
-	__typeof__(((TYPE *)0)->FIELD) *__FIELD_PTR = (PTR);		\
-	(TYPE *)((char *) __FIELD_PTR - offsetof(TYPE, FIELD));		\
-})
+	#define container_of(PTR, TYPE, FIELD) ({			\
+		__typeof__(((TYPE *)0)->FIELD) *__FIELD_PTR = (PTR);	\
+		(TYPE *)((char *) __FIELD_PTR - offsetof(TYPE, FIELD));	\
+	})
 #endif
 
 void *__uring_malloc(size_t len);
@@ -46,19 +54,4 @@ static inline void uring_free(void *ptr)
 #endif
 }
 
-static inline long get_page_size(void)
-{
-#ifdef CONFIG_NOLIBC
-	return __arch_impl_get_page_size();
-#else
-	long page_size;
-
-	page_size = sysconf(_SC_PAGESIZE);
-	if (page_size < 0)
-		page_size = 4096;
-
-	return page_size;
-#endif
-}
-
 #endif /* #ifndef LIBURING_LIB_H */
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH liburing v1 4/4] Change all syscall function name prefix to __sys
  2022-02-11 15:57 [PATCH liburing v1 0/4] Refactor arch dependent code and x86-64 improvement Ammar Faizi
                   ` (2 preceding siblings ...)
  2022-02-11 15:57 ` [PATCH liburing v1 3/4] lib.h: Split off lib header for arch specific and generic Ammar Faizi
@ 2022-02-11 15:57 ` Ammar Faizi
  2022-02-11 16:39 ` [PATCH liburing v1 0/4] Refactor arch dependent code and x86-64 improvement Jens Axboe
  4 siblings, 0 replies; 6+ messages in thread
From: Ammar Faizi @ 2022-02-11 15:57 UTC (permalink / raw)
  To: Jens Axboe
  Cc: io-uring Mailing List, GNU/Weeb Mailing List,
	Tea Inside Mailing List, Alviro Iskandar Setiawan,
	Alviro Iskandar Setiawan, Ammar Faizi, Nugra

Instead of using uring_mmap, uring_close, uring_madvise, etc. Let's
use __sys_mmap, __sys_close, __sys_madvise, etc. That looks better
convention for syscall function name like what we do with
__sys_io_uring* functions.

Cc: Nugra <[email protected]>
Signed-off-by: Ammar Faizi <[email protected]>
---
 src/arch/generic/syscall.h | 12 ++++++------
 src/arch/x86/syscall.h     | 12 ++++++------
 src/nolibc.c               |  4 ++--
 src/register.c             |  4 ++--
 src/setup.c                | 22 +++++++++++-----------
 5 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/src/arch/generic/syscall.h b/src/arch/generic/syscall.h
index 6b10fe3..fa93064 100644
--- a/src/arch/generic/syscall.h
+++ b/src/arch/generic/syscall.h
@@ -41,7 +41,7 @@ static inline int ____sys_io_uring_enter(int fd, unsigned to_submit,
 				       _NSIG / 8);
 }
 
-static inline void *uring_mmap(void *addr, size_t length, int prot, int flags,
+static inline void *__sys_mmap(void *addr, size_t length, int prot, int flags,
 			       int fd, off_t offset)
 {
 	void *ret;
@@ -49,35 +49,35 @@ static inline void *uring_mmap(void *addr, size_t length, int prot, int flags,
 	return (ret == MAP_FAILED) ? ERR_PTR(-errno) : ret;
 }
 
-static inline int uring_munmap(void *addr, size_t length)
+static inline int __sys_munmap(void *addr, size_t length)
 {
 	int ret;
 	ret = munmap(addr, length);
 	return (ret < 0) ? -errno : ret;
 }
 
-static inline int uring_madvise(void *addr, size_t length, int advice)
+static inline int __sys_madvise(void *addr, size_t length, int advice)
 {
 	int ret;
 	ret = madvise(addr, length, advice);
 	return (ret < 0) ? -errno : ret;
 }
 
-static inline int uring_getrlimit(int resource, struct rlimit *rlim)
+static inline int __sys_getrlimit(int resource, struct rlimit *rlim)
 {
 	int ret;
 	ret = getrlimit(resource, rlim);
 	return (ret < 0) ? -errno : ret;
 }
 
-static inline int uring_setrlimit(int resource, const struct rlimit *rlim)
+static inline int __sys_setrlimit(int resource, const struct rlimit *rlim)
 {
 	int ret;
 	ret = setrlimit(resource, rlim);
 	return (ret < 0) ? -errno : ret;
 }
 
-static inline int uring_close(int fd)
+static inline int __sys_close(int fd)
 {
 	int ret;
 	ret = close(fd);
diff --git a/src/arch/x86/syscall.h b/src/arch/x86/syscall.h
index 2d5642c..02677f2 100644
--- a/src/arch/x86/syscall.h
+++ b/src/arch/x86/syscall.h
@@ -29,7 +29,7 @@
  *   %r11 == %rflags and %rcx == %rip.
  */
 
-static inline void *uring_mmap(void *addr, size_t length, int prot, int flags,
+static inline void *__sys_mmap(void *addr, size_t length, int prot, int flags,
 			       int fd, off_t offset)
 {
 	void *rax;
@@ -52,7 +52,7 @@ static inline void *uring_mmap(void *addr, size_t length, int prot, int flags,
 	return rax;
 }
 
-static inline int uring_munmap(void *addr, size_t length)
+static inline int __sys_munmap(void *addr, size_t length)
 {
 	long rax;
 
@@ -67,7 +67,7 @@ static inline int uring_munmap(void *addr, size_t length)
 	return (int) rax;
 }
 
-static inline int uring_madvise(void *addr, size_t length, int advice)
+static inline int __sys_madvise(void *addr, size_t length, int advice)
 {
 	long rax;
 
@@ -83,7 +83,7 @@ static inline int uring_madvise(void *addr, size_t length, int advice)
 	return (int) rax;
 }
 
-static inline int uring_getrlimit(int resource, struct rlimit *rlim)
+static inline int __sys_getrlimit(int resource, struct rlimit *rlim)
 {
 	long rax;
 
@@ -98,7 +98,7 @@ static inline int uring_getrlimit(int resource, struct rlimit *rlim)
 	return (int) rax;
 }
 
-static inline int uring_setrlimit(int resource, const struct rlimit *rlim)
+static inline int __sys_setrlimit(int resource, const struct rlimit *rlim)
 {
 	long rax;
 
@@ -113,7 +113,7 @@ static inline int uring_setrlimit(int resource, const struct rlimit *rlim)
 	return (int) rax;
 }
 
-static inline int uring_close(int fd)
+static inline int __sys_close(int fd)
 {
 	long rax;
 
diff --git a/src/nolibc.c b/src/nolibc.c
index f7848d3..1e17d22 100644
--- a/src/nolibc.c
+++ b/src/nolibc.c
@@ -27,7 +27,7 @@ void *__uring_malloc(size_t len)
 {
 	struct uring_heap *heap;
 
-	heap = uring_mmap(NULL, sizeof(*heap) + len, PROT_READ | PROT_WRITE,
+	heap = __sys_mmap(NULL, sizeof(*heap) + len, PROT_READ | PROT_WRITE,
 			  MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
 	if (IS_ERR(heap))
 		return NULL;
@@ -44,5 +44,5 @@ void __uring_free(void *p)
 		return;
 
 	heap = container_of(p, struct uring_heap, user_p);
-	uring_munmap(heap, heap->len);
+	__sys_munmap(heap, heap->len);
 }
diff --git a/src/register.c b/src/register.c
index a1b1a22..cd73fce 100644
--- a/src/register.c
+++ b/src/register.c
@@ -100,13 +100,13 @@ static int increase_rlimit_nofile(unsigned nr)
 	int ret;
 	struct rlimit rlim;
 
-	ret = uring_getrlimit(RLIMIT_NOFILE, &rlim);
+	ret = __sys_getrlimit(RLIMIT_NOFILE, &rlim);
 	if (ret < 0)
 		return ret;
 
 	if (rlim.rlim_cur < nr) {
 		rlim.rlim_cur += nr;
-		uring_setrlimit(RLIMIT_NOFILE, &rlim);
+		__sys_setrlimit(RLIMIT_NOFILE, &rlim);
 	}
 
 	return 0;
diff --git a/src/setup.c b/src/setup.c
index 1e4dbf4..544adaf 100644
--- a/src/setup.c
+++ b/src/setup.c
@@ -10,9 +10,9 @@
 
 static void io_uring_unmap_rings(struct io_uring_sq *sq, struct io_uring_cq *cq)
 {
-	uring_munmap(sq->ring_ptr, sq->ring_sz);
+	__sys_munmap(sq->ring_ptr, sq->ring_sz);
 	if (cq->ring_ptr && cq->ring_ptr != sq->ring_ptr)
-		uring_munmap(cq->ring_ptr, cq->ring_sz);
+		__sys_munmap(cq->ring_ptr, cq->ring_sz);
 }
 
 static int io_uring_mmap(int fd, struct io_uring_params *p,
@@ -29,7 +29,7 @@ static int io_uring_mmap(int fd, struct io_uring_params *p,
 			sq->ring_sz = cq->ring_sz;
 		cq->ring_sz = sq->ring_sz;
 	}
-	sq->ring_ptr = uring_mmap(0, sq->ring_sz, PROT_READ | PROT_WRITE,
+	sq->ring_ptr = __sys_mmap(0, sq->ring_sz, PROT_READ | PROT_WRITE,
 				  MAP_SHARED | MAP_POPULATE, fd,
 				  IORING_OFF_SQ_RING);
 	if (IS_ERR(sq->ring_ptr))
@@ -38,7 +38,7 @@ static int io_uring_mmap(int fd, struct io_uring_params *p,
 	if (p->features & IORING_FEAT_SINGLE_MMAP) {
 		cq->ring_ptr = sq->ring_ptr;
 	} else {
-		cq->ring_ptr = uring_mmap(0, cq->ring_sz, PROT_READ | PROT_WRITE,
+		cq->ring_ptr = __sys_mmap(0, cq->ring_sz, PROT_READ | PROT_WRITE,
 					  MAP_SHARED | MAP_POPULATE, fd,
 					  IORING_OFF_CQ_RING);
 		if (IS_ERR(cq->ring_ptr)) {
@@ -57,7 +57,7 @@ static int io_uring_mmap(int fd, struct io_uring_params *p,
 	sq->array = sq->ring_ptr + p->sq_off.array;
 
 	size = p->sq_entries * sizeof(struct io_uring_sqe);
-	sq->sqes = uring_mmap(0, size, PROT_READ | PROT_WRITE,
+	sq->sqes = __sys_mmap(0, size, PROT_READ | PROT_WRITE,
 			      MAP_SHARED | MAP_POPULATE, fd, IORING_OFF_SQES);
 	if (IS_ERR(sq->sqes)) {
 		ret = PTR_ERR(sq->sqes);
@@ -109,18 +109,18 @@ int io_uring_ring_dontfork(struct io_uring *ring)
 		return -EINVAL;
 
 	len = *ring->sq.kring_entries * sizeof(struct io_uring_sqe);
-	ret = uring_madvise(ring->sq.sqes, len, MADV_DONTFORK);
+	ret = __sys_madvise(ring->sq.sqes, len, MADV_DONTFORK);
 	if (ret < 0)
 		return ret;
 
 	len = ring->sq.ring_sz;
-	ret = uring_madvise(ring->sq.ring_ptr, len, MADV_DONTFORK);
+	ret = __sys_madvise(ring->sq.ring_ptr, len, MADV_DONTFORK);
 	if (ret < 0)
 		return ret;
 
 	if (ring->cq.ring_ptr != ring->sq.ring_ptr) {
 		len = ring->cq.ring_sz;
-		ret = uring_madvise(ring->cq.ring_ptr, len, MADV_DONTFORK);
+		ret = __sys_madvise(ring->cq.ring_ptr, len, MADV_DONTFORK);
 		if (ret < 0)
 			return ret;
 	}
@@ -139,7 +139,7 @@ int io_uring_queue_init_params(unsigned entries, struct io_uring *ring,
 
 	ret = io_uring_queue_mmap(fd, p, ring);
 	if (ret) {
-		uring_close(fd);
+		__sys_close(fd);
 		return ret;
 	}
 
@@ -166,9 +166,9 @@ void io_uring_queue_exit(struct io_uring *ring)
 	struct io_uring_sq *sq = &ring->sq;
 	struct io_uring_cq *cq = &ring->cq;
 
-	uring_munmap(sq->sqes, *sq->kring_entries * sizeof(struct io_uring_sqe));
+	__sys_munmap(sq->sqes, *sq->kring_entries * sizeof(struct io_uring_sqe));
 	io_uring_unmap_rings(sq, cq);
-	uring_close(ring->ring_fd);
+	__sys_close(ring->ring_fd);
 }
 
 struct io_uring_probe *io_uring_get_probe_ring(struct io_uring *ring)
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH liburing v1 0/4] Refactor arch dependent code and x86-64 improvement
  2022-02-11 15:57 [PATCH liburing v1 0/4] Refactor arch dependent code and x86-64 improvement Ammar Faizi
                   ` (3 preceding siblings ...)
  2022-02-11 15:57 ` [PATCH liburing v1 4/4] Change all syscall function name prefix to __sys Ammar Faizi
@ 2022-02-11 16:39 ` Jens Axboe
  4 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2022-02-11 16:39 UTC (permalink / raw)
  To: Ammar Faizi
  Cc: Nugra, GNU/Weeb Mailing List, Tea Inside Mailing List,
	io-uring Mailing List, Alviro Iskandar Setiawan,
	Alviro Iskandar Setiawan

On Fri, 11 Feb 2022 22:57:49 +0700, Ammar Faizi wrote:
> We have many #ifdef/#endif in syscall.h since nolibc support is added.
> This series tries to clean them up, separate the definitions into
> smaller more manageable pieces.
> 
> Also, optimize function call for x86-64. Avoid libc function call for
> syscall even if CONFIG_NOLIBC is disabled. If this patchset is applied,
> CONFIG_NOLIBC is still meaningful, we may still use libc for malloc(),
> free() and memset().
> 
> [...]

Applied, thanks!

[1/4] arch/generic: Create arch generic syscall wrappers
      commit: d238216f0d45d7670d7aa10e753ac049c2b9bd61
[2/4] arch/x86, syscall: Refactor arch specific and generic syscall wrappers
      commit: 8347a3d9553a2f31affddacb7bd9eaa14f2e7ed7
[3/4] lib.h: Split off lib header for arch specific and generic
      commit: c099b832a97dc1880b89734ef6a5420497a1be0f
[4/4] Change all syscall function name prefix to __sys
      commit: e1f89765f957accc4c9a0e3ca233532c6564548b

Best regards,
-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-02-11 16:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-11 15:57 [PATCH liburing v1 0/4] Refactor arch dependent code and x86-64 improvement Ammar Faizi
2022-02-11 15:57 ` [PATCH liburing v1 1/4] arch/generic: Create arch generic syscall wrappers Ammar Faizi
2022-02-11 15:57 ` [PATCH liburing v1 2/4] arch/x86, syscall: Refactor arch specific and " Ammar Faizi
2022-02-11 15:57 ` [PATCH liburing v1 3/4] lib.h: Split off lib header for arch specific and generic Ammar Faizi
2022-02-11 15:57 ` [PATCH liburing v1 4/4] Change all syscall function name prefix to __sys Ammar Faizi
2022-02-11 16:39 ` [PATCH liburing v1 0/4] Refactor arch dependent code and x86-64 improvement Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox