Re: [PATCH liburing v1 1/7] liburing.h: Export `__io_uring_flush_sq()` function

[Ammar Faizi <ammarfaizi2@gnuweeb.org>]

On 11/24/22 5:14 PM, Dylan Yudaken wrote:
> I think changing the tests to use the public API is probably better
> than exporting this function. I don't believe it has much general use?

But there is no public API that does the same thing. I'll mark it
as static and create a copy of that function in iopoll.c (in v2).

Something like this, what do you think?

  src/queue.c   |  2 +-
  test/iopoll.c | 33 ++++++++++++++++++++++++++++++++-
  2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/src/queue.c b/src/queue.c
index feea0ad..b784b10 100644
--- a/src/queue.c
+++ b/src/queue.c
@@ -201,7 +201,7 @@ again:
   * Sync internal state with kernel ring state on the SQ side. Returns the
   * number of pending items in the SQ ring, for the shared ring.
   */
-unsigned __io_uring_flush_sq(struct io_uring *ring)
+static unsigned __io_uring_flush_sq(struct io_uring *ring)
  {
  	struct io_uring_sq *sq = &ring->sq;
  	unsigned tail = sq->sqe_tail;
diff --git a/test/iopoll.c b/test/iopoll.c
index 20f91c7..5edd5c3 100644
--- a/test/iopoll.c
+++ b/test/iopoll.c
@@ -201,7 +201,38 @@ err:
  	return 1;
  }
  
-extern unsigned __io_uring_flush_sq(struct io_uring *ring);
+/*
+ * Sync internal state with kernel ring state on the SQ side. Returns the
+ * number of pending items in the SQ ring, for the shared ring.
+ */
+static unsigned __io_uring_flush_sq(struct io_uring *ring)
+{
+	struct io_uring_sq *sq = &ring->sq;
+	unsigned tail = sq->sqe_tail;
+
+	if (sq->sqe_head != tail) {
+		sq->sqe_head = tail;
+		/*
+		 * Ensure kernel sees the SQE updates before the tail update.
+		 */
+		if (!(ring->flags & IORING_SETUP_SQPOLL))
+			IO_URING_WRITE_ONCE(*sq->ktail, tail);
+		else
+			io_uring_smp_store_release(sq->ktail, tail);
+	}
+	/*
+	 * This _may_ look problematic, as we're not supposed to be reading
+	 * SQ->head without acquire semantics. When we're in SQPOLL mode, the
+	 * kernel submitter could be updating this right now. For non-SQPOLL,
+	 * task itself does it, and there's no potential race. But even for
+	 * SQPOLL, the load is going to be potentially out-of-date the very
+	 * instant it's done, regardless or whether or not it's done
+	 * atomically. Worst case, we're going to be over-estimating what
+	 * we can submit. The point is, we need to be able to deal with this
+	 * situation regardless of any perceived atomicity.
+	 */
+	return tail - *sq->khead;
+}
  
  /*
   * if we are polling io_uring_submit needs to always enter the


-- 
Ammar Faizi