fix io_uring cancel_wait #264
Conversation
io/iouring-wrapper.cpp
Outdated
| } | ||
| void run_wait_canceller() { | ||
| uint32_t poll_mask = evmap.translate_bitwisely(EVENT_READ); | ||
| async_io(&io_uring_prep_poll_multishot, -1, RingFlagsWaitCanceller, m_wait_canceller_fd, poll_mask); | ||
| } |
There was a problem hiding this comment.
wait_canceller will fall into sleep in async_io. Because the multishot poll needs to keep the ioCtx on stack for the whole time.
There was a problem hiding this comment.
actually we don't need to use async_io() to register the multishot poll, and we can set the user data as nullptr.
When we get a event with nullptr data, we know it's cancel wait.
beef9999
force-pushed
the
beef9999/fix-iouring-cancel-wait
branch
2 times, most recently
from
3d56c07
to
6305097
Compare
| while (m_cancel_poller_running) { | ||
| uint32_t poll_mask = evmap.translate_bitwisely(EVENT_READ); | ||
| int ret = async_io(&io_uring_prep_poll_add, -1, 0, m_cancel_fd, poll_mask); | ||
| if (ret < 0) { |
There was a problem hiding this comment.
Should not use one-shot poll. There is a risk of missing eventfd notification, right before starting the next round of polling.
|
Thanks for fixing this so quickly! I tested this locally and it eliminates the stalls in my workload. |
io/iouring-wrapper.cpp
Outdated
| @@ -130,16 +131,17 @@ class iouringEngine : public MasterEventEngine, public CascadingEventEngine { | |||
|
|
|||
| if (m_master) { | |||
| // Setup a cancel poller to watch on master engine | |||
| m_cancel_fd = eventfd(0, EFD_CLOEXEC); | |||
| if (m_cancel_fd < 0) { | |||
| m_wait_canceller_fd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK); | |||
There was a problem hiding this comment.
m_wait_canceller_fd and m_cascading_event_fd can be unified as m_eventfd?
io/iouring-wrapper.cpp
Outdated
| } | ||
| void run_wait_canceller() { | ||
| uint32_t poll_mask = evmap.translate_bitwisely(EVENT_READ); | ||
| async_io(&io_uring_prep_poll_multishot, -1, RingFlagsWaitCanceller, m_wait_canceller_fd, poll_mask); | ||
| } |
There was a problem hiding this comment.
actually we don't need to use async_io() to register the multishot poll, and we can set the user data as nullptr.
When we get a event with nullptr data, we know it's cancel wait.
beef9999
force-pushed
the
beef9999/fix-iouring-cancel-wait
branch
2 times, most recently
from
9e0ef9c
to
d889094
Compare
beef9999
force-pushed
the
beef9999/fix-iouring-cancel-wait
branch
from
d889094
to
b5c5351
Compare
Fix #261