GNU/Weeb Mailing List <[email protected]>
 help / color / mirror / Atom feed
* [PATCH 1/3] queue, liburing.h: Avoid `io_uring_get_sqe()` code duplication
@ 2022-02-26 11:01 Ammar Faizi
  2022-02-26 11:01 ` [PATCH 2/3] .gitignore: Add `/test/fpos` to .gitignore Ammar Faizi
  2022-02-26 11:01 ` [PATCH 3/3] src/Makefile: Don't use stack protector for all builds by default Ammar Faizi
  0 siblings, 2 replies; 3+ messages in thread
From: Ammar Faizi @ 2022-02-26 11:01 UTC (permalink / raw)
  To: GNU/Weeb Mailing List

Since commit 8be8af4afcb4909104c ("queue: provide io_uring_get_sqe()
symbol again"), we have the same definition of `io_uring_get_sqe()` in
queue.c and liburing.h.

Make it simpler, maintain it in a single place, create a new static
inline function wrapper with name `_io_uring_get_sqe()`. Then tail
call both `io_uring_get_sqe()` functions to `_io_uring_get_sqe()`.

Cc: Nugra <[email protected]>
Cc: Alviro Iskandar Setiawan <[email protected]>
Cc: GNU/Weeb Mailing List <[email protected]>
Cc: Tea Inside Mailing List <[email protected]>
Cc: io-uring Mailing List <[email protected]>
Link: https://lore.kernel.org/io-uring/[email protected] # v1
Reviewed-by: Alviro Iskandar Setiawan <[email protected]>
Tested-by: Alviro Iskandar Setiawan <[email protected]>
Signed-off-by: Ammar Faizi <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Jens Axboe <[email protected]>
---
 src/include/liburing.h |  9 +++++++--
 src/queue.c            | 11 +----------
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/src/include/liburing.h b/src/include/liburing.h
index 590fe7f..ef5a4cd 100644
--- a/src/include/liburing.h
+++ b/src/include/liburing.h
@@ -831,8 +831,7 @@ static inline int io_uring_wait_cqe(struct io_uring *ring,
  *
  * Returns a vacant sqe, or NULL if we're full.
  */
-#ifndef LIBURING_INTERNAL
-static inline struct io_uring_sqe *io_uring_get_sqe(struct io_uring *ring)
+static inline struct io_uring_sqe *_io_uring_get_sqe(struct io_uring *ring)
 {
 	struct io_uring_sq *sq = &ring->sq;
 	unsigned int head = io_uring_smp_load_acquire(sq->khead);
@@ -845,6 +844,12 @@ static inline struct io_uring_sqe *io_uring_get_sqe(struct io_uring *ring)
 	}
 	return sqe;
 }
+
+#ifndef LIBURING_INTERNAL
+static inline struct io_uring_sqe *io_uring_get_sqe(struct io_uring *ring)
+{
+	return _io_uring_get_sqe(ring);
+}
 #else
 struct io_uring_sqe *io_uring_get_sqe(struct io_uring *ring);
 #endif
diff --git a/src/queue.c b/src/queue.c
index 6b63490..f9b6c86 100644
--- a/src/queue.c
+++ b/src/queue.c
@@ -405,16 +405,7 @@ int io_uring_submit_and_wait(struct io_uring *ring, unsigned wait_nr)
 
 struct io_uring_sqe *io_uring_get_sqe(struct io_uring *ring)
 {
-	struct io_uring_sq *sq = &ring->sq;
-	unsigned int head = io_uring_smp_load_acquire(sq->khead);
-	unsigned int next = sq->sqe_tail + 1;
-	struct io_uring_sqe *sqe = NULL;
-
-	if (next - head <= *sq->kring_entries) {
-		sqe = &sq->sqes[sq->sqe_tail & *sq->kring_mask];
-		sq->sqe_tail = next;
-	}
-	return sqe;
+	return _io_uring_get_sqe(ring);
 }
 
 int __io_uring_sqring_wait(struct io_uring *ring)
-- 
2.32.0


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

* [PATCH 2/3] .gitignore: Add `/test/fpos` to .gitignore
  2022-02-26 11:01 [PATCH 1/3] queue, liburing.h: Avoid `io_uring_get_sqe()` code duplication Ammar Faizi
@ 2022-02-26 11:01 ` Ammar Faizi
  2022-02-26 11:01 ` [PATCH 3/3] src/Makefile: Don't use stack protector for all builds by default Ammar Faizi
  1 sibling, 0 replies; 3+ messages in thread
From: Ammar Faizi @ 2022-02-26 11:01 UTC (permalink / raw)
  To: GNU/Weeb Mailing List

Dylan forgot to add it to .gitignore when creating this test.

Signed-off-by: Ammar Faizi <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Jens Axboe <[email protected]>
---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index 15559ab..c9dc77f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -60,6 +60,7 @@
 /test/files-exit-hang-poll
 /test/files-exit-hang-timeout
 /test/fixed-link
+/test/fpos
 /test/fsync
 /test/hardlink
 /test/io-cancel
-- 
2.32.0


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

* [PATCH 3/3] src/Makefile: Don't use stack protector for all builds by default
  2022-02-26 11:01 [PATCH 1/3] queue, liburing.h: Avoid `io_uring_get_sqe()` code duplication Ammar Faizi
  2022-02-26 11:01 ` [PATCH 2/3] .gitignore: Add `/test/fpos` to .gitignore Ammar Faizi
@ 2022-02-26 11:01 ` Ammar Faizi
  1 sibling, 0 replies; 3+ messages in thread
From: Ammar Faizi @ 2022-02-26 11:01 UTC (permalink / raw)
  To: GNU/Weeb Mailing List

Stack protector adds extra mov, extra stack allocation and extra branch
to save and validate the stack canary. While this feature could be
useful to detect stack corruption in some scenarios, it is not really
needed for liburing which is simple enough to review.

Good code shouldn't corrupt the stack. We don't need this extra
checking at the moment. Just for comparison, let's take a hot function
__io_uring_get_cqe.

Before this patch:
```
0000000000002b80 <__io_uring_get_cqe>:
  2b80:   f3 0f 1e fa             endbr64
  2b84:   48 83 ec 28             sub    $0x28,%rsp
  2b88:   64 48 8b 04 25 28 00    mov    %fs:0x28,%rax
  2b8f:   00 00
  2b91:   48 89 44 24 18          mov    %rax,0x18(%rsp)
  2b96:   31 c0                   xor    %eax,%eax
  2b98:   89 14 24                mov    %edx,(%rsp)
  2b9b:   48 89 e2                mov    %rsp,%rdx
  2b9e:   48 b8 00 00 00 00 08    movabs $0x800000000,%rax
  2ba5:   00 00 00
  2ba8:   89 4c 24 04             mov    %ecx,0x4(%rsp)
  2bac:   48 89 44 24 08          mov    %rax,0x8(%rsp)
  2bb1:   4c 89 44 24 10          mov    %r8,0x10(%rsp)
  2bb6:   e8 45 fe ff ff          call   2a00 <_io_uring_get_cqe>
  2bbb:   48 8b 54 24 18          mov    0x18(%rsp),%rdx
  2bc0:   64 48 2b 14 25 28 00    sub    %fs:0x28,%rdx
  2bc7:   00 00
  2bc9:   75 05                   jne    2bd0 <__io_uring_get_cqe+0x50>
  2bcb:   48 83 c4 28             add    $0x28,%rsp
  2bcf:   c3                      ret
  2bd0:   e8 9b f5 ff ff          call   2170 <__stack_chk_fail@plt>
  2bd5:   66 66 2e 0f 1f 84 00    data16 cs nopw 0x0(%rax,%rax,1)
  2bdc:   00 00 00 00
```

After this patch:
```
0000000000002ab0 <__io_uring_get_cqe>:
  2ab0:   f3 0f 1e fa             endbr64
  2ab4:   48 b8 00 00 00 00 08    movabs $0x800000000,%rax
  2abb:   00 00 00
  2abe:   48 83 ec 28             sub    $0x28,%rsp
  2ac2:   89 14 24                mov    %edx,(%rsp)
  2ac5:   48 89 e2                mov    %rsp,%rdx
  2ac8:   89 4c 24 04             mov    %ecx,0x4(%rsp)
  2acc:   48 89 44 24 08          mov    %rax,0x8(%rsp)
  2ad1:   4c 89 44 24 10          mov    %r8,0x10(%rsp)
  2ad6:   e8 55 fe ff ff          call   2930 <_io_uring_get_cqe>
  2adb:   48 83 c4 28             add    $0x28,%rsp
  2adf:   c3                      ret
```

Previously, we only use `-fno-stack-protector` for nolibc build as the
stack protector needs to call `__stack_chk_fail@plt` function from the
libc. Now, we always use `-fno-stack-protector` for both nolibc and
libc builds to generate shorter Assembly code.

Cc: Nugra <[email protected]>
Cc: Alviro Iskandar Setiawan <[email protected]>
Cc: GNU/Weeb Mailing List <[email protected]>
Cc: Tea Inside Mailing List <[email protected]>
Cc: io-uring Mailing List <[email protected]>
Signed-off-by: Ammar Faizi <[email protected]>
Reviewed-by: Alviro Iskandar Setiawan <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Jens Axboe <[email protected]>
---
 src/Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/Makefile b/src/Makefile
index cc6c871..3e1192f 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -8,7 +8,7 @@ libdevdir ?= $(prefix)/lib
 CPPFLAGS ?=
 override CPPFLAGS += -D_GNU_SOURCE \
 	-Iinclude/ -include ../config-host.h
-CFLAGS ?= -g -fomit-frame-pointer -O2 -Wall -Wextra
+CFLAGS ?= -g -fomit-frame-pointer -O2 -Wall -Wextra -fno-stack-protector
 override CFLAGS += -Wno-unused-parameter -Wno-sign-compare -DLIBURING_INTERNAL
 SO_CFLAGS=-fPIC $(CFLAGS)
 L_CFLAGS=$(CFLAGS)
@@ -36,8 +36,8 @@ liburing_srcs := setup.c queue.c register.c
 
 ifeq ($(CONFIG_NOLIBC),y)
 	liburing_srcs += nolibc.c
-	override CFLAGS += -nostdlib -nodefaultlibs -ffreestanding -fno-stack-protector
-	override CPPFLAGS += -nostdlib -nodefaultlibs -ffreestanding -fno-stack-protector
+	override CFLAGS += -nostdlib -nodefaultlibs -ffreestanding
+	override CPPFLAGS += -nostdlib -nodefaultlibs -ffreestanding
 	override LINK_FLAGS += -nostdlib -nodefaultlibs
 else
 	liburing_srcs += syscall.c
-- 
2.32.0


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

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

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-26 11:01 [PATCH 1/3] queue, liburing.h: Avoid `io_uring_get_sqe()` code duplication Ammar Faizi
2022-02-26 11:01 ` [PATCH 2/3] .gitignore: Add `/test/fpos` to .gitignore Ammar Faizi
2022-02-26 11:01 ` [PATCH 3/3] src/Makefile: Don't use stack protector for all builds by default Ammar Faizi

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