From 6074d45aeb339b1e68570ef01f2c96839ec2be16 Mon Sep 17 00:00:00 2001 From: Chirantan Ekbote Date: Tue, 6 Jul 2021 16:35:07 +0900 Subject: [PATCH] cros_async: Don't use io_uring on kernels < 5.10 Kernels before 5.10 had known bugs in the io_uring implementation. Don't use io_uring when we detect this. Also skip all the io_uring tests in this case. BUG=none TEST=cargo test Change-Id: I5fd6203ad25a6fb85ff28f1a6ddb0181f836ad89 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3006309 Tested-by: kokoro Commit-Queue: Chirantan Ekbote Reviewed-by: Keiichi Watanabe Reviewed-by: Woody Chow --- Cargo.lock | 1 + cros_async/Cargo.toml | 3 +- cros_async/src/uring_executor.rs | 76 +++++++++++++++++++-------- cros_async/src/uring_source.rs | 77 ++++++++++++++++++++-------- seccomp/aarch64/common_device.policy | 1 + seccomp/aarch64/gpu_device.policy | 2 +- seccomp/aarch64/xhci.policy | 1 - seccomp/arm/common_device.policy | 1 + seccomp/arm/gpu_device.policy | 2 +- seccomp/arm/xhci.policy | 1 - seccomp/x86_64/common_device.policy | 1 + seccomp/x86_64/gpu_device.policy | 2 +- seccomp/x86_64/video_device.policy | 1 - seccomp/x86_64/xhci.policy | 1 - 14 files changed, 121 insertions(+), 49 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c73c4337d0..bb1f45d99b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -194,6 +194,7 @@ dependencies = [ "intrusive-collections", "io_uring", "libc", + "once_cell", "paste", "pin-utils", "slab", diff --git a/cros_async/Cargo.toml b/cros_async/Cargo.toml index fcaf95bc7e..9b1299ebb1 100644 --- a/cros_async/Cargo.toml +++ b/cros_async/Cargo.toml @@ -7,15 +7,16 @@ edition = "2018" [dependencies] async-trait = "0.1.36" async-task = "4" +data_model = { path = "../data_model" } # provided by ebuild intrusive-collections = "0.9" io_uring = { path = "../io_uring" } # provided by ebuild libc = "*" +once_cell = "1.7.2" paste = "1.0" pin-utils = "0.1.0-alpha.4" slab = "0.4" sync = { path = "../sync" } # provided by ebuild sys_util = { path = "../sys_util" } # provided by ebuild -data_model = { path = "../data_model" } # provided by ebuild thiserror = "1.0.20" [dependencies.futures] diff --git a/cros_async/src/uring_executor.rs b/cros_async/src/uring_executor.rs index 08a644826b..bedf56979f 100644 --- a/cros_async/src/uring_executor.rs +++ b/cros_async/src/uring_executor.rs @@ -52,13 +52,14 @@ //! ensure it lives long enough. use std::convert::TryInto; +use std::ffi::CStr; use std::fs::File; use std::future::Future; use std::io; -use std::mem; +use std::mem::{self, MaybeUninit}; use std::os::unix::io::{AsRawFd, FromRawFd, RawFd}; use std::pin::Pin; -use std::sync::atomic::{AtomicI32, AtomicU32, Ordering}; +use std::sync::atomic::{AtomicI32, Ordering}; use std::sync::{Arc, Weak}; use std::task::Waker; use std::task::{Context, Poll}; @@ -67,6 +68,7 @@ use std::thread::{self, ThreadId}; use async_task::Task; use futures::task::noop_waker; use io_uring::URingContext; +use once_cell::sync::Lazy; use pin_utils::pin_mut; use slab::Slab; use sync::Mutex; @@ -130,29 +132,41 @@ impl From for io::Error { } } +static USE_URING: Lazy = Lazy::new(|| { + let mut utsname = MaybeUninit::zeroed(); + + // Safe because this will only modify `utsname` and we check the return value. + let res = unsafe { libc::uname(utsname.as_mut_ptr()) }; + if res < 0 { + return false; + } + + // Safe because the kernel has initialized `utsname`. + let utsname = unsafe { utsname.assume_init() }; + + // Safe because the pointer is valid and the kernel guarantees that this is a valid C string. + let release = unsafe { CStr::from_ptr(utsname.release.as_ptr()) }; + + let mut components = match release.to_str().map(|r| r.split('.').map(str::parse)) { + Ok(c) => c, + Err(_) => return false, + }; + + // Kernels older than 5.10 either didn't support io_uring or had bugs in the implementation. + match (components.next(), components.next()) { + (Some(Ok(major)), Some(Ok(minor))) if (major, minor) >= (5, 10) => { + // The kernel version is new enough so check if we can actually make a uring context. + URingContext::new(8).is_ok() + } + _ => false, + } +}); + // 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 use_uring() -> bool { - const UNKNOWN: u32 = 0; - const URING: u32 = 1; - const FD: u32 = 2; - static USE_URING: AtomicU32 = AtomicU32::new(UNKNOWN); - match USE_URING.load(Ordering::Relaxed) { - UNKNOWN => { - // Create a dummy uring context to check that the kernel understands the syscalls. - if URingContext::new(8).is_ok() { - USE_URING.store(URING, Ordering::Relaxed); - true - } else { - USE_URING.store(FD, Ordering::Relaxed); - false - } - } - URING => true, - FD => false, - _ => unreachable!("invalid use uring state"), - } + *USE_URING } pub struct RegisteredSource { @@ -880,6 +894,10 @@ mod tests { #[test] fn dont_drop_backing_mem_read() { + if !use_uring() { + return; + } + // Create a backing memory wrapped in an Arc and check that the drop isn't called while the // op is pending. let bm = @@ -920,6 +938,10 @@ mod tests { #[test] fn dont_drop_backing_mem_write() { + if !use_uring() { + return; + } + // Create a backing memory wrapped in an Arc and check that the drop isn't called while the // op is pending. let bm = @@ -961,6 +983,10 @@ mod tests { #[test] fn canceled_before_completion() { + if !use_uring() { + return; + } + async fn cancel_io(op: PendingOperation) { mem::drop(op); } @@ -999,6 +1025,10 @@ mod tests { #[test] fn drop_before_completion() { + if !use_uring() { + return; + } + const VALUE: u64 = 0xef6c_a8df_b842_eb9c; async fn check_op(op: PendingOperation) { @@ -1044,6 +1074,10 @@ mod tests { #[test] fn drop_on_different_thread() { + if !use_uring() { + return; + } + let ex = URingExecutor::new().unwrap(); let ex2 = ex.clone(); diff --git a/cros_async/src/uring_source.rs b/cros_async/src/uring_source.rs index fa1b3c2c62..c9f0f746e1 100644 --- a/cros_async/src/uring_source.rs +++ b/cros_async/src/uring_source.rs @@ -18,26 +18,6 @@ use crate::AsyncResult; /// `UringSource` wraps FD backed IO sources for use with io_uring. It is a thin wrapper around /// registering an IO source with the uring that provides an `IoSource` implementation. /// Most useful functions are provided by 'IoSourceExt'. -/// -/// # Example -/// ```rust -/// use std::fs::File; -/// use cros_async::{UringSource, ReadAsync, URingExecutor}; -/// -/// async fn read_four_bytes(source: &UringSource) -> (usize, Vec) { -/// let mem = vec![0u8; 4]; -/// source.read_to_vec(0, mem).await.unwrap() -/// } -/// -/// fn read_file(f: File) -> Result<(), Box> { -/// let ex = URingExecutor::new()?; -/// let async_source = UringSource::new(f, &ex)?; -/// let (nread, vec) = ex.run_until(read_four_bytes(&async_source))?; -/// assert_eq!(nread, 4); -/// assert_eq!(vec.len(), 4); -/// Ok(()) -/// } -/// ``` pub struct UringSource { registered_source: RegisteredSource, source: F, @@ -236,12 +216,17 @@ mod tests { use std::path::PathBuf; use crate::io_ext::{ReadAsync, WriteAsync}; + use crate::uring_executor::use_uring; use crate::UringSource; use super::*; #[test] fn read_to_mem() { + if !use_uring() { + return; + } + use crate::mem::VecIoWrapper; use std::io::Write; use tempfile::tempfile; @@ -275,6 +260,10 @@ mod tests { #[test] fn readvec() { + if !use_uring() { + return; + } + async fn go(ex: &URingExecutor) { let f = File::open("/dev/zero").unwrap(); let source = UringSource::new(f, ex).unwrap(); @@ -293,6 +282,10 @@ mod tests { #[test] fn readmulti() { + if !use_uring() { + return; + } + async fn go(ex: &URingExecutor) { let f = File::open("/dev/zero").unwrap(); let source = UringSource::new(f, ex).unwrap(); @@ -321,6 +314,10 @@ mod tests { #[test] fn u64_from_file() { + if !use_uring() { + return; + } + let f = File::open("/dev/zero").unwrap(); let ex = URingExecutor::new().unwrap(); let source = UringSource::new(f, &ex).unwrap(); @@ -330,6 +327,10 @@ mod tests { #[test] fn event() { + if !use_uring() { + return; + } + use sys_util::EventFd; async fn write_event(ev: EventFd, wait: EventFd, ex: &URingExecutor) { @@ -367,6 +368,10 @@ mod tests { #[test] fn pend_on_pipe() { + if !use_uring() { + return; + } + use std::io::Write; use futures::future::Either; @@ -393,6 +398,10 @@ mod tests { #[test] fn readmem() { + if !use_uring() { + return; + } + async fn go(ex: &URingExecutor) { let f = File::open("/dev/zero").unwrap(); let source = UringSource::new(f, ex).unwrap(); @@ -443,6 +452,10 @@ mod tests { #[test] fn range_error() { + if !use_uring() { + return; + } + async fn go(ex: &URingExecutor) { let f = File::open("/dev/zero").unwrap(); let source = UringSource::new(f, ex).unwrap(); @@ -467,6 +480,10 @@ mod tests { #[test] fn fallocate() { + if !use_uring() { + return; + } + async fn go(ex: &URingExecutor) { let dir = tempfile::TempDir::new().unwrap(); let mut file_path = PathBuf::from(dir.path()); @@ -500,6 +517,10 @@ mod tests { #[test] fn fsync() { + if !use_uring() { + return; + } + async fn go(ex: &URingExecutor) { let f = tempfile::tempfile().unwrap(); let source = UringSource::new(f, ex).unwrap(); @@ -512,6 +533,10 @@ mod tests { #[test] fn wait_read() { + if !use_uring() { + return; + } + async fn go(ex: &URingExecutor) { let f = File::open("/dev/zero").unwrap(); let source = UringSource::new(f, ex).unwrap(); @@ -524,6 +549,10 @@ mod tests { #[test] fn writemem() { + if !use_uring() { + return; + } + async fn go(ex: &URingExecutor) { let f = OpenOptions::new() .create(true) @@ -546,6 +575,10 @@ mod tests { #[test] fn writevec() { + if !use_uring() { + return; + } + async fn go(ex: &URingExecutor) { let f = OpenOptions::new() .create(true) @@ -567,6 +600,10 @@ mod tests { #[test] fn writemulti() { + if !use_uring() { + return; + } + async fn go(ex: &URingExecutor) { let f = OpenOptions::new() .create(true) diff --git a/seccomp/aarch64/common_device.policy b/seccomp/aarch64/common_device.policy index 349afe9aa0..8899c316a3 100644 --- a/seccomp/aarch64/common_device.policy +++ b/seccomp/aarch64/common_device.policy @@ -44,3 +44,4 @@ sigaltstack: 1 write: 1 writev: 1 fcntl: 1 +uname: 1 diff --git a/seccomp/aarch64/gpu_device.policy b/seccomp/aarch64/gpu_device.policy index 4ceac5c46e..b452a9526d 100644 --- a/seccomp/aarch64/gpu_device.policy +++ b/seccomp/aarch64/gpu_device.policy @@ -42,6 +42,7 @@ set_robust_list: 1 sigaltstack: 1 write: 1 writev: 1 +uname: 1 # Required for perfetto tracing getsockopt: 1 @@ -77,7 +78,6 @@ tgkill: 1 clock_gettime: 1 # Rules specific to Mesa. -uname: 1 sched_setscheduler: 1 sched_setaffinity: 1 kcmp: 1 diff --git a/seccomp/aarch64/xhci.policy b/seccomp/aarch64/xhci.policy index 684ae0d2f6..b949ec21b5 100644 --- a/seccomp/aarch64/xhci.policy +++ b/seccomp/aarch64/xhci.policy @@ -17,7 +17,6 @@ openat: 1 setsockopt: 1 bind: 1 socket: arg0 == AF_NETLINK -uname: 1 # The following ioctls are: # 0x4004550d == USBDEVFS_REAPURBNDELAY32 # 0x550b == USBDEVFS_DISCARDURB diff --git a/seccomp/arm/common_device.policy b/seccomp/arm/common_device.policy index 165bfda6e5..859d7e1847 100644 --- a/seccomp/arm/common_device.policy +++ b/seccomp/arm/common_device.policy @@ -53,3 +53,4 @@ sigaltstack: 1 write: 1 writev: 1 fcntl64: 1 +uname: 1 diff --git a/seccomp/arm/gpu_device.policy b/seccomp/arm/gpu_device.policy index ec5a5b4815..40fd5ecc24 100644 --- a/seccomp/arm/gpu_device.policy +++ b/seccomp/arm/gpu_device.policy @@ -48,6 +48,7 @@ set_robust_list: 1 sigaltstack: 1 write: 1 writev: 1 +uname: 1 # Required for perfetto tracing getsockopt: 1 @@ -89,7 +90,6 @@ clock_gettime: 1 clock_gettime64: 1 # Rules specific to Mesa. -uname: 1 sched_setscheduler: 1 sched_setaffinity: 1 kcmp: 1 diff --git a/seccomp/arm/xhci.policy b/seccomp/arm/xhci.policy index ca1a73dfcd..322a2962ee 100644 --- a/seccomp/arm/xhci.policy +++ b/seccomp/arm/xhci.policy @@ -19,7 +19,6 @@ bind: 1 socket: arg0 == AF_NETLINK stat: 1 statx: 1 -uname: 1 # The following ioctls are: # 0x4004550d == USBDEVFS_REAPURBNDELAY32 # 0x550b == USBDEVFS_DISCARDURB diff --git a/seccomp/x86_64/common_device.policy b/seccomp/x86_64/common_device.policy index 49d452051d..f74b9c9a69 100644 --- a/seccomp/x86_64/common_device.policy +++ b/seccomp/x86_64/common_device.policy @@ -47,3 +47,4 @@ sigaltstack: 1 write: 1 writev: 1 fcntl: 1 +uname: 1 diff --git a/seccomp/x86_64/gpu_device.policy b/seccomp/x86_64/gpu_device.policy index dcfc081715..1386d75479 100644 --- a/seccomp/x86_64/gpu_device.policy +++ b/seccomp/x86_64/gpu_device.policy @@ -45,6 +45,7 @@ set_robust_list: 1 sigaltstack: 1 write: 1 writev: 1 +uname: 1 # Rules specific to gpu connect: 1 @@ -90,7 +91,6 @@ setpriority: 1 unlink: 1 # Rules specific to AMD gpus. -uname: 1 sched_setscheduler: 1 sched_setaffinity: 1 kcmp: 1 diff --git a/seccomp/x86_64/video_device.policy b/seccomp/x86_64/video_device.policy index 1f81f73df0..975f4029da 100644 --- a/seccomp/x86_64/video_device.policy +++ b/seccomp/x86_64/video_device.policy @@ -35,7 +35,6 @@ mprotect: arg2 == PROT_READ|PROT_WRITE || arg2 == PROT_NONE || arg2 == PROT_READ readlink: 1 sched_setaffinity: 1 sched_setscheduler: arg1 == SCHED_IDLE || arg1 == SCHED_BATCH -uname: 1 # Required by mesa on AMD GPU sysinfo: 1 diff --git a/seccomp/x86_64/xhci.policy b/seccomp/x86_64/xhci.policy index 9ef376698b..50b41d1945 100644 --- a/seccomp/x86_64/xhci.policy +++ b/seccomp/x86_64/xhci.policy @@ -18,7 +18,6 @@ open: return ENOENT openat: 1 socket: arg0 == AF_NETLINK stat: 1 -uname: 1 # The following ioctls are: # 0x4008550d == USBDEVFS_REAPURBNDELAY # 0x41045508 == USBDEVFS_GETDRIVER