1:59 p.m.
On 9/4/2023 8:23 PM, Eric Blake wrote:
On Mon, Sep 04, 2023 at 04:07:54PM +0000, Tage Johansson wrote:
> From: Eric Blake <eblake(a)redhat.com>
>
> CI shows our async handle fails to build on FreeBSD and MacOS (where
> epoll() is not available as a syscall, and therefore not available as
> a Rust crate). We can instead accomplish the same level-probing
> effects by doing a zero-timeout poll with mio (which defers under the
> hood to epoll on Linux, and kqueue on BSD).
>
> Fixes: 223a9965 ("rust: async: Create an async friendly handle type")
> CC: Tage Johansson <tage.j.lists(a)posteo.net>
> Signed-off-by: Eric Blake <eblake(a)redhat.com>
> ---
> rust/Cargo.toml | 3 ++-
> rust/src/async_handle.rs | 40 +++++++++++++++++++++++-----------------
> 2 files changed, 25 insertions(+), 18 deletions(-)
>
> - // Use epoll to check the current read/write availability on the fd.
> + // Use mio poll to check the current read/write availability on the fd.
> // This is needed because Tokio supports only edge-triggered
> // notifications but Libnbd requires level-triggered notifications.
> - let mut revent = epoll::Event { data: 0, events: 0 };
> // Setting timeout to 0 means that it will return immediately.
> - epoll::wait(epfd, 0, std::slice::from_mut(&mut revent))?;
> - let revents = Events::from_bits(revent.events).unwrap();
> - if !revents.contains(Events::EPOLLIN) {
> - ready_guard.clear_ready_matching(IoReady::READABLE);
> - }
> - if !revents.contains(Events::EPOLLOUT) {
> - ready_guard.clear_ready_matching(IoReady::WRITABLE);
> + // mio states that it is OS-dependent on whether a single event
> + // can be both readable and writable, but we survive just fine
> + // if we only see one direction even when both are available.
> + poll.registry().register(
> + &mut SourceFd(&fd),
> + Token(0),
> + MioInterest::READABLE | MioInterest::WRITABLE,
> + )?;
> + poll.poll(&mut events, Some(Duration::ZERO))?;
Why do we want 'poll.poll()?;', that is, to fail this function if the
poll returns an error? We _expect_ poll to sometimes return an error
(namely, the fact that it timed out) if there is nothing pending on
the fd, at which point we WANT to successfully clear the ready_guard
for both read and write, rather than to error out of this function.
You are right. I thought that the poll() call would return Ok(()) upon
timeout, but according to the documentation:
Currently if the timeout elapses without any readiness events
triggering this will return Ok(()). However we’re not guaranteeing
this behaviour as this depends on the OS.
So I guess it is best to ignore any errors from the poll call as in your
patch.
--
Best regards,
Tage
> + for event in &events {
> + if !event.is_readable() {
> + ready_guard.clear_ready_matching(IoReady::READABLE);
> + }
> + if !event.is_writable() {
> + ready_guard.clear_ready_matching(IoReady::WRITABLE);
> + }
> }
> ready_guard.retain_ready();
> + poll.registry().deregister(&mut SourceFd(&fd))?;
Furthermore, if the regsiter()/deregister() should always occur in
pairs, the early poll()? exit breaks that assumption (I don't know if
Rust has enough smarts to automatically clean up the unpaired
registration).