public inbox for [email protected]
 help / color / mirror / Atom feed
From: Ammar Faizi <[email protected]>
To: Jens Axboe <[email protected]>
Cc: Ammar Faizi <[email protected]>,
	Dylan Yudaken <[email protected]>,
	Pavel Begunkov <[email protected]>,
	Gilang Fachrezy <[email protected]>,
	Muhammad Rizki <[email protected]>,
	Alviro Iskandar Setiawan <[email protected]>,
	VNLX Kernel Department <[email protected]>,
	io-uring Mailing List <[email protected]>,
	GNU/Weeb Mailing List <[email protected]>
Subject: [PATCH liburing v2 2/8] queue: Mark `__io_uring_flush_sq()` as static
Date: Thu, 24 Nov 2022 23:28:55 +0700	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>

From: Ammar Faizi <[email protected]>

This function is not exported, mark it as static. Clang says:

  queue.c:204:10: error: no previous prototype for function \
  '__io_uring_flush_sq' [-Werror,-Wmissing-prototypes] \
  unsigned __io_uring_flush_sq(struct io_uring *ring)
           ^
  queue.c:204:1: note: declare 'static' if the function is not intended \
  to be used outside of this translation unit \
  unsigned __io_uring_flush_sq(struct io_uring *ring)

Side note:
There was an attempt to export this function because it is used by
test/iopoll.c and test/io_uring_passthrough.c. But after a discussion
with Dylan and Jens, it's better to make a copy of this function for
those tests only instead of exporting it. Therefore, also create a
copy of this function in test/helpers.c.

Cc: Dylan Yudaken <[email protected]>
Cc: Jens Axboe <[email protected]>
Link: https://lore.kernel.org/io-uring/[email protected]
Signed-off-by: Ammar Faizi <[email protected]>
---
 src/queue.c                 |  2 +-
 test/helpers.c              | 33 +++++++++++++++++++++++++++++++++
 test/helpers.h              |  2 ++
 test/io_uring_passthrough.c |  2 --
 test/iopoll.c               |  2 --
 5 files changed, 36 insertions(+), 5 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/helpers.c b/test/helpers.c
index 8fb32b8..869e903 100644
--- a/test/helpers.c
+++ b/test/helpers.c
@@ -266,3 +266,36 @@ bool t_probe_defer_taskrun(void)
 	io_uring_queue_exit(&ring);
 	return true;
 }
+
+/*
+ * 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)
+{
+	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;
+}
diff --git a/test/helpers.h b/test/helpers.h
index 4375a9e..cb814ca 100644
--- a/test/helpers.h
+++ b/test/helpers.h
@@ -85,6 +85,8 @@ enum t_setup_ret t_register_buffers(struct io_uring *ring,
 
 bool t_probe_defer_taskrun(void);
 
+unsigned __io_uring_flush_sq(struct io_uring *ring);
+
 #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
 
 #ifdef __cplusplus
diff --git a/test/io_uring_passthrough.c b/test/io_uring_passthrough.c
index ee9ab87..d8468c5 100644
--- a/test/io_uring_passthrough.c
+++ b/test/io_uring_passthrough.c
@@ -268,8 +268,6 @@ static int test_io(const char *file, int tc, int read, int sqthread,
 	return ret;
 }
 
-extern unsigned __io_uring_flush_sq(struct io_uring *ring);
-
 /*
  * Send a passthrough command that nvme will fail during submission.
  * This comes handy for testing error handling.
diff --git a/test/iopoll.c b/test/iopoll.c
index 20f91c7..f8ab1f1 100644
--- a/test/iopoll.c
+++ b/test/iopoll.c
@@ -201,8 +201,6 @@ err:
 	return 1;
 }
 
-extern unsigned __io_uring_flush_sq(struct io_uring *ring);
-
 /*
  * if we are polling io_uring_submit needs to always enter the
  * kernel to fetch events
-- 
Ammar Faizi


  parent reply	other threads:[~2022-11-24 16:29 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-24 16:28 [PATCH liburing v2 0/8] Ensure we mark non-exported functions and variables as static Ammar Faizi
2022-11-24 16:28 ` [PATCH liburing v2 1/8] queue: Fix typo "entererd" -> "entered" Ammar Faizi
2022-11-24 16:28 ` Ammar Faizi [this message]
2022-11-24 16:28 ` [PATCH liburing v2 3/8] test/io_uring_setup: Remove unused functions Ammar Faizi
2022-11-24 16:28 ` [PATCH liburing v2 4/8] ucontext-cp: Remove an unused function Ammar Faizi
2022-11-24 16:28 ` [PATCH liburing v2 5/8] tests: Mark non-exported functions as static Ammar Faizi
2022-11-24 16:28 ` [PATCH liburing v2 6/8] ucontext-cp: Mark a non-exported function " Ammar Faizi
2022-11-24 16:29 ` [PATCH liburing v2 7/8] test/Makefile: Omit `-Wmissing-prototypes` from the C++ compiler flags Ammar Faizi
2022-11-24 16:29 ` [PATCH liburing v2 8/8] github: Add `-Wmissing-prototypes` for GitHub CI bot Ammar Faizi
2022-11-28 14:15 ` [PATCH liburing v2 0/8] Ensure we mark non-exported functions and variables as static Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox