Eric Blake
2023-Aug-30 20:11 UTC
[Libguestfs] [libnbd PATCH 2/2] rust: Use mio::poll instead of requiring epoll
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 at posteo.net> Signed-off-by: Eric Blake <eblake at redhat.com> --- rust/Cargo.toml | 2 +- rust/src/async_handle.rs | 46 +++++++++++++++++++++++++--------------- 2 files changed, 30 insertions(+), 18 deletions(-) diff --git a/rust/Cargo.toml b/rust/Cargo.toml index 0879b34c..391c80e9 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -49,7 +49,7 @@ thiserror = "1.0.40" log = { version = "0.4.19", optional = true } libc = "0.2.147" tokio = { optional = true, version = "1.29.1", default-features = false, features = ["rt", "sync", "net"] } -epoll = "4.3.3" +mio = "0.8.0" [features] default = ["log", "tokio"] diff --git a/rust/src/async_handle.rs b/rust/src/async_handle.rs index ab28b910..35b1c0d2 100644 --- a/rust/src/async_handle.rs +++ b/rust/src/async_handle.rs @@ -35,9 +35,11 @@ use crate::sys; use crate::Handle; use crate::{Error, FatalErrorKind, Result}; use crate::{AIO_DIRECTION_BOTH, AIO_DIRECTION_READ, AIO_DIRECTION_WRITE}; -use epoll::Events; +use mio::unix::SourceFd; +use mio::{Events, Interest as MioInterest, Poll, Token}; use std::sync::Arc; use std::sync::Mutex; +use std::time::Duration; use tokio::io::{unix::AsyncFd, Interest, Ready as IoReady}; use tokio::sync::Notify; use tokio::task; @@ -176,12 +178,12 @@ async fn polling_task(handle_data: &HandleData) -> Result<(), FatalErrorKind> { } = handle_data; let fd = handle.aio_get_fd().map_err(Error::to_fatal)?; let tokio_fd = AsyncFd::new(fd)?; - let epfd = epoll::create(false)?; - epoll::ctl( - epfd, - epoll::ControlOptions::EPOLL_CTL_ADD, - fd, - epoll::Event::new(Events::EPOLLIN | Events::EPOLLOUT, 42), + let mut events = Events::with_capacity(1); + let mut poll = Poll::new()?; + poll.registry().register( + &mut SourceFd(&fd), + Token(0), + MioInterest::READABLE | MioInterest::WRITABLE, )?; // The following loop does approximately the following things: @@ -248,19 +250,29 @@ async fn polling_task(handle_data: &HandleData) -> Result<(), FatalErrorKind> { } drop(pending_cmds_lock); - // 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. + match poll.poll(&mut events, Some(Duration::ZERO)) { + Ok(_) => { + 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); + } + } + } + Err(_) => { + ready_guard.clear_ready_matching(IoReady::READABLE); + ready_guard.clear_ready_matching(IoReady::WRITABLE); + } + }; ready_guard.retain_ready(); } } -- 2.41.0
Tage Johansson
2023-Sep-01 08:35 UTC
[Libguestfs] [libnbd PATCH 2/2] rust: Use mio::poll instead of requiring epoll
On 8/30/2023 10:11 PM, Eric Blake wrote:> 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).The problem with mio is that it uses edge triggered notifications. According to this page <https://docs.rs/mio/latest/mio/struct.Poll.html>:> Draining readiness > Once a readiness event is received, the corresponding operation must > be performed repeatedly until it returns variant > std::io::error::ErrorKind::WouldBlock. Unless this is done, there is > no guarantee that another readiness event will be delivered, even if > further data is received for the event source.The motivation behind using epoll in addition to tokio in the first place was to check if fd is readable/writable without necessarily knowing if the last read/write operation returned [ErrorKind::WouldBlock]. And it doesn't seems that mio full fills that requirenment. I tried using the polling <https://docs.rs/polling/latest/polling/> crate, which uses epoll and kqueue under the hood. but for some reason the disconnect call failes, at least on my Linux machine. I have know idea what's the problem. Another idea would be that we create our own abstraction upon epoll and kqueue. But I am unable to test any kqueue implementation, I think that I can not even compile it on my machine. It would of course also be possible to disable the Rust bindings (or at least the asynchronous API) on BSD and MacOS, but that would not be a good solution I think. -- Best regards, Tage> Fixes: 223a9965 ("rust: async: Create an async friendly handle type") > CC: Tage Johansson<tage.j.lists at posteo.net> > Signed-off-by: Eric Blake<eblake at redhat.com> > --- > rust/Cargo.toml | 2 +- > rust/src/async_handle.rs | 46 +++++++++++++++++++++++++--------------- > 2 files changed, 30 insertions(+), 18 deletions(-) > > diff --git a/rust/Cargo.toml b/rust/Cargo.toml > index 0879b34c..391c80e9 100644 > --- a/rust/Cargo.toml > +++ b/rust/Cargo.toml > @@ -49,7 +49,7 @@ thiserror = "1.0.40" > log = { version = "0.4.19", optional = true } > libc = "0.2.147" > tokio = { optional = true, version = "1.29.1", default-features = false, features = ["rt", "sync", "net"] } > -epoll = "4.3.3" > +mio = "0.8.0" > > [features] > default = ["log", "tokio"] > diff --git a/rust/src/async_handle.rs b/rust/src/async_handle.rs > index ab28b910..35b1c0d2 100644 > --- a/rust/src/async_handle.rs > +++ b/rust/src/async_handle.rs > @@ -35,9 +35,11 @@ use crate::sys; > use crate::Handle; > use crate::{Error, FatalErrorKind, Result}; > use crate::{AIO_DIRECTION_BOTH, AIO_DIRECTION_READ, AIO_DIRECTION_WRITE}; > -use epoll::Events; > +use mio::unix::SourceFd; > +use mio::{Events, Interest as MioInterest, Poll, Token}; > use std::sync::Arc; > use std::sync::Mutex; > +use std::time::Duration; > use tokio::io::{unix::AsyncFd, Interest, Ready as IoReady}; > use tokio::sync::Notify; > use tokio::task; > @@ -176,12 +178,12 @@ async fn polling_task(handle_data: &HandleData) -> Result<(), FatalErrorKind> { > } = handle_data; > let fd = handle.aio_get_fd().map_err(Error::to_fatal)?; > let tokio_fd = AsyncFd::new(fd)?; > - let epfd = epoll::create(false)?; > - epoll::ctl( > - epfd, > - epoll::ControlOptions::EPOLL_CTL_ADD, > - fd, > - epoll::Event::new(Events::EPOLLIN | Events::EPOLLOUT, 42), > + let mut events = Events::with_capacity(1); > + let mut poll = Poll::new()?; > + poll.registry().register( > + &mut SourceFd(&fd), > + Token(0), > + MioInterest::READABLE | MioInterest::WRITABLE, > )?; > > // The following loop does approximately the following things: > @@ -248,19 +250,29 @@ async fn polling_task(handle_data: &HandleData) -> Result<(), FatalErrorKind> { > } > drop(pending_cmds_lock); > > - // 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. > + match poll.poll(&mut events, Some(Duration::ZERO)) { > + Ok(_) => { > + 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); > + } > + } > + } > + Err(_) => { > + ready_guard.clear_ready_matching(IoReady::READABLE); > + ready_guard.clear_ready_matching(IoReady::WRITABLE); > + } > + }; > ready_guard.retain_ready(); > } > }-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://listman.redhat.com/archives/libguestfs/attachments/20230901/b1d3c643/attachment.htm>