io_uring_sqring_wait doesn't wait for free SQ entry

[erenon]

The man page says:

IORING_ENTER_SQ_WAIT
If the ring has been created with IORING_SETUP_SQPOLL,
[...] If the system call is used with this flag
set, then it will wait until at least one entry is free in
the SQ ring.

In my experiments, IORING_SETUP_SQPOLL=1, and after the SQ gets full, io_uring_sqring_wait returns, even before there's free space in the SQ. Also in the liburing example, examples/proxy.c, we have:

static struct io_uring_sqe *get_sqe(struct io_uring *ring)
{
	struct io_uring_sqe *sqe;

	do {
		sqe = io_uring_get_sqe(ring);
		if (sqe)
			break;
		if (!sqpoll)
			io_uring_submit(ring);
		else
			io_uring_sqring_wait(ring);
	} while (1);

	return sqe;
}

Here io_uring_sqring_wait is used in a loop. Why? Shouldn't it wait for at least one free SQ entry, as documented?

[axboe]

Yep looks like there's a kernel bug there. I think the below should fix it:

diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h
index bd1d4b6e46f0..4de799d12764 100644
--- a/io_uring/io_uring.h
+++ b/io_uring/io_uring.h
@@ -284,7 +284,7 @@ static inline bool io_sqring_full(struct io_ring_ctx *ctx)
 {
 	struct io_rings *r = ctx->rings;
 
-	return READ_ONCE(r->sq.tail) - ctx->cached_sq_head == ctx->sq_entries;
+	return READ_ONCE(r->sq.tail) - READ_ONCE(r->sq.head) == ctx->sq_entries;
 }
 
 static inline unsigned int io_sqring_entries(struct io_ring_ctx *ctx)

[axboe]

If we don't read the actual head, then we could return that there are entries left, even though the SQPOLL thread has not yet committed the SQRING consumption just yet. That race leaves a gap where the kernel returns "yep there are entries you can use" but the sqpoll thread hasn't committed the ones it's used just yet.

[axboe]

Committed a fix here:

https://git.kernel.dk/cgit/linux/commit/?h=io_uring-6.12&id=91560bef6395aba347be1dd93c623435ce6760f4

If you'd like, I can add a Reported-by: tag to attribute the report to you. If you want that, then please give me a name + email that I can use for that. If you don't want it or don't care, that's fine too.

[erenon]

Reported-by: Benedek Thaler thaler@thaler.hu

Thanks for the super quick fix!

[axboe]

Great, added! And no problem on the fix, honestly I'm sure I ran into it (hence the loop) and just forgot to triage that issue. Hence thanks for the report, now we can actually get it fixed...

[axboe]

Added a test case as well to liburing. I'll close this one up, fix should go upstream this week and make its way back to the stable kernels as well.