* [PATCH 1/3] Fix incorrect close in test for multishot accept
2022-06-17 14:36 [PATCH liburing 0/3] multishot accept test fix and clean Hao Xu
@ 2022-06-17 14:36 ` Hao Xu
2022-06-17 14:36 ` [PATCH 2/3] test/accept: fix minus one error when calculating multishot_mask Hao Xu
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Hao Xu @ 2022-06-17 14:36 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, Pavel Begunkov
From: Donald Hunter <[email protected]>
This fixes a bug in accept_conn handling in the accept tests that caused it
to incorrectly skip the multishot tests and also lose the warning message
to a closed stdout. This can be seen in the strace output below.
close(1) = 0
io_uring_setup(32, { ...
...
write(1, "Fixed Multishot Accept not suppo"..., 47) = -1 EINVAL
Unfortunately this exposes a a bug with gcc -O2 where multishot_mask logic
gets optimized incorrectly and "Fixed Multishot Accept misses events" is
wrongly reported. I am investigating this separately.
Fixes: 66cf84527c34 ("test/accept.c: add test for multishot mode accept")
Signed-off-by: Donald Hunter <[email protected]>
---
test/accept.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/test/accept.c b/test/accept.c
index 4e2f58767b3c..c6654baa3925 100644
--- a/test/accept.c
+++ b/test/accept.c
@@ -103,7 +103,7 @@ static void queue_accept_conn(struct io_uring *ring, int fd,
}
}
-static int accept_conn(struct io_uring *ring, int fixed_idx)
+static int accept_conn(struct io_uring *ring, int fixed_idx, bool multishot)
{
struct io_uring_cqe *cqe;
int ret;
@@ -115,8 +115,10 @@ static int accept_conn(struct io_uring *ring, int fixed_idx)
if (fixed_idx >= 0) {
if (ret > 0) {
- close(ret);
- return -EINVAL;
+ if (!multishot) {
+ close(ret);
+ return -EINVAL;
+ }
} else if (!ret) {
ret = fixed_idx;
}
@@ -208,7 +210,7 @@ static int test_loop(struct io_uring *ring,
queue_accept_conn(ring, recv_s0, args);
for (i = 0; i < MAX_FDS; i++) {
- s_fd[i] = accept_conn(ring, args.fixed ? 0 : -1);
+ s_fd[i] = accept_conn(ring, args.fixed ? 0 : -1, multishot);
if (s_fd[i] == -EINVAL) {
if (args.accept_should_error)
goto out;
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] test/accept: clean code of accept test
2022-06-17 14:36 [PATCH liburing 0/3] multishot accept test fix and clean Hao Xu
2022-06-17 14:36 ` [PATCH 1/3] Fix incorrect close in test for multishot accept Hao Xu
2022-06-17 14:36 ` [PATCH 2/3] test/accept: fix minus one error when calculating multishot_mask Hao Xu
@ 2022-06-17 14:36 ` Hao Xu
2022-06-17 14:41 ` [PATCH liburing 0/3] multishot accept test fix and clean Hao Xu
2022-06-17 14:42 ` Jens Axboe
4 siblings, 0 replies; 6+ messages in thread
From: Hao Xu @ 2022-06-17 14:36 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, Pavel Begunkov
From: Hao Xu <[email protected]>
This does three things:
- change multishot_mask from uint to uint32_t
- change multishot_mask != UINT_MAX check to ~multishot_mask != 0
The above two avoid compiler and arch influence. Make the logic more robost.
- other cleaning to make code clearer.
Signed-off-by: Hao Xu <[email protected]>
---
test/accept.c | 66 +++++++++++++++++++++++----------------------------
1 file changed, 30 insertions(+), 36 deletions(-)
diff --git a/test/accept.c b/test/accept.c
index b322c018c7a9..78eba352a943 100644
--- a/test/accept.c
+++ b/test/accept.c
@@ -41,6 +41,21 @@ struct accept_test_args {
int extra_loops;
};
+static void close_fds(int fds[], int nr)
+{
+ int i;
+
+ for (i = 0; i < nr; i++)
+ close(fds[i]);
+}
+
+static void close_sock_fds(int s_fd[], int c_fd[], int nr, bool fixed)
+{
+ if (!fixed)
+ close_fds(s_fd, nr);
+ close_fds(c_fd, nr);
+}
+
static void queue_send(struct io_uring *ring, int fd)
{
struct io_uring_sqe *sqe;
@@ -198,19 +213,17 @@ static int test_loop(struct io_uring *ring,
int i, ret, s_fd[MAX_FDS], c_fd[MAX_FDS], done = 0;
bool fixed = args.fixed;
bool multishot = args.multishot;
- unsigned int multishot_mask = 0;
+ uint32_t multishot_mask = 0;
+ int nr_fds = multishot ? MAX_FDS : 1;
- for (i = 0; i < MAX_FDS; i++) {
+ for (i = 0; i < nr_fds; i++)
c_fd[i] = set_client_fd(addr);
- if (!multishot)
- break;
- }
if (!args.queue_accept_before_connect)
queue_accept_conn(ring, recv_s0, args);
- for (i = 0; i < MAX_FDS; i++) {
- s_fd[i] = accept_conn(ring, args.fixed ? 0 : -1, multishot);
+ for (i = 0; i < nr_fds; i++) {
+ s_fd[i] = accept_conn(ring, fixed ? 0 : -1, multishot);
if (s_fd[i] == -EINVAL) {
if (args.accept_should_error)
goto out;
@@ -241,14 +254,17 @@ static int test_loop(struct io_uring *ring,
i, s_fd[i]);
goto err;
}
+ /*
+ * for fixed multishot accept test, the file slots
+ * allocated are [0, 32), this means we finally end up
+ * with each bit of a u32 being 1.
+ */
multishot_mask |= (1U << s_fd[i]);
}
- if (!multishot)
- break;
}
if (multishot) {
- if (fixed && multishot_mask != UINT_MAX) {
+ if (fixed && (~multishot_mask != 0U)) {
fprintf(stderr, "Fixed Multishot Accept misses events\n");
goto err;
}
@@ -256,7 +272,7 @@ static int test_loop(struct io_uring *ring,
}
queue_send(ring, c_fd[0]);
- queue_recv(ring, s_fd[0], args.fixed);
+ queue_recv(ring, s_fd[0], fixed);
ret = io_uring_submit_and_wait(ring, 2);
assert(ret != -1);
@@ -280,32 +296,10 @@ static int test_loop(struct io_uring *ring,
}
out:
- if (!args.fixed) {
- for (i = 0; i < MAX_FDS; i++) {
- close(s_fd[i]);
- if (!multishot)
- break;
- }
- }
- for (i = 0; i < MAX_FDS; i++) {
- close(c_fd[i]);
- if (!multishot)
- break;
- }
+ close_sock_fds(s_fd, c_fd, nr_fds, fixed);
return 0;
err:
- if (!args.fixed) {
- for (i = 0; i < MAX_FDS; i++) {
- close(s_fd[i]);
- if (!multishot)
- break;
- }
- }
- for (i = 0; i < MAX_FDS; i++) {
- close(c_fd[i]);
- if (!multishot)
- break;
- }
+ close_sock_fds(s_fd, c_fd, nr_fds, fixed);
return 1;
}
@@ -627,7 +621,7 @@ static int test_accept_fixed(void)
static int test_multishot_fixed_accept(void)
{
struct io_uring m_io_uring;
- int ret, fd[100];
+ int ret, fd[MAX_FDS];
struct accept_test_args args = {
.fixed = true,
.multishot = true
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread