epoll: do proper edge detection inside the epoll system #4676
Conversation
|
Thank you for contributing to Miri! A reviewer will take a look at your PR, typically within a week or two. |
| ecx.epoll_send_fd_ready_events(eventfd)?; | ||
| // Linux seems to cause spurious wakeups here, and Tokio seems to rely on that | ||
| // (see <https://github.com/rust-lang/miri/issues/4374#issuecomment-3508873493>). | ||
| ecx.epoll_send_fd_ready_events(eventfd, /* force_edge */ true)?; |
There was a problem hiding this comment.
@Darksonn this is the suspicious part... if I make this false, the Tokio test deadlocks....
|
|
||
| // This does not change the status, so we should get no event. | ||
| // However, Linux performs a spurious wakeup. | ||
| check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)]); |
There was a problem hiding this comment.
... Setting force_edge to false makes it so that we get the empty event list here. Which is what I would expect since the readiness did not change. However, Linux does seem to always emit an epoll event on eventfd writes, even if readiness does not change -- and it looks like Tokio relies on that? At least that is the only explanation I have for the deadlocks I have been seeing.
There was a problem hiding this comment.
Yes, it looks like Tokio does rely on that. Mio has no logic to consume the notification except when the 64-bit counter overflows. I guess this avoids a read syscall, so it makes some sort of sense. See eventfd.rs for details.
There was a problem hiding this comment.
Okay, thanks for confirming!
There was a problem hiding this comment.
Fun, Mio also references https://www.illumos.org/issues/16700 -- looks like Illumos was also requested to be bug-for-bug compatible with Linux here.
I wouldn't mind this so much if it was documented, but epoll overall has pretty bad documentation I have to say. :/
There was a problem hiding this comment.
@Darksonn FWIW it seems like Linux triggers a wakeup even if you write [0u8; 8] to the eventfd. That way you'd never even have to worry about overflow.
RalfJung
force-pushed
the
epoll-edge
branch
2 times, most recently
from
b66ef3a to
4c670e2
Compare
3 participants
Previously, the individual FDs were expected to only call
epoll_send_fd_ready_eventswhen something changed. However, they couldn't actually know which changes are relevant to the epoll interests, so instead move the change detection into the epoll core. To ensure we behave consistently with Linux, some events need to force a notification even if there was no change.Also, fix a bug where when an event got un-available, we'd still keep it in the ready list. (Without the above, that would cause tons of existing tests to fail.) And fix EPOLL_CTL_MOD to make it update the
dataof events already in the ready list.Fixes #4374.