-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Early work-in-progress io_uring support #1356
base: master
Are you sure you want to change the base?
Early work-in-progress io_uring support #1356
Conversation
Great!
Actually this is not that important, and maybe it is better to do it separately, since right now there is no such auto tunning.
Yes, but it will also not harm, right? |
Is it possible to install |
Sure, just modify:
|
6a7036d
to
7644b74
Compare
I've relaxed |
8facc5c
to
626677e
Compare
NOTE: in the last version of the patch, |
b1e531f
to
0241dc3
Compare
@dmantipov do you have some workload that can benefit from this? |
@@ -73,7 +73,7 @@ jobs: | |||
- name: Install Depends | |||
run: | | |||
sudo apt-get update | |||
sudo apt-get install -y libmbedtls-dev | |||
sudo apt-get install -y libmbedtls-dev liburing-dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, there is no liburing-dev in ubuntu 18.04, let's switch to ubuntu-22.04
#ifdef EVENT__HAVE_LIBURING | ||
struct io_uring io_uring; | ||
unsigned io_uring_sqe_count; | ||
struct __kernel_timespec io_uring_cqe_timeout; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using plain timespec
instead of private __kernel_timespec
base->io_uring_cqe_timeout.tv_nsec = 0; | ||
} | ||
if (io_uring_queue_init(queue_size, &base->io_uring, 0) == 0) | ||
base->flags |= EVENT_BASE_FLAG_IO_URING; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An error if it fails?
event.c
Outdated
@@ -1769,6 +1841,14 @@ event_process_active_single_queue(struct event_base *base, | |||
if (base->event_continue) | |||
break; | |||
} | |||
#ifdef EVENT__HAVE_LIBURING | |||
/* FIXME: handle unfinished I/O requests properly. */ | |||
if (base->io_uring_sqe_count > 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this is bad? It can be handled in another iteration
@@ -1755,6 +1817,16 @@ event_process_active_single_queue(struct event_base *base, | |||
} | |||
#endif | |||
|
|||
#ifdef EVENT__HAVE_LIBURING | |||
if (base->io_uring_sqe_count) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, interface looks clumsy:
- checking
io_uring_sqe_count
after each event looks odd, it should be checked in theevent_base_loop
, or it should be binded to specific event and processed only when the callback for that particular event is called (one way is to add new callback for event in case of uring, however a) this will bloat the structure and b) it will break ABI compatibility - even though it is highly not recommended to useevent
structure directly) io_uring_sqe_count
is set from thebuffer.c
, and checked here, how about adding some wrappers for this?io_uring_buffers
modified directly from thebufferevent
code, how about adding some wrappers for this?
buffer.c
Outdated
else { | ||
/* Caller might want to check errno, which | ||
is never used by liburing itself. */ | ||
n = -1, errno = -buf->io_uring_result; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
n = -1, errno = -buf->io_uring_result; | |
n = -1; | |
errno = -buf->io_uring_result; |
buffer.c
Outdated
else { | ||
/* Caller might want to check errno, which | ||
is never used by liburing itself. */ | ||
n = -1, errno = -buffer->io_uring_result; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
n = -1, errno = -buffer->io_uring_result; | |
n = -1; | |
errno = -buffer->io_uring_result; |
buffer.c
Outdated
buf->io_uring_status = IO_URING_NONE; | ||
goto complete; | ||
} else if (buf->io_uring_status == IO_URING_RUNNING) { | ||
result = -1, errno = EINPROGRESS; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
result = -1, errno = EINPROGRESS; | |
result = -1; | |
errno = EINPROGRESS; |
buffer.c
Outdated
buffer->io_uring_status = IO_URING_NONE; | ||
goto complete; | ||
} else if (buffer->io_uring_status == IO_URING_RUNNING) { | ||
n = -1, errno = EINPROGRESS; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
n = -1, errno = EINPROGRESS; | |
n = -1; | |
errno = EINPROGRESS; |
I will mark it as draft for now. |
0241dc3
to
a575de7
Compare
CI failures:
But by some reason one build with clang passed - https://github.com/libevent/libevent/actions/runs/3308507890/jobs/5460943018 |
a575de7
to
d8ecb88
Compare
97b88ad
to
bfe3f23
Compare
bfe3f23
to
ee65796
Compare
ee65796
to
68d51a3
Compare
Signed-off-by: Dmitry Antipov <dantipov@cloudlinux.com>
68d51a3
to
b528661
Compare
This code aims to provide an initial and very basic support for
io_uring(7)
, a Linux-specific API for asynchronous I/O. Currently this work aims to provide anio_uring
-based I/O for socket-based bufferevents. The most important issue of the whole thing is that bufferevent interface assumes the synchronous callbacks, somewhat similar to the convenientread()
andwrite()
system calls, but the wholeio_uring
subsystem is designed to be (mostly) asynchronous. Current approach is designed in attempt to avoid changing an externallibevent
API and basically works as follows:evbuffer_io_uring_submit_rwv()
should get and SQE, prepare it to issue I/O request, submit to the ring, and callevent_base_active_by_fd()
to enforce the next iteration of (per-base) event processing loop.event_process_active_single_queue()
issuesio_uring_wait_cqe_timeout()
to fetch CQE and finally callsevent_process_cqe()
to complete an I/O.I would strongly suggests everyone interested in this feature to try this code, run
sample/bufferevent-benchmark
test, share the numbers and maybe even an ideas on further improvements of this code, whatever.Signed-off-by: Dmitry Antipov dantipov@cloudlinux.com
Fixes: #1019