Rewrite the aio module

The existing AIO implementation has some problems:
1) The in_progress field is checked at runtime, not compile time.
2) The mutable field is checked at runtime, not compile time.
3) A downstream lio_listio user must store extra state to track whether
   the whole operation is partially, completely, or not at all
   submitted.
4) Nix does heap allocation itself, rather than allowing the caller to
   choose it.  This can result in double (or triple, or quadruple)
   boxing.
5) There's no easy way to use lio_listio to submit multiple operations with
   a single syscall, but poll each individually.
6) The lio_listio usage is far from transparent and zero-cost.
7) No aio_readv or aio_writev support.
8) priority has type c_int; should be i32
9) aio_return should return a usize instead of an isize, since it only
   uses negative values to indicate errors, which Rust represents via
   the Result type.

This rewrite solves several problems:
1) Unsolved.  I don't think it can be solved without something like
   C++'s guaranteed type elision.  It might require changing the
   signature of Future::poll too.
2) Solved.
3) Solved, by the new in_progress method and by removing the complicated
   lio_listio resubmit code.
4) Solved.
5) Solved.
6) Solved, by removing the lio_listo resubmit code.  It can be
   reimplemented downstream if necessary.  Or even in Nix, but it
   doesn't fit Nix's theme of zero-cost abstractions.
7) Solved.
8) Solved.
9) Solved.

The rewrite includes functions that don't work on FreeBSD, so add CI
testing for FreeBSD 14 too.

By default only enable tests that will pass on FreeBSD 12.3.  But run a
CI job on FreeBSD 14 and set a flag that will enable such tests.
This commit is contained in:
Alan Somers 2022-04-30 14:10:28 -06:00
parent 1c36d49c0b
commit 0c07a9e469
9 changed files with 1592 additions and 1547 deletions

View File

@ -36,11 +36,18 @@ test: &TEST
# 64-bit kernel and in a 64-bit environment. Our tests don't execute any of # 64-bit kernel and in a 64-bit environment. Our tests don't execute any of
# the system's binaries, so the environment shouldn't matter. # the system's binaries, so the environment shouldn't matter.
task: task:
name: FreeBSD amd64 & i686
env: env:
TARGET: x86_64-unknown-freebsd TARGET: x86_64-unknown-freebsd
freebsd_instance: matrix:
image: freebsd-12-3-release-amd64 - name: FreeBSD 12 amd64 & i686
freebsd_instance:
image: freebsd-12-3-release-amd64
- name: FreeBSD 14 amd64 & i686
freebsd_instance:
image_family: freebsd-14-0-snap
# Enable tests that would fail on FreeBSD 12
RUSTFLAGS: --cfg fbsd14 -D warnings
RUSTDOCFLAGS: --cfg fbsd14
setup_script: setup_script:
- kldload mqueuefs - kldload mqueuefs
- fetch https://sh.rustup.rs -o rustup.sh - fetch https://sh.rustup.rs -o rustup.sh

View File

