[PATCHSET v3 RFC liburing 0/4] Implement the kernel style return value

[PATCHSET v3 RFC liburing 0/4] Implement the kernel style return value

[Ammar Faizi]

Hi everyone,

Sorry, v3 just to remove duplicate Signed-off-by.

This is the v3 of RFC to implement the kernel style return value for
liburing.

The main purpose of these changes is to make it possible to remove
the dependency of `errno` variable. If we can land this on liburing,
we will start working on no libc support with raw system call written
in Assembly (no libc). These changes should not affect the user, only
affect liburing internal sources.

We do not plan to drop the libc dependency, we just want to make it
possible to build liburing without the libc.

Currently, we expose these functions to userland:
  1) `__sys_io_uring_register`
  2) `__sys_io_uring_setup`
  3) `__sys_io_uring_enter2`
  4) `__sys_io_uring_enter`

The tests in `test/io_uring_{enter,register,setup}.c` are the examples
of it. Since the userland needs to check the `errno` value to use them
properly, this means those functions always depend on libc. So we
cannot change their behavior. This ensures the changes only affect
liburing internal and no visible functionality changes for the users.

Then we introduce new functions with the same name (with extra
underscore as prefix, 4 underscores):
  1) `____sys_io_uring_register`
  2) `____sys_io_uring_setup`
  3) `____sys_io_uring_enter2`
  4) `____sys_io_uring_enter`
    
These functions do not use `errno` variable *on the caller*, they use
the kernel style return value (return a negative value of error code
when errors).

These functions are defined as `inline static` in `src/syscall.h`.
They are just a wrapper to make sure liburing internal sources do not
touch `errno` variable from C files directly.

After that, we need to deal with other syscalls. Currently we have 5
wrapper functions for all syscalls used in liburing. They are:
  1) `liburing_mmap`
  2) `liburing_munmap`
  3) `liburing_madvise`
  4) `liburing_getrlimit`
  5) `liburing_setrlimit`

Also add kernel error header `src/kernel_err.h`, this is taken from
the Linux kernel source `include/linux/err.h` with a bit modification.

The purpose of `src/kernel_err.h` file is to use `PTR_ERR()`,
`ERR_PTR()`, etc. to implement the kernel style return value for
pointer return value. Currently only `liburing_mmap()` that depends
on this kernel error header file.

If you want a git repository, you can take a look at:
    
    git://github.com/ammarfaizi2/liburing.git kernel-style-retval

Please review.
----------------------------------------------------------------
Changes since v1:
- Make all wrapper functions be `static inline`, so they don't pollute
  the global scope.
- Reduce the number of patches. Now we only have 4 patches (it was 6).

Changes since v2:
- Remove duplicate Signed-off-by.

Link: https://github.com/axboe/liburing/issues/443
Link: [v1] https://lore.kernel.org/io-uring/20210929101606.62822-1-ammar.faizi@students.amikom.ac.id/T/
Link: [v2] https://lore.kernel.org/io-uring/20211002012817.107517-1-ammar.faizi@students.amikom.ac.id/T
----------------------------------------------------------------
Ammar Faizi (4):
      src/syscall: Implement the kernel style return value
      src/{queue,register,setup}: Don't use `__sys_io_uring*`
      Wrap all syscalls in a kernel style return value
      src/{queue,register,setup}: Remove `#include <errno.h>`

 src/kernel_err.h |  75 ++++++++++++++++++
 src/queue.c      |  28 +++----
 src/register.c   | 190 ++++++++++++++-------------------------------
 src/setup.c      |  61 ++++++++-------
 src/syscall.c    |  42 +---------
 src/syscall.h    | 130 +++++++++++++++++++++++++++++++
 6 files changed, 310 insertions(+), 216 deletions(-)
 create mode 100644 src/kernel_err.h

--
Ammar Faizi

[PATCH v3 RFC liburing 1/4] src/syscall: Implement the kernel style return value

[Ammar Faizi]

Make it possible to remove the dependency of `errno` variable (which
comes from libc).

Currently, we expose these functions to userland:
  1) `__sys_io_uring_register`
  2) `__sys_io_uring_setup`
  3) `__sys_io_uring_enter2`
  4) `__sys_io_uring_enter`

The tests in `test/io_uring_{enter,register,setup}.c` are the examples
of it. Since the userland needs to check the `errno` value to use them
properly, this means those functions always depend on libc. So we
cannot change their behavior. This ensures the changes only affect
liburing internal and no visible functionality changes for the users.

Then we introduce new functions with the same name (with extra
underscore as prefix, 4 underscores):
  1) `____sys_io_uring_register`
  2) `____sys_io_uring_setup`
  3) `____sys_io_uring_enter2`
  4) `____sys_io_uring_enter`

These functions do not use `errno` variable *on the caller*, they use
the kernel style return value (return a negative value of error code
when errors).

These functions are defined as `inline static` in `src/syscall.h`.
They are just a wrapper to make sure liburing internal sources do not
touch `errno` variable from C files directly.

Link: https://github.com/axboe/liburing/issues/443#issuecomment-927873932
Cc: Bedirhan KURT <windowz414@gnuweeb.org>
Suggested-by: Louvian Lyndal <louvianlyndal@gmail.com>
Signed-off-by: Ammar Faizi <ammar.faizi@students.amikom.ac.id>
---
 src/syscall.c | 36 ----------------------
 src/syscall.h | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 82 insertions(+), 36 deletions(-)

