From 631c28c9e3814de45b1a55b0d475d41623981dae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20Anic=CC=81?= Date: Sun, 31 Mar 2024 23:51:40 +0200 Subject: [PATCH 1/3] io_uring: add failing test For issue: #19451 --- lib/std/os/linux/IoUring.zig | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/lib/std/os/linux/IoUring.zig b/lib/std/os/linux/IoUring.zig index 0e3c4ce33858..7c8eeae7b2d6 100644 --- a/lib/std/os/linux/IoUring.zig +++ b/lib/std/os/linux/IoUring.zig @@ -4230,3 +4230,25 @@ fn expect_buf_grp_cqe( return cqe; } + +test "failing test for issue 19451" { + var ring = try IoUring.init(2, 0); + defer ring.deinit(); + + try testing.expectEqual(2, ring.sq.sqes.len); + try testing.expectEqual(4, ring.cq.cqes.len); + + for (0..4) |i| { + const sqe = try ring.get_sqe(); + sqe.prep_timeout(&.{ .tv_sec = 0, .tv_nsec = 10000 }, 0, 0); + sqe.user_data = i; + _ = try ring.submit(); + } + + var cqe_count: u32 = 0; + while (cqe_count < 4) { + var cqes: [8]linux.io_uring_cqe = undefined; + cqe_count += try ring.copy_cqes(&cqes, 4 - cqe_count); + } + try testing.expectEqual(4, cqe_count); +} From 704660c81a3e53713645528e658881283af013dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20Anic=CC=81?= Date: Sun, 31 Mar 2024 23:52:46 +0200 Subject: [PATCH 2/3] io_uring: fix copy_cqes logic --- lib/std/os/linux/IoUring.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/std/os/linux/IoUring.zig b/lib/std/os/linux/IoUring.zig index 7c8eeae7b2d6..08855fa72f85 100644 --- a/lib/std/os/linux/IoUring.zig +++ b/lib/std/os/linux/IoUring.zig @@ -284,7 +284,7 @@ fn copy_cqes_ready(self: *IoUring, cqes: []linux.io_uring_cqe) u32 { const head = self.cq.head.* & self.cq.mask; const tail = (self.cq.head.* +% count) & self.cq.mask; - if (head <= tail) { + if (head < tail) { // head behind tail -> no wrapping @memcpy(cqes[0..count], self.cq.cqes[head..tail]); } else { From 254c05a9e117cd4926eb8c3bb1d83e9137b53553 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20Anic=CC=81?= Date: Sun, 31 Mar 2024 23:57:16 +0200 Subject: [PATCH 3/3] io_uring: simplify copy_cqe logic First copy as much as we can in this cycle. If there is more needed wrap and start from the buffer 0 position. --- lib/std/os/linux/IoUring.zig | 71 +++++++++++++++++++++++------------- 1 file changed, 46 insertions(+), 25 deletions(-) diff --git a/lib/std/os/linux/IoUring.zig b/lib/std/os/linux/IoUring.zig index 08855fa72f85..263321bb6537 100644 --- a/lib/std/os/linux/IoUring.zig +++ b/lib/std/os/linux/IoUring.zig @@ -282,19 +282,15 @@ fn copy_cqes_ready(self: *IoUring, cqes: []linux.io_uring_cqe) u32 { const ready = self.cq_ready(); const count = @min(cqes.len, ready); const head = self.cq.head.* & self.cq.mask; - const tail = (self.cq.head.* +% count) & self.cq.mask; - - if (head < tail) { - // head behind tail -> no wrapping - @memcpy(cqes[0..count], self.cq.cqes[head..tail]); - } else { - // head in front of tail -> buffer wraps - const two_copies_required: bool = self.cq.cqes.len - head < count; - const amount_to_copy_in_first = if (two_copies_required) self.cq.cqes.len - head else count; - @memcpy(cqes[0..amount_to_copy_in_first], self.cq.cqes[head .. head + amount_to_copy_in_first]); - if (two_copies_required) { - @memcpy(cqes[amount_to_copy_in_first..count], self.cq.cqes[0..tail]); - } + + // before wrapping + const n = @min(self.cq.cqes.len - head, count); + @memcpy(cqes[0..n], self.cq.cqes[head..][0..n]); + + if (count > n) { + // wrap self.cq.cqes + const w = count - n; + @memcpy(cqes[n..][0..w], self.cq.cqes[0..w]); } self.cq_advance(count); @@ -4231,24 +4227,49 @@ fn expect_buf_grp_cqe( return cqe; } -test "failing test for issue 19451" { - var ring = try IoUring.init(2, 0); +test "copy_cqes with wrapping sq.cqes buffer" { + if (!is_linux) return error.SkipZigTest; + + var ring = IoUring.init(2, 0) catch |err| switch (err) { + error.SystemOutdated => return error.SkipZigTest, + error.PermissionDenied => return error.SkipZigTest, + else => return err, + }; defer ring.deinit(); try testing.expectEqual(2, ring.sq.sqes.len); try testing.expectEqual(4, ring.cq.cqes.len); - for (0..4) |i| { - const sqe = try ring.get_sqe(); - sqe.prep_timeout(&.{ .tv_sec = 0, .tv_nsec = 10000 }, 0, 0); - sqe.user_data = i; - _ = try ring.submit(); + // submit 2 entries, receive 2 completions + var cqes: [8]linux.io_uring_cqe = undefined; + { + for (0..2) |_| { + const sqe = try ring.get_sqe(); + sqe.prep_timeout(&.{ .tv_sec = 0, .tv_nsec = 10000 }, 0, 0); + try testing.expect(try ring.submit() == 1); + } + var cqe_count: u32 = 0; + while (cqe_count < 2) { + cqe_count += try ring.copy_cqes(&cqes, 2 - cqe_count); + } } - var cqe_count: u32 = 0; - while (cqe_count < 4) { - var cqes: [8]linux.io_uring_cqe = undefined; - cqe_count += try ring.copy_cqes(&cqes, 4 - cqe_count); + try testing.expectEqual(2, ring.cq.head.*); + + // sq.sqes len is 4, starting at position 2 + // every 4 entries submit wraps completion buffer + // we are reading ring.cq.cqes at indexes 2,3,0,1 + for (1..1024) |i| { + for (0..4) |_| { + const sqe = try ring.get_sqe(); + sqe.prep_timeout(&.{ .tv_sec = 0, .tv_nsec = 10000 }, 0, 0); + try testing.expect(try ring.submit() == 1); + } + var cqe_count: u32 = 0; + while (cqe_count < 4) { + cqe_count += try ring.copy_cqes(&cqes, 4 - cqe_count); + } + try testing.expectEqual(4, cqe_count); + try testing.expectEqual(2 + 4 * i, ring.cq.head.*); } - try testing.expectEqual(4, cqe_count); }