| Message ID | 20221124075846.3784701-2-ammar.faizi@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Ensure we mark internal functions and variables as static | expand |
On Thu, 2022-11-24 at 15:00 +0700, Ammar Faizi wrote: > From: Ammar Faizi <ammarfaizi2@gnuweeb.org> > > 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) > > This function is used by test/iopoll.c, therefore, it can't be > static. > Export it. > > Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org> > --- > src/include/liburing.h | 1 + > src/liburing.map | 5 +++++ > 2 files changed, 6 insertions(+) > 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? Dylan
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
On 11/24/22 4:47 AM, Ammar Faizi wrote: > 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? Yeah I think this is better for now, and then we can work on fixing the test. Sanity of the core parts of the library is a lot more important than some test case.
diff --git a/src/include/liburing.h b/src/include/liburing.h index 12a703f..c1d8edb 100644 --- a/src/include/liburing.h +++ b/src/include/liburing.h @@ -237,6 +237,7 @@ int io_uring_register_file_alloc_range(struct io_uring *ring, int io_uring_get_events(struct io_uring *ring); int io_uring_submit_and_get_events(struct io_uring *ring); +unsigned __io_uring_flush_sq(struct io_uring *ring); /* * io_uring syscalls. diff --git a/src/liburing.map b/src/liburing.map index 06c64f8..6b2f4b2 100644 --- a/src/liburing.map +++ b/src/liburing.map @@ -67,3 +67,8 @@ LIBURING_2.3 { io_uring_get_events; io_uring_submit_and_get_events; } LIBURING_2.2; + +LIBURING_2.4 { + global: + __io_uring_flush_sq; +} LIBURING_2.3;