@ -6,14 +6,30 @@ This project adheres to [Semantic Versioning](https://semver.org/).
## [Unreleased] - ReleaseDate ## [Unreleased] - ReleaseDate
### Added ### Added
- Added `aio_writev` and `aio_readv`.
(#[1713](https://github.com/nix-rust/nix/pull/1713))
- impl From<SockaddrIn> for std::net::SocketAddrV4 and - impl From<SockaddrIn> for std::net::SocketAddrV4 and
impl From<SockaddrIn6> for std::net::SocketAddrV6. impl From<SockaddrIn6> for std::net::SocketAddrV6.
(#[1711](https://github.com/nix-rust/nix/pull/1711)) (#[1711](https://github.com/nix-rust/nix/pull/1711))
### Changed ### Changed
- Rewrote the aio module. The new module:
* Does more type checking at compile time rather than runtime.
* Gives the caller control over whether and when to `Box` an aio operation.
* Changes the type of the `priority` arguments to `i32`.
* Changes the return type of `aio_return` to `usize`.
(#[1713](https://github.com/nix-rust/nix/pull/1713))
### Fixed ### Fixed
### Removed ### Removed
- Removed support for resubmitting partially complete `lio_listio` operations.
It was too complicated, and didn't fit Nix's theme of zero-cost abstractions.
Instead, it can be reimplemented downstream.
(#[1713](https://github.com/nix-rust/nix/pull/1713))
## [0.24.1] - 2022-04-22 ## [0.24.1] - 2022-04-22
### Added ### Added
### Changed ### Changed

View File

@ -27,9 +27,10 @@ targets = [
] ]
[dependencies] [dependencies]
libc = { version = "0.2.124", features = [ "extra_traits" ] } libc = { git = "http://github.com/rust-lang/libc.git", rev = "cd99f681181c310abfba742aef11115d2eff03dc", features = [ "extra_traits" ] }
bitflags = "1.1" bitflags = "1.1"
cfg-if = "1.0" cfg-if = "1.0"
pin-utils = { version = "0.1.0", optional = true }
[target.'cfg(not(target_os = "redox"))'.dependencies] [target.'cfg(not(target_os = "redox"))'.dependencies]
memoffset = { version = "0.6.3", optional = true } memoffset = { version = "0.6.3", optional = true }
@ -44,7 +45,7 @@ default = [
] ]
acct = [] acct = []
aio = [] aio = ["pin-utils"]
dir = ["fs"] dir = ["fs"]
env = [] env = []
event = [] event = []
@ -102,10 +103,6 @@ path = "test/sys/test_aio_drop.rs"
name = "test-clearenv" name = "test-clearenv"
path = "test/test_clearenv.rs" path = "test/test_clearenv.rs"
[[test]]
name = "test-lio-listio-resubmit"
path = "test/sys/test_lio_listio_resubmit.rs"
[[test]] [[test]]
name = "test-mount" name = "test-mount"
path = "test/test_mount.rs" path = "test/test_mount.rs"

View File

@ -5,7 +5,8 @@ status = [
"Android i686", "Android i686",
"Android x86_64", "Android x86_64",
"DragonFly BSD x86_64", "DragonFly BSD x86_64",
"FreeBSD amd64 & i686", "FreeBSD 12 amd64 & i686",
"FreeBSD 14 amd64 & i686",
"Fuchsia x86_64", "Fuchsia x86_64",
"Linux MIPS", "Linux MIPS",
"Linux MIPS64 el", "Linux MIPS64 el",

View File

@ -742,8 +742,8 @@ impl SpacectlRange {
/// ///
/// # Example /// # Example
/// ///
// no_run because it fails to link until FreeBSD 14.0 #[cfg_attr(fbsd14, doc = " ```")]
/// ```no_run #[cfg_attr(not(fbsd14), doc = " ```no_run")]
/// # use std::io::Write; /// # use std::io::Write;
/// # use std::os::unix::fs::FileExt; /// # use std::os::unix::fs::FileExt;
/// # use std::os::unix::io::AsRawFd; /// # use std::os::unix::io::AsRawFd;
@ -788,8 +788,8 @@ pub fn fspacectl(fd: RawFd, range: SpacectlRange) -> Result<SpacectlRange> {
/// ///
/// # Example /// # Example
/// ///
// no_run because it fails to link until FreeBSD 14.0 #[cfg_attr(fbsd14, doc = " ```")]
/// ```no_run #[cfg_attr(not(fbsd14), doc = " ```no_run")]
/// # use std::io::Write; /// # use std::io::Write;
/// # use std::os::unix::fs::FileExt; /// # use std::os::unix::fs::FileExt;
/// # use std::os::unix::io::AsRawFd; /// # use std::os::unix::io::AsRawFd;

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

View File

@ -20,11 +20,10 @@ fn test_drop() {
let f = tempfile().unwrap(); let f = tempfile().unwrap();
f.set_len(6).unwrap(); f.set_len(6).unwrap();
let mut aiocb = AioCb::from_slice( f.as_raw_fd(), let mut aiocb = Box::pin(AioWrite::new(f.as_raw_fd(),
2, //offset 2, //offset
WBUF, WBUF,
0, //priority 0, //priority
SigevNotify::SigevNone, SigevNotify::SigevNone));
LioOpcode::LIO_NOP); aiocb.as_mut().submit().unwrap();
aiocb.write().unwrap();
} }

View File

@ -1,106 +0,0 @@
// vim: tw=80
// Annoyingly, Cargo is unable to conditionally build an entire test binary. So
// we must disable the test here rather than in Cargo.toml
#![cfg(target_os = "freebsd")]
use nix::errno::*;
use nix::libc::off_t;
use nix::sys::aio::*;
use nix::sys::signal::SigevNotify;
use nix::unistd::{SysconfVar, sysconf};
use std::os::unix::io::AsRawFd;
use std::{thread, time};
use sysctl::{CtlValue, Sysctl};
use tempfile::tempfile;
const BYTES_PER_OP: usize = 512;
/// Attempt to collect final status for all of `liocb`'s operations, freeing
/// system resources
fn finish_liocb(liocb: &mut LioCb) {
for j in 0..liocb.len() {
loop {
let e = liocb.error(j);
match e {
Ok(()) => break,
Err(Errno::EINPROGRESS) =>
thread::sleep(time::Duration::from_millis(10)),
Err(x) => panic!("aio_error({:?})", x)
}
}
assert_eq!(liocb.aio_return(j).unwrap(), BYTES_PER_OP as isize);
}
}
// Deliberately exceed system resource limits, causing lio_listio to return EIO.
// This test must run in its own process since it deliberately uses all AIO
// resources. ATM it is only enabled on FreeBSD, because I don't know how to
// check system AIO limits on other operating systems.
#[test]
fn test_lio_listio_resubmit() {
let mut resubmit_count = 0;
// Lookup system resource limits
let alm = sysconf(SysconfVar::AIO_LISTIO_MAX)
.expect("sysconf").unwrap() as usize;
let ctl = sysctl::Ctl::new("vfs.aio.max_aio_queue_per_proc").unwrap();
let maqpp = if let CtlValue::Int(x) = ctl.value().unwrap() {
x as usize
} else {
panic!("unknown sysctl");
};
// Find lio_listio sizes that satisfy the AIO_LISTIO_MAX constraint and also
// result in a final lio_listio call that can only partially be queued
let target_ops = maqpp + alm / 2;
let num_listios = (target_ops + alm - 3) / (alm - 2);
let ops_per_listio = (target_ops + num_listios - 1) / num_listios;
assert!((num_listios - 1) * ops_per_listio < maqpp,
"the last lio_listio won't make any progress; fix the algorithm");
println!("Using {:?} LioCbs of {:?} operations apiece", num_listios,
ops_per_listio);
let f = tempfile().unwrap();
let buffer_set = (0..num_listios).map(|_| {
(0..ops_per_listio).map(|_| {
vec![0u8; BYTES_PER_OP]
}).collect::<Vec<_>>()
}).collect::<Vec<_>>();
let mut liocbs = (0..num_listios).map(|i| {
let mut builder = LioCbBuilder::with_capacity(ops_per_listio);
for j in 0..ops_per_listio {
let offset = (BYTES_PER_OP * (i * ops_per_listio + j)) as off_t;
builder = builder.emplace_slice(f.as_raw_fd(),
offset,
&buffer_set[i][j][..],
0, //priority
SigevNotify::SigevNone,
LioOpcode::LIO_WRITE);
}
let mut liocb = builder.finish();
let mut err = liocb.listio(LioMode::LIO_NOWAIT, SigevNotify::SigevNone);
while err == Err(Errno::EIO) ||
err == Err(Errno::EAGAIN) ||
err == Err(Errno::EINTR) {
//
thread::sleep(time::Duration::from_millis(10));
resubmit_count += 1;
err = liocb.listio_resubmit(LioMode::LIO_NOWAIT,
SigevNotify::SigevNone);
}
liocb
}).collect::<Vec<_>>();
// Ensure that every AioCb completed
for liocb in liocbs.iter_mut() {
finish_liocb(liocb);
}
if resubmit_count > 0 {
println!("Resubmitted {:?} times, test passed", resubmit_count);
} else {
println!("Never resubmitted. Test ambiguous");
}
}