diff --git a/src/syscall.c b/src/syscall.c
index 69027e5..221f0f1 100644
--- a/src/syscall.c
+++ b/src/syscall.c
@@ -5,47 +5,11 @@
  * Will go away once libc support is there
  */
 #include <unistd.h>
-#include <sys/syscall.h>
 #include <sys/uio.h>
 #include "liburing/compat.h"
 #include "liburing/io_uring.h"
 #include "syscall.h"
 
-#ifdef __alpha__
-/*
- * alpha and mips are exception, other architectures have
- * common numbers for new system calls.
- */
-# ifndef __NR_io_uring_setup
-#  define __NR_io_uring_setup		535
-# endif
-# ifndef __NR_io_uring_enter
-#  define __NR_io_uring_enter		536
-# endif
-# ifndef __NR_io_uring_register
-#  define __NR_io_uring_register	537
-# endif
-#elif defined __mips__
-# ifndef __NR_io_uring_setup
-#  define __NR_io_uring_setup           (__NR_Linux + 425)
-# endif
-# ifndef __NR_io_uring_enter
-#  define __NR_io_uring_enter           (__NR_Linux + 426)
-# endif
-# ifndef __NR_io_uring_register
-#  define __NR_io_uring_register        (__NR_Linux + 427)
-# endif
-#else /* !__alpha__ and !__mips__ */
-# ifndef __NR_io_uring_setup
-#  define __NR_io_uring_setup		425
-# endif
-# ifndef __NR_io_uring_enter
-#  define __NR_io_uring_enter		426
-# endif
-# ifndef __NR_io_uring_register
-#  define __NR_io_uring_register	427
-# endif
-#endif
 
 int __sys_io_uring_register(int fd, unsigned opcode, const void *arg,
 			    unsigned nr_args)
diff --git a/src/syscall.h b/src/syscall.h
index 2368f83..5f7343f 100644
--- a/src/syscall.h
+++ b/src/syscall.h
@@ -2,7 +2,47 @@
 #ifndef LIBURING_SYSCALL_H
 #define LIBURING_SYSCALL_H
 
+#include <errno.h>
 #include <signal.h>
+#include <unistd.h>
+#include <sys/syscall.h>
+
+#ifdef __alpha__
+/*
+ * alpha and mips are exception, other architectures have
+ * common numbers for new system calls.
+ */
+# ifndef __NR_io_uring_setup
+#  define __NR_io_uring_setup		535
+# endif
+# ifndef __NR_io_uring_enter
+#  define __NR_io_uring_enter		536
+# endif
+# ifndef __NR_io_uring_register
+#  define __NR_io_uring_register	537
+# endif
+#elif defined __mips__
+# ifndef __NR_io_uring_setup
+#  define __NR_io_uring_setup           (__NR_Linux + 425)
+# endif
+# ifndef __NR_io_uring_enter
+#  define __NR_io_uring_enter           (__NR_Linux + 426)
+# endif
+# ifndef __NR_io_uring_register
+#  define __NR_io_uring_register        (__NR_Linux + 427)
+# endif
+#else /* !__alpha__ and !__mips__ */
+# ifndef __NR_io_uring_setup
+#  define __NR_io_uring_setup		425
+# endif
+# ifndef __NR_io_uring_enter
+#  define __NR_io_uring_enter		426
+# endif
+# ifndef __NR_io_uring_register
+#  define __NR_io_uring_register	427
+# endif
+#endif
+
 
 struct io_uring_params;
 
@@ -17,4 +57,46 @@ int __sys_io_uring_enter2(int fd, unsigned to_submit, unsigned min_complete,
 int __sys_io_uring_register(int fd, unsigned int opcode, const void *arg,
 			    unsigned int nr_args);
 
+
+
+/*
+ * Syscall with kernel style return value.
+ */
+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);
+}
+
 #endif
-- 
2.30.2

[PATCH v3 RFC liburing 2/4] src/{queue,register,setup}: Don't use `__sys_io_uring*`

[Ammar Faizi]

Don't use `__sys_io_uring*` for liburing internal. These functions
are now supposed for user backward compatibility. Instead, we use
`____sys_io_uring*` (4 underscores). These are `static inline`
functions that wrap the `errno` variable with the kernel style return
value (directly returns negative error code when errors).

The main purpose of this change is to make it possible to remove the
`errno` variable dependency from liburing C sources, so that later
we will be able to implement no libc environment which doesn't use
`errno` variable at all.

Cc: Bedirhan KURT <windowz414@gnuweeb.org>
Cc: Louvian Lyndal <louvianlyndal@gmail.com>
Signed-off-by: Ammar Faizi <ammar.faizi@students.amikom.ac.id>
---
 src/queue.c    |  27 +++-----
 src/register.c | 184 ++++++++++++++++---------------------------------
 src/setup.c    |   4 +-
 src/syscall.c  |   6 +-
 4 files changed, 73 insertions(+), 148 deletions(-)

