diff --git a/cros_async/src/io_ext.rs b/cros_async/src/io_ext.rs index 7873eb7b0e..96f9b290c1 100644 --- a/cros_async/src/io_ext.rs +++ b/cros_async/src/io_ext.rs @@ -275,7 +275,7 @@ mod tests { use crate::sys::unix::executor::async_poll_from_local; use crate::sys::unix::executor::async_uring_from; use crate::sys::unix::executor::async_uring_from_local; - use crate::sys::unix::uring_executor::is_uring_stable; + use crate::sys::unix::uring_executor::use_uring; use crate::sys::unix::FdExecutor; use crate::sys::unix::PollSource; use crate::sys::unix::URingExecutor; @@ -319,7 +319,7 @@ mod tests { #[test] fn await_uring_from_poll() { - if !is_uring_stable() { + if !use_uring() { return; } // Start a uring operation and then await the result from an FdExecutor. @@ -353,7 +353,7 @@ mod tests { #[test] fn await_poll_from_uring() { - if !is_uring_stable() { + if !use_uring() { return; } // Start a poll operation and then await the result from a URingExecutor. @@ -387,7 +387,7 @@ mod tests { #[test] fn readvec() { - if !is_uring_stable() { + if !use_uring() { return; } async fn go(async_source: Box>) { @@ -423,7 +423,7 @@ mod tests { #[test] fn writevec() { - if !is_uring_stable() { + if !use_uring() { return; } async fn go(async_source: Box>) { @@ -458,7 +458,7 @@ mod tests { #[test] fn readmem() { - if !is_uring_stable() { + if !use_uring() { return; } async fn go(async_source: Box>) { @@ -511,7 +511,7 @@ mod tests { #[test] fn writemem() { - if !is_uring_stable() { + if !use_uring() { return; } async fn go(async_source: Box>) { @@ -550,7 +550,7 @@ mod tests { #[test] fn read_u64s() { - if !is_uring_stable() { + if !use_uring() { return; } async fn go(async_source: File, ex: URingExecutor) -> u64 { @@ -566,7 +566,7 @@ mod tests { #[test] fn read_eventfds() { - if !is_uring_stable() { + if !use_uring() { return; } @@ -605,7 +605,7 @@ mod tests { #[test] fn fsync() { - if !is_uring_stable() { + if !use_uring() { return; } async fn go(source: Box>) { diff --git a/cros_async/src/lib.rs b/cros_async/src/lib.rs index 569002efac..9105d47e05 100644 --- a/cros_async/src/lib.rs +++ b/cros_async/src/lib.rs @@ -70,7 +70,6 @@ mod select; pub mod sync; pub mod sys; pub use sys::Executor; -pub use sys::ExecutorKind; mod timer; mod waker; diff --git a/cros_async/src/sys.rs b/cros_async/src/sys.rs index 81bf39321d..6af6fd361c 100644 --- a/cros_async/src/sys.rs +++ b/cros_async/src/sys.rs @@ -5,16 +5,9 @@ cfg_if::cfg_if! { if #[cfg(unix)] { pub mod unix; - pub use unix as platform; + pub use unix::{async_types, event, executor::Executor, run_one}; } else if #[cfg(windows)] { pub mod windows; - pub use windows as platform; + pub use windows::{async_types, event, executor::Executor, run_one}; } } - -pub use platform::async_types; -pub use platform::event; -pub use platform::executor::Executor; -pub use platform::executor::ExecutorKind; -pub use platform::executor::SetDefaultExecutorKindError; -pub use platform::run_one; diff --git a/cros_async/src/sys/unix/event.rs b/cros_async/src/sys/unix/event.rs index 4cd984d9d1..d98822c29b 100644 --- a/cros_async/src/sys/unix/event.rs +++ b/cros_async/src/sys/unix/event.rs @@ -37,7 +37,7 @@ impl EventAsync { #[cfg(test)] mod tests { use super::*; - use crate::sys::unix::uring_executor::is_uring_stable; + use crate::sys::unix::uring_executor::use_uring; #[test] fn next_val_reads_value() { @@ -55,7 +55,7 @@ mod tests { #[test] fn next_val_reads_value_poll_and_ring() { - if !is_uring_stable() { + if !use_uring() { return; } diff --git a/cros_async/src/sys/unix/executor.rs b/cros_async/src/sys/unix/executor.rs index 77ee03b743..22cd32c31b 100644 --- a/cros_async/src/sys/unix/executor.rs +++ b/cros_async/src/sys/unix/executor.rs @@ -3,19 +3,13 @@ // found in the LICENSE file. use std::future::Future; -use std::str::FromStr; use async_task::Task; -use base::warn; use base::AsRawDescriptors; use base::RawDescriptor; -use once_cell::sync::OnceCell; -use thiserror::Error as ThisError; use super::poll_source::Error as PollError; -use super::uring_executor::check_uring_availability; -use super::uring_executor::is_uring_stable; -use super::uring_executor::Error as UringError; +use super::uring_executor::use_uring; use super::FdExecutor; use super::PollSource; use super::URingExecutor; @@ -160,86 +154,18 @@ pub enum Executor { Fd(FdExecutor), } -/// An enum to express the kind of the backend of `Executor` -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub enum ExecutorKind { - Uring, - Fd, -} - -/// If set, [`ExecutorKind::default()`] returns the value of `DEFAULT_EXECUTOR_KIND`. -/// If not set, [`ExecutorKind::default()`] returns a statically-chosen default value, and -/// [`ExecutorKind::default()`] initializes `DEFAULT_EXECUTOR_KIND` with that value. -static DEFAULT_EXECUTOR_KIND: OnceCell = OnceCell::new(); - -impl Default for ExecutorKind { - fn default() -> Self { - *DEFAULT_EXECUTOR_KIND.get_or_init(|| ExecutorKind::Fd) - } -} - -impl FromStr for ExecutorKind { - type Err = &'static str; - - fn from_str(s: &str) -> Result { - match s { - "uring" => Ok(ExecutorKind::Uring), - // For command-line parsing, user-friendly "epoll" is chosen instead of fd. - "epoll" => Ok(ExecutorKind::Fd), - _ => Err("unknown executor kind"), - } - } -} - -/// The error type for [`Executor::set_default_executor_kind()`]. -#[derive(Debug, ThisError)] -pub enum SetDefaultExecutorKindError { - /// The default executor kind is set more than once. - #[error("The default executor kind is already set to {0:?}")] - SetMoreThanOnce(ExecutorKind), - - /// io_uring is unavailable. The reason might be the lack of the kernel support, - /// but is not limited to that. - #[error("io_uring is unavailable: {0}")] - UringUnavailable(UringError), -} - impl Executor { /// Create a new `Executor`. pub fn new() -> AsyncResult { - match ExecutorKind::default() { - ExecutorKind::Uring => Ok(URingExecutor::new().map(Executor::Uring)?), - ExecutorKind::Fd => Ok(FdExecutor::new() + if use_uring() { + Ok(URingExecutor::new().map(Executor::Uring)?) + } else { + Ok(FdExecutor::new() .map(Executor::Fd) - .map_err(PollError::Executor)?), + .map_err(PollError::Executor)?) } } - /// Set the default ExecutorKind for [`Self::new()`]. This call is effective only once. - /// If a call is the first call, it sets the default, and `set_default_executor_kind` - /// returns `Ok(())`. Otherwise, it returns `SetDefaultExecutorKindError::SetMoreThanOnce` - /// which contains the existing ExecutorKind value configured by the first call. - pub fn set_default_executor_kind( - executor_kind: ExecutorKind, - ) -> Result<(), SetDefaultExecutorKindError> { - if executor_kind == ExecutorKind::Uring { - check_uring_availability().map_err(SetDefaultExecutorKindError::UringUnavailable)?; - if !is_uring_stable() { - warn!( - "Enabling io_uring executor on the kernel version where io_uring is unstable" - ); - } - } - - DEFAULT_EXECUTOR_KIND.set(executor_kind).map_err(|_| - // `expect` succeeds since this closure runs only when DEFAULT_EXECUTOR_KIND is set. - SetDefaultExecutorKindError::SetMoreThanOnce( - *DEFAULT_EXECUTOR_KIND - .get() - .expect("Failed to get DEFAULT_EXECUTOR_KIND"), - )) - } - /// Create a new `Box>` associated with `self`. Callers may then use the /// returned `IoSourceExt` to directly start async operations without needing a separate /// reference to the executor. diff --git a/cros_async/src/sys/unix/timer.rs b/cros_async/src/sys/unix/timer.rs index 13c844224f..62efbc0976 100644 --- a/cros_async/src/sys/unix/timer.rs +++ b/cros_async/src/sys/unix/timer.rs @@ -28,7 +28,7 @@ mod tests { use std::time::Instant; use super::*; - use crate::sys::unix::uring_executor::is_uring_stable; + use crate::sys::unix::uring_executor::use_uring; use crate::Executor; #[test] @@ -46,7 +46,7 @@ mod tests { #[test] fn one_shot() { - if !is_uring_stable() { + if !use_uring() { return; } diff --git a/cros_async/src/sys/unix/uring_executor.rs b/cros_async/src/sys/unix/uring_executor.rs index dfc9bd133e..1b3ef7f492 100644 --- a/cros_async/src/sys/unix/uring_executor.rs +++ b/cros_async/src/sys/unix/uring_executor.rs @@ -148,7 +148,7 @@ impl From for io::Error { } } -static IS_URING_STABLE: Lazy = Lazy::new(|| { +static USE_URING: Lazy = Lazy::new(|| { let mut utsname = MaybeUninit::zeroed(); // Safe because this will only modify `utsname` and we check the return value. @@ -178,20 +178,11 @@ static IS_URING_STABLE: Lazy = Lazy::new(|| { } }); -// Checks if the uring executor is stable. +// Checks if the uring executor is available. // Caches the result so that the check is only run once. // Useful for falling back to the FD executor on pre-uring kernels. -pub(crate) fn is_uring_stable() -> bool { - *IS_URING_STABLE -} - -// Checks the uring availability by checking if the uring creation succeeds. -// If uring creation succeeds, it returns `Ok(())`. It returns an `URingContextError` otherwise. -// It fails if the kernel does not support io_uring, but note that the cause is not limited to it. -pub(crate) fn check_uring_availability() -> Result<()> { - URingContext::new(8) - .map(drop) - .map_err(Error::URingContextError) +pub(crate) fn use_uring() -> bool { + *USE_URING } pub struct RegisteredSource { @@ -1001,7 +992,7 @@ mod tests { #[test] fn dont_drop_backing_mem_read() { - if !is_uring_stable() { + if !use_uring() { return; } @@ -1045,7 +1036,7 @@ mod tests { #[test] fn dont_drop_backing_mem_write() { - if !is_uring_stable() { + if !use_uring() { return; } @@ -1090,7 +1081,7 @@ mod tests { #[test] fn canceled_before_completion() { - if !is_uring_stable() { + if !use_uring() { return; } @@ -1134,7 +1125,7 @@ mod tests { #[ignore] #[test] fn drop_before_completion() { - if !is_uring_stable() { + if !use_uring() { return; } @@ -1183,7 +1174,7 @@ mod tests { #[test] fn drop_on_different_thread() { - if !is_uring_stable() { + if !use_uring() { return; } diff --git a/cros_async/src/sys/unix/uring_source.rs b/cros_async/src/sys/unix/uring_source.rs index 4177e1adc0..f98062fd2f 100644 --- a/cros_async/src/sys/unix/uring_source.rs +++ b/cros_async/src/sys/unix/uring_source.rs @@ -230,7 +230,7 @@ mod tests { use std::fs::OpenOptions; use std::path::PathBuf; - use super::super::uring_executor::is_uring_stable; + use super::super::uring_executor::use_uring; use super::super::UringSource; use super::*; use crate::io_ext::ReadAsync; @@ -238,7 +238,7 @@ mod tests { #[test] fn read_to_mem() { - if !is_uring_stable() { + if !use_uring() { return; } @@ -277,7 +277,7 @@ mod tests { #[test] fn readvec() { - if !is_uring_stable() { + if !use_uring() { return; } @@ -299,7 +299,7 @@ mod tests { #[test] fn readmulti() { - if !is_uring_stable() { + if !use_uring() { return; } @@ -334,7 +334,7 @@ mod tests { #[test] fn u64_from_file() { - if !is_uring_stable() { + if !use_uring() { return; } @@ -347,7 +347,7 @@ mod tests { #[test] fn event() { - if !is_uring_stable() { + if !use_uring() { return; } @@ -388,7 +388,7 @@ mod tests { #[test] fn pend_on_pipe() { - if !is_uring_stable() { + if !use_uring() { return; } @@ -418,7 +418,7 @@ mod tests { #[test] fn readmem() { - if !is_uring_stable() { + if !use_uring() { return; } @@ -472,7 +472,7 @@ mod tests { #[test] fn range_error() { - if !is_uring_stable() { + if !use_uring() { return; } @@ -500,7 +500,7 @@ mod tests { #[test] fn fallocate() { - if !is_uring_stable() { + if !use_uring() { return; } @@ -539,7 +539,7 @@ mod tests { #[test] fn fsync() { - if !is_uring_stable() { + if !use_uring() { return; } @@ -555,7 +555,7 @@ mod tests { #[test] fn wait_read() { - if !is_uring_stable() { + if !use_uring() { return; } @@ -571,7 +571,7 @@ mod tests { #[test] fn writemem() { - if !is_uring_stable() { + if !use_uring() { return; } @@ -597,7 +597,7 @@ mod tests { #[test] fn writevec() { - if !is_uring_stable() { + if !use_uring() { return; } @@ -622,7 +622,7 @@ mod tests { #[test] fn writemulti() { - if !is_uring_stable() { + if !use_uring() { return; } diff --git a/cros_async/src/sys/windows/executor.rs b/cros_async/src/sys/windows/executor.rs index 6638583344..cbc0dbf997 100644 --- a/cros_async/src/sys/windows/executor.rs +++ b/cros_async/src/sys/windows/executor.rs @@ -3,11 +3,8 @@ // found in the LICENSE file. use std::future::Future; -use std::str::FromStr; use async_task::Task; -use once_cell::sync::OnceCell; -use thiserror::Error as ThisError; use super::HandleExecutor; use super::HandleSource; @@ -125,49 +122,10 @@ pub enum Executor { Handle(HandleExecutor), } -/// An enum to express the kind of the backend of `Executor` -#[derive(Clone, Copy, Debug)] -pub enum ExecutorKind { - Handle, -} - -/// If set, [`Executor::new()`] is created with `ExecutorKind` of `DEFAULT_EXECUTOR_KIND`. -static DEFAULT_EXECUTOR_KIND: OnceCell = OnceCell::new(); - -impl FromStr for ExecutorKind { - type Err = &'static str; - - fn from_str(s: &str) -> Result { - match s { - "handle" => Ok(ExecutorKind::Handle), - _ => Err("unknown executor kind"), - } - } -} - -impl Default for ExecutorKind { - fn default() -> Self { - DEFAULT_EXECUTOR_KIND - .get() - .copied() - .unwrap_or(ExecutorKind::Handle) - } -} - -/// The error type for [`Executor::set_default_executor_kind()`]. -#[derive(ThisError, Debug)] -pub enum SetDefaultExecutorKindError { - /// The default executor kind is set more than once. - #[error("The default executor kind is already set to {0:?}")] - SetMoreThanOnce(ExecutorKind), -} - impl Executor { /// Create a new `Executor`. pub fn new() -> AsyncResult { - match ExecutorKind::default() { - ExecutorKind::Handle => Ok(Executor::Handle(HandleExecutor::new())), - } + Ok(Executor::Handle(HandleExecutor::new())) } /// Create a new `Box>` associated with `self`. Callers may then use the @@ -182,22 +140,6 @@ impl Executor { } } - /// Set the default ExecutorKind for [`Self::new()`]. This call is effective only once. - /// If a call is the first call, it sets the default, and `set_default_executor_kind` - /// returns `Ok(())`. Otherwise, it returns `SetDefaultExecutorKindError::SetMoreThanOnce` - /// which contains the existing ExecutorKind value configured by the first call. - pub fn set_default_executor_kind( - executor_kind: ExecutorKind, - ) -> Result<(), SetDefaultExecutorKindError> { - DEFAULT_EXECUTOR_KIND.set(executor_kind).map_err(|_| - // `expect` succeeds since this closure runs only when DEFAULT_EXECUTOR_KIND is set. - SetDefaultExecutorKindError::SetMoreThanOnce( - *DEFAULT_EXECUTOR_KIND - .get() - .expect("Failed to get DEFAULT_EXECUTOR_KIND"), - )) - } - /// Spawn a new future for this executor to run to completion. Callers may use the returned /// `Task` to await on the result of `f`. Dropping the returned `Task` will cancel `f`, /// preventing it from being polled again. To drop a `Task` without canceling the future diff --git a/src/crosvm/cmdline.rs b/src/crosvm/cmdline.rs index 4e4efaefcb..1b5b65e0c2 100644 --- a/src/crosvm/cmdline.rs +++ b/src/crosvm/cmdline.rs @@ -30,7 +30,6 @@ use arch::Pstore; use arch::VcpuAffinity; use argh::FromArgs; use base::getpid; -use cros_async::ExecutorKind; use devices::virtio::block::block::DiskOption; #[cfg(any(feature = "video-decoder", feature = "video-encoder"))] use devices::virtio::device_constants::video::VideoDeviceConfig; @@ -106,10 +105,6 @@ pub struct CrosvmCmdlineArgs { #[argh(switch)] /// disable output to syslog pub no_syslog: bool, - /// configure async executor backend; "uring" or "epoll" on Linux, "handle" on Windows. - /// If this option is omitted on Linux, "epoll" is used by default. - #[argh(option, arg_name = "EXECUTOR")] - pub async_executor: Option, #[argh(subcommand)] pub command: Command, } diff --git a/src/main.rs b/src/main.rs index f20889255b..40fe530939 100644 --- a/src/main.rs +++ b/src/main.rs @@ -528,11 +528,6 @@ fn crosvm_main() -> Result { ..Default::default() }; - if let Some(async_executor) = args.async_executor { - cros_async::Executor::set_default_executor_kind(async_executor) - .context("Failed to set the default async executor")?; - } - let ret = match args.command { Command::CrossPlatform(command) => { // Past this point, usage of exit is in danger of leaking zombie processes.