diff --git a/src/queue.c b/src/queue.c
index 10ef31c..e85ea1d 100644
--- a/src/queue.c
+++ b/src/queue.c
@@ -117,11 +117,11 @@ static int _io_uring_get_cqe(struct io_uring *ring, struct io_uring_cqe **cqe_pt
 		if (!need_enter)
 			break;
 
-		ret = __sys_io_uring_enter2(ring->ring_fd, data->submit,
-				data->wait_nr, flags, data->arg,
-				data->sz);
+		ret = ____sys_io_uring_enter2(ring->ring_fd, data->submit,
+					      data->wait_nr, flags, data->arg,
+					      data->sz);
 		if (ret < 0) {
-			err = -errno;
+			err = ret;
 			break;
 		}
 
@@ -178,8 +178,8 @@ again:
 		goto done;
 
 	if (cq_ring_needs_flush(ring)) {
-		__sys_io_uring_enter(ring->ring_fd, 0, 0,
-				     IORING_ENTER_GETEVENTS, NULL);
+		____sys_io_uring_enter(ring->ring_fd, 0, 0,
+				       IORING_ENTER_GETEVENTS, NULL);
 		overflow_checked = true;
 		goto again;
 	}
@@ -333,10 +333,8 @@ static int __io_uring_submit(struct io_uring *ring, unsigned submitted,
 		if (wait_nr || (ring->flags & IORING_SETUP_IOPOLL))
 			flags |= IORING_ENTER_GETEVENTS;
 
-		ret = __sys_io_uring_enter(ring->ring_fd, submitted, wait_nr,
-						flags, NULL);
-		if (ret < 0)
-			return -errno;
+		ret = ____sys_io_uring_enter(ring->ring_fd, submitted, wait_nr,
+					     flags, NULL);
 	} else
 		ret = submitted;
 
@@ -391,11 +389,6 @@ struct io_uring_sqe *io_uring_get_sqe(struct io_uring *ring)
 
 int __io_uring_sqring_wait(struct io_uring *ring)
 {
-	int ret;
-
-	ret = __sys_io_uring_enter(ring->ring_fd, 0, 0, IORING_ENTER_SQ_WAIT,
-					NULL);
-	if (ret < 0)
-		ret = -errno;
-	return ret;
+	return ____sys_io_uring_enter(ring->ring_fd, 0, 0, IORING_ENTER_SQ_WAIT,
+				      NULL);
 }
diff --git a/src/register.c b/src/register.c
index 5ea4331..944852e 100644
--- a/src/register.c
+++ b/src/register.c
@@ -26,12 +26,10 @@ int io_uring_register_buffers_update_tag(struct io_uring *ring, unsigned off,
 		.tags = (unsigned long)tags,
 		.nr = nr,
 	};
-	int ret;
 
-	ret = __sys_io_uring_register(ring->ring_fd,
-				      IORING_REGISTER_BUFFERS_UPDATE,
-				      &up, sizeof(up));
-	return ret < 0 ? -errno : ret;
+	return ____sys_io_uring_register(ring->ring_fd,
+					 IORING_REGISTER_BUFFERS_UPDATE, &up,
+					 sizeof(up));
 }
 
 int io_uring_register_buffers_tags(struct io_uring *ring,
@@ -44,11 +42,10 @@ int io_uring_register_buffers_tags(struct io_uring *ring,
 		.data = (unsigned long)iovecs,
 		.tags = (unsigned long)tags,
 	};
-	int ret;
 
-	ret = __sys_io_uring_register(ring->ring_fd, IORING_REGISTER_BUFFERS2,
-				      &reg, sizeof(reg));
-	return ret < 0 ? -errno : ret;
+	return ____sys_io_uring_register(ring->ring_fd,
+					 IORING_REGISTER_BUFFERS2, &reg,
+					 sizeof(reg));
 }
 
 int io_uring_register_buffers(struct io_uring *ring, const struct iovec *iovecs,
@@ -56,24 +53,18 @@ int io_uring_register_buffers(struct io_uring *ring, const struct iovec *iovecs,
 {
 	int ret;
 
-	ret = __sys_io_uring_register(ring->ring_fd, IORING_REGISTER_BUFFERS,
+	ret = ____sys_io_uring_register(ring->ring_fd, IORING_REGISTER_BUFFERS,
 					iovecs, nr_iovecs);
-	if (ret < 0)
-		return -errno;
-
-	return 0;
+	return (ret < 0) ? ret : 0;
 }
 
 int io_uring_unregister_buffers(struct io_uring *ring)
 {
 	int ret;
 
-	ret = __sys_io_uring_register(ring->ring_fd, IORING_UNREGISTER_BUFFERS,
+	ret = ____sys_io_uring_register(ring->ring_fd, IORING_UNREGISTER_BUFFERS,
 					NULL, 0);
-	if (ret < 0)
-		return -errno;
-
-	return 0;
+	return (ret < 0) ? ret : 0;
 }
 
 int io_uring_register_files_update_tag(struct io_uring *ring, unsigned off,
@@ -86,12 +77,10 @@ int io_uring_register_files_update_tag(struct io_uring *ring, unsigned off,
 		.tags = (unsigned long)tags,
 		.nr = nr_files,
 	};
-	int ret;
 
-	ret = __sys_io_uring_register(ring->ring_fd,
-					IORING_REGISTER_FILES_UPDATE2,
-					&up, sizeof(up));
-	return ret < 0 ? -errno : ret;
+	return ____sys_io_uring_register(ring->ring_fd,
+					 IORING_REGISTER_FILES_UPDATE2, &up,
+					 sizeof(up));
 }
 
 /*
@@ -108,15 +97,10 @@ int io_uring_register_files_update(struct io_uring *ring, unsigned off,
 		.offset	= off,
 		.fds	= (unsigned long) files,
 	};
-	int ret;
-
-	ret = __sys_io_uring_register(ring->ring_fd,
-					IORING_REGISTER_FILES_UPDATE, &up,
-					nr_files);
-	if (ret < 0)
-		return -errno;
 
-	return ret;
+	return ____sys_io_uring_register(ring->ring_fd,
+					 IORING_REGISTER_FILES_UPDATE, &up,
+					 nr_files);
 }
 
 static int increase_rlimit_nofile(unsigned nr)
@@ -145,12 +129,12 @@ int io_uring_register_files_tags(struct io_uring *ring,
 	int ret, did_increase = 0;
 
 	do {
-		ret = __sys_io_uring_register(ring->ring_fd,
-					      IORING_REGISTER_FILES2, &reg,
-					      sizeof(reg));
+		ret = ____sys_io_uring_register(ring->ring_fd,
+						IORING_REGISTER_FILES2, &reg,
+						sizeof(reg));
 		if (ret >= 0)
 			break;
-		if (errno == EMFILE && !did_increase) {
+		if (ret == -EMFILE && !did_increase) {
 			did_increase = 1;
 			increase_rlimit_nofile(nr);
 			continue;
@@ -158,7 +142,7 @@ int io_uring_register_files_tags(struct io_uring *ring,
 		break;
 	} while (1);
 
-	return ret < 0 ? -errno : ret;
+	return (ret < 0) ? ret : 0;
 }
 
 int io_uring_register_files(struct io_uring *ring, const int *files,
@@ -167,12 +151,12 @@ int io_uring_register_files(struct io_uring *ring, const int *files,
 	int ret, did_increase = 0;
 
 	do {
-		ret = __sys_io_uring_register(ring->ring_fd,
-					      IORING_REGISTER_FILES, files,
-					      nr_files);
+		ret = ____sys_io_uring_register(ring->ring_fd,
+						IORING_REGISTER_FILES, files,
+						nr_files);
 		if (ret >= 0)
 			break;
-		if (errno == EMFILE && !did_increase) {
+		if (ret == -EMFILE && !did_increase) {
 			did_increase = 1;
 			increase_rlimit_nofile(nr_files);
 			continue;
@@ -180,55 +164,44 @@ int io_uring_register_files(struct io_uring *ring, const int *files,
 		break;
 	} while (1);
 
-	return ret < 0 ? -errno : ret;
+	return (ret < 0) ? ret : 0;
 }
 
 int io_uring_unregister_files(struct io_uring *ring)
 {
 	int ret;
 
-	ret = __sys_io_uring_register(ring->ring_fd, IORING_UNREGISTER_FILES,
+	ret = ____sys_io_uring_register(ring->ring_fd, IORING_UNREGISTER_FILES,
 					NULL, 0);
-	if (ret < 0)
-		return -errno;
-
-	return 0;
+	return (ret < 0) ? ret : 0;
 }
 
 int io_uring_register_eventfd(struct io_uring *ring, int event_fd)
 {
 	int ret;
 
-	ret = __sys_io_uring_register(ring->ring_fd, IORING_REGISTER_EVENTFD,
+	ret = ____sys_io_uring_register(ring->ring_fd, IORING_REGISTER_EVENTFD,
 					&event_fd, 1);
-	if (ret < 0)
-		return -errno;
-
-	return 0;
+	return (ret < 0) ? ret : 0;
 }
 
 int io_uring_unregister_eventfd(struct io_uring *ring)
 {
 	int ret;
 
-	ret = __sys_io_uring_register(ring->ring_fd, IORING_UNREGISTER_EVENTFD,
-					NULL, 0);
-	if (ret < 0)
-		return -errno;
-
-	return 0;
+	ret = ____sys_io_uring_register(ring->ring_fd,
+					IORING_UNREGISTER_EVENTFD, NULL, 0);
+	return (ret < 0) ? ret : 0;
 }
 
 int io_uring_register_eventfd_async(struct io_uring *ring, int event_fd)
 {
 	int ret;
 
-	ret = __sys_io_uring_register(ring->ring_fd, IORING_REGISTER_EVENTFD_ASYNC,
-			&event_fd, 1);
-	if (ret < 0)
-		return -errno;
-
-	return 0;
+	ret = ____sys_io_uring_register(ring->ring_fd,
+					IORING_REGISTER_EVENTFD_ASYNC,
+					&event_fd, 1);
+	return (ret < 0) ? ret : 0;
 }
 
 int io_uring_register_probe(struct io_uring *ring, struct io_uring_probe *p,
@@ -236,36 +209,22 @@ int io_uring_register_probe(struct io_uring *ring, struct io_uring_probe *p,
 {
 	int ret;
 
-	ret = __sys_io_uring_register(ring->ring_fd, IORING_REGISTER_PROBE,
-					p, nr_ops);
-	if (ret < 0)
-		return -errno;
-
-	return 0;
+	ret = ____sys_io_uring_register(ring->ring_fd, IORING_REGISTER_PROBE, p,
+					nr_ops);
+	return (ret < 0) ? ret : 0;
 }
 
 int io_uring_register_personality(struct io_uring *ring)
 {
-	int ret;
-
-	ret = __sys_io_uring_register(ring->ring_fd, IORING_REGISTER_PERSONALITY,
-					NULL, 0);
-	if (ret < 0)
-		return -errno;
-
-	return ret;
+	return ____sys_io_uring_register(ring->ring_fd,
+					 IORING_REGISTER_PERSONALITY, NULL, 0);
 }
 
 int io_uring_unregister_personality(struct io_uring *ring, int id)
 {
-	int ret;
-
-	ret = __sys_io_uring_register(ring->ring_fd, IORING_UNREGISTER_PERSONALITY,
-					NULL, id);
-	if (ret < 0)
-		return -errno;
-
-	return ret;
+	return ____sys_io_uring_register(ring->ring_fd,
+					 IORING_UNREGISTER_PERSONALITY, NULL,
+					 id);
 }
 
 int io_uring_register_restrictions(struct io_uring *ring,
@@ -274,61 +233,34 @@ int io_uring_register_restrictions(struct io_uring *ring,
 {
 	int ret;
 
-	ret = __sys_io_uring_register(ring->ring_fd, IORING_REGISTER_RESTRICTIONS,
-				      res, nr_res);
-	if (ret < 0)
-		return -errno;
-
-	return 0;
+	ret = ____sys_io_uring_register(ring->ring_fd,
+					IORING_REGISTER_RESTRICTIONS, res,
+					nr_res);
+	return (ret < 0) ? ret : 0;
 }
 
 int io_uring_enable_rings(struct io_uring *ring)
 {
-	int ret;
-
-	ret = __sys_io_uring_register(ring->ring_fd,
-				      IORING_REGISTER_ENABLE_RINGS, NULL, 0);
-	if (ret < 0)
-		return -errno;
-
-	return ret;
+	return ____sys_io_uring_register(ring->ring_fd,
+					 IORING_REGISTER_ENABLE_RINGS, NULL, 0);
 }
 
 int io_uring_register_iowq_aff(struct io_uring *ring, size_t cpusz,
 			       const cpu_set_t *mask)
 {
-	int ret;
-
-	ret = __sys_io_uring_register(ring->ring_fd,
-					IORING_REGISTER_IOWQ_AFF, mask, cpusz);
-	if (ret < 0)
-		return -errno;
-
-	return ret;
+	return ____sys_io_uring_register(ring->ring_fd,
+					 IORING_REGISTER_IOWQ_AFF, mask, cpusz);
 }
 
 int io_uring_unregister_iowq_aff(struct io_uring *ring)
 {
-	int ret;
-
-	ret = __sys_io_uring_register(ring->ring_fd,
-					IORING_REGISTER_IOWQ_AFF, NULL, 0);
-	if (ret < 0)
-		return -errno;
-
-	return ret;
+	return ____sys_io_uring_register(ring->ring_fd,
+					 IORING_REGISTER_IOWQ_AFF, NULL, 0);
 }
 
 int io_uring_register_iowq_max_workers(struct io_uring *ring, unsigned int *val)
 {
-	int ret;
-
-	ret = __sys_io_uring_register(ring->ring_fd,
-					IORING_REGISTER_IOWQ_MAX_WORKERS,
-					val, 2);
-	if (ret < 0)
-		return -errno;
-
-	return ret;
-
+	return ____sys_io_uring_register(ring->ring_fd,
+					 IORING_REGISTER_IOWQ_MAX_WORKERS, val,
+					 2);
 }
diff --git a/src/setup.c b/src/setup.c
index 54225e8..edfe94e 100644
--- a/src/setup.c
+++ b/src/setup.c
@@ -140,9 +140,9 @@ int io_uring_queue_init_params(unsigned entries, struct io_uring *ring,
 {
 	int fd, ret;
 
-	fd = __sys_io_uring_setup(entries, p);
+	fd = ____sys_io_uring_setup(entries, p);
 	if (fd < 0)
-		return -errno;
+		return fd;
 
 	ret = io_uring_queue_mmap(fd, p, ring);
 	if (ret) {
diff --git a/src/syscall.c b/src/syscall.c
index 221f0f1..dfadf83 100644
--- a/src/syscall.c
+++ b/src/syscall.c
@@ -25,13 +25,13 @@ int __sys_io_uring_setup(unsigned entries, struct io_uring_params *p)
 int __sys_io_uring_enter2(int fd, unsigned to_submit, unsigned min_complete,
 			 unsigned flags, sigset_t *sig, int sz)
 {
-	return syscall(__NR_io_uring_enter, fd, to_submit, min_complete,
-			flags, sig, sz);
+	return syscall(__NR_io_uring_enter, fd, to_submit, min_complete, flags,
+		       sig, sz);
 }
 
 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);
+				     _NSIG / 8);
 }
-- 
2.30.2

[PATCH v3 RFC liburing 3/4] Wrap all syscalls in a kernel style return value

[Ammar Faizi]

Add new syscall wrapper functions (5):
  1) `liburing_mmap`
  2) `liburing_munmap`
  3) `liburing_madvise`
  4) `liburing_getrlimit`
  5) `liburing_setrlimit`

All of them are `static inline`.

Use them to wrap the syscalls in a kernel style return value. The
main purpose of this change is to make it possible to remove the
dependency of `errno` variable in liburing sources (so that later,
we can support no libc environment).

Also add kernel error header `src/kernel_err.h`, this is taken from
the Linux kernel source `include/linux/err.h` with a bit modification.

The purpose of `src/kernel_err.h` file is to use `PTR_ERR()`,
`ERR_PTR()`, etc. to implement the kernel style return value (for
pointer return value). Currently only `liburing_mmap()` that depends
on this kernel error header file.

A bit modification summary on kernel erro header file:
  1) Add `__must_check` attribute macro.
  2) `#include <liburing.h>` to take the `uring_likely` and
     `uring_unlikely` macros.

Link: https://github.com/axboe/liburing/issues/443#issuecomment-927873932
Cc: Bedirhan KURT <windowz414@gnuweeb.org>
Suggested-by: Louvian Lyndal <louvianlyndal@gmail.com>
Signed-off-by: Ammar Faizi <ammar.faizi@students.amikom.ac.id>
---
 src/kernel_err.h | 75 ++++++++++++++++++++++++++++++++++++++++++++++++
 src/register.c   |  5 ++--
 src/setup.c      | 56 +++++++++++++++++++-----------------
 src/syscall.h    | 48 +++++++++++++++++++++++++++++++
 4 files changed, 155 insertions(+), 29 deletions(-)
 create mode 100644 src/kernel_err.h

diff --git a/src/kernel_err.h b/src/kernel_err.h
new file mode 100644
index 0000000..b9ea5fe
--- /dev/null
+++ b/src/kernel_err.h
@@ -0,0 +1,75 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_ERR_H
+#define _LINUX_ERR_H
+
+#include <linux/types.h>
+
+#include <asm/errno.h>
+
+#include <stdbool.h>
+#include <liburing.h>
+
+/*
+ * Kernel pointers have redundant information, so we can use a
+ * scheme where we can return either an error code or a normal
+ * pointer with the same return value.
+ *
+ * This should be a per-architecture thing, to allow different
+ * error and pointer decisions.
+ */
+#define MAX_ERRNO	4095
+
+#ifndef __ASSEMBLY__
+
+#define IS_ERR_VALUE(x) uring_unlikely((unsigned long)(void *)(x) >= (unsigned long)-MAX_ERRNO)
+
+/*
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-warn_005funused_005fresult-function-attribute
+ * clang: https://clang.llvm.org/docs/AttributeReference.html#nodiscard-warn-unused-result
+ */
+#define __must_check __attribute__((__warn_unused_result__))
+
+static inline void * __must_check ERR_PTR(long error)
+{
+	return (void *) error;
+}
+
+static inline long __must_check PTR_ERR(const void *ptr)
+{
+	return (long) ptr;
+}
+
+static inline bool __must_check IS_ERR(const void *ptr)
+{
+	return IS_ERR_VALUE((unsigned long)ptr);
+}
+
+static inline bool __must_check IS_ERR_OR_NULL(const void *ptr)
+{
+	return uring_unlikely(!ptr) || IS_ERR_VALUE((unsigned long)ptr);
+}
+
+/**
+ * ERR_CAST - Explicitly cast an error-valued pointer to another pointer type
+ * @ptr: The pointer to cast.
+ *
+ * Explicitly cast an error-valued pointer to another pointer type in such a
+ * way as to make it clear that's what's going on.
+ */
+static inline void * __must_check ERR_CAST(const void *ptr)
+{
+	/* cast away the const */
+	return (void *) ptr;
+}
+
+static inline int __must_check PTR_ERR_OR_ZERO(const void *ptr)
+{
+	if (IS_ERR(ptr))
+		return PTR_ERR(ptr);
+	else
+		return 0;
+}
+
+#endif /* #ifndef __ASSEMBLY__ */
+
+#endif /* #ifndef _LINUX_ERR_H */
diff --git a/src/register.c b/src/register.c
index 944852e..770a672 100644
--- a/src/register.c
+++ b/src/register.c
@@ -4,7 +4,6 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/mman.h>
-#include <sys/resource.h>
 #include <unistd.h>
 #include <errno.h>
 #include <string.h>
@@ -107,11 +106,11 @@ static int increase_rlimit_nofile(unsigned nr)
 {
 	struct rlimit rlim;
 
-	if (getrlimit(RLIMIT_NOFILE, &rlim) < 0)
+	if (liburing_getrlimit(RLIMIT_NOFILE, &rlim) < 0)
 		return -errno;
 	if (rlim.rlim_cur < nr) {
 		rlim.rlim_cur += nr;
-		setrlimit(RLIMIT_NOFILE, &rlim);
+		liburing_setrlimit(RLIMIT_NOFILE, &rlim);
 	}
 
 	return 0;
diff --git a/src/setup.c b/src/setup.c
index edfe94e..7476e1e 100644
--- a/src/setup.c
+++ b/src/setup.c
@@ -3,7 +3,6 @@
 
 #include <sys/types.h>
 #include <sys/stat.h>
-#include <sys/mman.h>
 #include <unistd.h>
 #include <errno.h>
 #include <string.h>
@@ -15,12 +14,13 @@
 #include "liburing.h"
 
 #include "syscall.h"
+#include "kernel_err.h"
 
 static void io_uring_unmap_rings(struct io_uring_sq *sq, struct io_uring_cq *cq)
 {
-	munmap(sq->ring_ptr, sq->ring_sz);
+	liburing_munmap(sq->ring_ptr, sq->ring_sz);
 	if (cq->ring_ptr && cq->ring_ptr != sq->ring_ptr)
-		munmap(cq->ring_ptr, cq->ring_sz);
+		liburing_munmap(cq->ring_ptr, cq->ring_sz);
 }
 
 static int io_uring_mmap(int fd, struct io_uring_params *p,
@@ -37,19 +37,22 @@ 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 = mmap(0, sq->ring_sz, PROT_READ | PROT_WRITE,
-			MAP_SHARED | MAP_POPULATE, fd, IORING_OFF_SQ_RING);
-	if (sq->ring_ptr == MAP_FAILED)
-		return -errno;
+	sq->ring_ptr = liburing_mmap(0, sq->ring_sz, PROT_READ | PROT_WRITE,
+				     MAP_SHARED | MAP_POPULATE, fd,
+				     IORING_OFF_SQ_RING);
+	if (IS_ERR(sq->ring_ptr))
+		return PTR_ERR(sq->ring_ptr);
 
 	if (p->features & IORING_FEAT_SINGLE_MMAP) {
 		cq->ring_ptr = sq->ring_ptr;
 	} else {
-		cq->ring_ptr = mmap(0, cq->ring_sz, PROT_READ | PROT_WRITE,
-				MAP_SHARED | MAP_POPULATE, fd, IORING_OFF_CQ_RING);
-		if (cq->ring_ptr == MAP_FAILED) {
+		cq->ring_ptr = liburing_mmap(0, cq->ring_sz,
+					     PROT_READ | PROT_WRITE,
+					     MAP_SHARED | MAP_POPULATE, fd,
+					     IORING_OFF_CQ_RING);
+		if (IS_ERR(cq->ring_ptr)) {
+			ret = PTR_ERR(cq->ring_ptr);
 			cq->ring_ptr = NULL;
-			ret = -errno;
 			goto err;
 		}
 	}
@@ -63,11 +66,11 @@ 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 = mmap(0, size, PROT_READ | PROT_WRITE,
-				MAP_SHARED | MAP_POPULATE, fd,
-				IORING_OFF_SQES);
-	if (sq->sqes == MAP_FAILED) {
-		ret = -errno;
+	sq->sqes = liburing_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);
 err:
 		io_uring_unmap_rings(sq, cq);
 		return ret;
@@ -116,20 +119,20 @@ int io_uring_ring_dontfork(struct io_uring *ring)
 		return -EINVAL;
 
 	len = *ring->sq.kring_entries * sizeof(struct io_uring_sqe);
-	ret = madvise(ring->sq.sqes, len, MADV_DONTFORK);
-	if (ret == -1)
-		return -errno;
+	ret = liburing_madvise(ring->sq.sqes, len, MADV_DONTFORK);
+	if (uring_unlikely(ret))
+		return ret;
 
 	len = ring->sq.ring_sz;
-	ret = madvise(ring->sq.ring_ptr, len, MADV_DONTFORK);
-	if (ret == -1)
-		return -errno;
+	ret = liburing_madvise(ring->sq.ring_ptr, len, MADV_DONTFORK);
+	if (uring_unlikely(ret))
+		return ret;
 
 	if (ring->cq.ring_ptr != ring->sq.ring_ptr) {
 		len = ring->cq.ring_sz;
-		ret = madvise(ring->cq.ring_ptr, len, MADV_DONTFORK);
-		if (ret == -1)
-			return -errno;
+		ret = liburing_madvise(ring->cq.ring_ptr, len, MADV_DONTFORK);
+		if (uring_unlikely(ret))
+			return ret;
 	}
 
 	return 0;
@@ -173,7 +176,8 @@ void io_uring_queue_exit(struct io_uring *ring)
 	struct io_uring_sq *sq = &ring->sq;
 	struct io_uring_cq *cq = &ring->cq;
 
-	munmap(sq->sqes, *sq->kring_entries * sizeof(struct io_uring_sqe));
+	liburing_munmap(sq->sqes,
+			*sq->kring_entries * sizeof(struct io_uring_sqe));
 	io_uring_unmap_rings(sq, cq);
 	close(ring->ring_fd);
 }
diff --git a/src/syscall.h b/src/syscall.h
index 5f7343f..0de021f 100644
--- a/src/syscall.h
+++ b/src/syscall.h
@@ -5,7 +5,10 @@
 #include <errno.h>
 #include <signal.h>
 #include <unistd.h>
+#include <sys/mman.h>
 #include <sys/syscall.h>
+#include <sys/resource.h>
+#include "kernel_err.h"
 
 #ifdef __alpha__
 /*
@@ -99,4 +102,49 @@ static inline int ____sys_io_uring_enter(int fd, unsigned to_submit,
 				       _NSIG / 8);
 }
 
+static inline void *liburing_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);
+	if (ret == MAP_FAILED)
+		ret = ERR_PTR(-errno);
+
+	return ret;
+}
+
+static inline int liburing_munmap(void *addr, size_t length)
+{
+	int ret;
+
+	ret = munmap(addr, length);
+	return (ret < 0) ? -errno : ret;
+}
+
+static inline int liburing_madvise(void *addr, size_t length, int advice)
+{
+	int ret;
+
+	ret = madvise(addr, length, advice);
+	return (ret < 0) ? -errno : ret;
+}
+
+static inline int liburing_getrlimit(int resource, struct rlimit *rlim)
+{
+	int ret;
+
+	ret = getrlimit(resource, rlim);
+	return (ret < 0) ? -errno : ret;
+}
+
+static inline int liburing_setrlimit(int resource, const struct rlimit *rlim)
+{
+	int ret;
+
+	ret = setrlimit(resource, rlim);
+	return (ret < 0) ? -errno : ret;
+}
+
+
 #endif
-- 
2.30.2

[PATCH v3 RFC liburing 4/4] src/{queue,register,setup}: Remove `#include <errno.h>`

[Ammar Faizi]

We don't need `#include <errno.h>` in these files anymore. For now,
`errno` variable is only allowed to be used in `src/syscall.h` to
make it possible to remove the dependency of `errno` variable.

Cc: Bedirhan KURT <windowz414@gnuweeb.org>
Cc: Louvian Lyndal <louvianlyndal@gmail.com>
Signed-off-by: Ammar Faizi <ammar.faizi@students.amikom.ac.id>
---
 src/queue.c    | 1 -
 src/register.c | 1 -
 src/setup.c    | 1 -
 3 files changed, 3 deletions(-)

diff --git a/src/queue.c b/src/queue.c
index e85ea1d..24ff8bc 100644
--- a/src/queue.c
+++ b/src/queue.c
@@ -5,7 +5,6 @@
 #include <sys/stat.h>
 #include <sys/mman.h>
 #include <unistd.h>
-#include <errno.h>
 #include <string.h>
 #include <stdbool.h>
 
diff --git a/src/register.c b/src/register.c
index 770a672..43964a4 100644
--- a/src/register.c
+++ b/src/register.c
@@ -5,7 +5,6 @@
 #include <sys/stat.h>
 #include <sys/mman.h>
 #include <unistd.h>
-#include <errno.h>
 #include <string.h>
 
 #include "liburing/compat.h"
diff --git a/src/setup.c b/src/setup.c
index 7476e1e..f873443 100644
--- a/src/setup.c
+++ b/src/setup.c
@@ -4,7 +4,6 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <unistd.h>
-#include <errno.h>
 #include <string.h>
 #include <stdlib.h>
 #include <signal.h>
-- 
2.30.2

Re: [PATCH v3 RFC liburing 2/4] src/{queue,register,setup}: Don't use `__sys_io_uring*`

[Louvian Lyndal]

On 10/2/21 8:48 AM, Ammar Faizi wrote:
> @@ -158,7 +142,7 @@ int io_uring_register_files_tags(struct io_uring *ring,
>   		break;
>   	} while (1);
>   
> -	return ret < 0 ? -errno : ret;
> +	return (ret < 0) ? ret : 0;
>   }

This is wrong, you changed the logic, should've been "return ret;".
Not all successful call returns 0.

>   
>   int io_uring_register_files(struct io_uring *ring, const int *files,
> @@ -167,12 +151,12 @@ int io_uring_register_files(struct io_uring *ring, const int *files,
>   	int ret, did_increase = 0;
>   
>   	do {
> -		ret = __sys_io_uring_register(ring->ring_fd,
> -					      IORING_REGISTER_FILES, files,
> -					      nr_files);
> +		ret = ____sys_io_uring_register(ring->ring_fd,
> +						IORING_REGISTER_FILES, files,
> +						nr_files);
>   		if (ret >= 0)
>   			break;
> -		if (errno == EMFILE && !did_increase) {
> +		if (ret == -EMFILE && !did_increase) {
>   			did_increase = 1;
>   			increase_rlimit_nofile(nr_files);
>   			continue;
> @@ -180,55 +164,44 @@ int io_uring_register_files(struct io_uring *ring, const int *files,
>   		break;
>   	} while (1);
>   
> -	return ret < 0 ? -errno : ret;
> +	return (ret < 0) ? ret : 0;
>   }

The same thing here!

-- 
Louvian Lyndal

Re: [PATCH v3 RFC liburing 2/4] src/{queue,register,setup}: Don't use `__sys_io_uring*`

[Ammar Faizi]

On Sat, Oct 2, 2021 at 3:12 PM Louvian Lyndal <louvianlyndal@gmail.com> wrote:
>
> On 10/2/21 8:48 AM, Ammar Faizi wrote:
> > @@ -158,7 +142,7 @@ int io_uring_register_files_tags(struct io_uring *ring,
> >               break;
> >       } while (1);
> >  
> > -     return ret < 0 ? -errno : ret;
> > +     return (ret < 0) ? ret : 0;
> >   }
>
> This is wrong, you changed the logic, should've been "return ret;".
> Not all successful call returns 0.
>
> >  
> >   int io_uring_register_files(struct io_uring *ring, const int *files,
> > @@ -167,12 +151,12 @@ int io_uring_register_files(struct io_uring *ring, const int *files,
> >       int ret, did_increase = 0;
> >  
> >       do {
> > -             ret = __sys_io_uring_register(ring->ring_fd,
> > -                                           IORING_REGISTER_FILES, files,
> > -                                           nr_files);
> > +             ret = ____sys_io_uring_register(ring->ring_fd,
> > +                                             IORING_REGISTER_FILES, files,
> > +                                             nr_files);
> >               if (ret >= 0)
> >                       break;
> > -             if (errno == EMFILE && !did_increase) {
> > +             if (ret == -EMFILE && !did_increase) {
> >                       did_increase = 1;
> >                       increase_rlimit_nofile(nr_files);
> >                       continue;
> > @@ -180,55 +164,44 @@ int io_uring_register_files(struct io_uring *ring, const int *files,
> >               break;
> >       } while (1);
> >  
> > -     return ret < 0 ? -errno : ret;
> > +     return (ret < 0) ? ret : 0;
> >   }
>
> The same thing here!
>
> --
> Louvian Lyndal

Ack, that changed the logic. I missed that, will fix this for v4.

Not an excuse, but the test did not fail from my end.

Looking at test/test_update_multiring.c:71 on test_update_multiring():
```
  if (io_uring_register_files(r1, fds, 10) ||
      io_uring_register_files(r2, fds, 10) ||
      io_uring_register_files(r3, fds, 10)) {
        fprintf(stderr, "%s: register files failed\n", __FUNCTION__);
        goto err;
  }
```
Based on this one, this function is expected to return 0. When it
returns non zero value, it will go to err. So it won't really change
the situation.

In anyway, it's a fact that I changed the logic, I admit that and will
get it fixed.

Thanks!

-- 
Ammar Faizi