From cd02e7f2c54014f56fe4a30c86c5f11a1773ca36 Mon Sep 17 00:00:00 2001 From: Luqman Aden Date: Wed, 1 Sep 2021 15:27:20 -0700 Subject: [PATCH 1/3] Use preadv/pwritev to efficiently handle multiple mappings at once. --- propolis/Cargo.toml | 3 +- propolis/src/block.rs | 41 +++--------------- propolis/src/vmm/mapping.rs | 86 ++++++++++++++++++++++++++++++++++++- 3 files changed, 93 insertions(+), 37 deletions(-) diff --git a/propolis/Cargo.toml b/propolis/Cargo.toml index 3d244ad53..efacaa906 100644 --- a/propolis/Cargo.toml +++ b/propolis/Cargo.toml @@ -8,7 +8,8 @@ edition = "2018" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -libc = "0.2" +#libc = "0.2" +libc = { git = "https://github.com/rust-lang/libc.git" } bitflags = "1.2" byteorder = "1" lazy_static = "1.4" diff --git a/propolis/src/block.rs b/propolis/src/block.rs index 836eaebca..39e5fa585 100644 --- a/propolis/src/block.rs +++ b/propolis/src/block.rs @@ -11,7 +11,7 @@ use std::sync::{Arc, Mutex, Weak}; use crate::common::*; use crate::dispatch::{DispCtx, Dispatcher}; -use crate::vmm::{MemCtx, SubMapping}; +use crate::vmm::{MappingExt, MemCtx, SubMapping}; /// Type of operations which may be issued to a virtual block device. #[derive(Copy, Clone, Debug, PartialEq)] @@ -165,40 +165,11 @@ impl FileBdev { let total_size: usize = mappings.iter().map(|x| x.len()).sum(); - let nbytes = { - let mut nbytes = 0; - - for mapping in mappings { - let inner_nbytes = if is_read { - mapping.pread( - &self.fp, - mapping.len(), - (offset + nbytes) as i64, - )? - } else { - mapping.pwrite( - &self.fp, - mapping.len(), - (offset + nbytes) as i64, - )? - }; - - if inner_nbytes != mapping.len() { - println!( - "{} at offset {} of size {} incomplete! only {} bytes", - if is_read { "read" } else { "write" }, - offset + nbytes, - mapping.len(), - inner_nbytes, - ); - return Ok(BlockResult::Failure); - } - - nbytes += inner_nbytes; - } - - nbytes - }; + let nbytes = if is_read { + mappings.preadv(&self.fp, offset as i64) + } else { + mappings.pwritev(&self.fp, offset as i64) + }?; assert_eq!(nbytes as usize, total_size); Ok(BlockResult::Success) diff --git a/propolis/src/vmm/mapping.rs b/propolis/src/vmm/mapping.rs index 83af60b0e..739a2791c 100644 --- a/propolis/src/vmm/mapping.rs +++ b/propolis/src/vmm/mapping.rs @@ -1,5 +1,7 @@ //! Module for managing guest memory mappings. +use libc::iovec; + use crate::common::PAGE_SIZE; use crate::util::aspace::ASpace; use crate::vmm::VmmFile; @@ -370,7 +372,7 @@ impl<'a> SubMapping<'a> { if !self.prot.contains(Prot::WRITE) { return Err(Error::new( ErrorKind::PermissionDenied, - "No read access", + "No write access", )); } @@ -517,6 +519,88 @@ impl<'a> SubMapping<'a> { } } +pub trait MappingExt { + /// preadv from `file` into multiple mappings + fn preadv(&self, file: &File, offset: i64) -> Result; + + /// pwritev from multiple mappings to `file` + fn pwritev(&self, file: &File, offset: i64) -> Result; +} + +impl<'a, T: AsRef<[SubMapping<'a>]>> MappingExt for T { + fn preadv(&self, file: &File, offset: i64) -> Result { + if !self + .as_ref() + .iter() + .all(|mapping| mapping.prot.contains(Prot::WRITE)) + { + return Err(Error::new( + ErrorKind::PermissionDenied, + "No write access", + )); + } + + let iov = self + .as_ref() + .iter() + .map(|mapping| iovec { + iov_base: mapping.ptr.as_ptr() as *mut libc::c_void, + iov_len: mapping.len, + }) + .collect::>(); + + let read = unsafe { + libc::preadv( + file.as_raw_fd(), + iov.as_ptr(), + iov.len() as libc::c_int, + offset, + ) + }; + if read == -1 { + return Err(Error::last_os_error()); + } + + Ok(read as usize) + } + + fn pwritev(&self, file: &File, offset: i64) -> Result { + if !self + .as_ref() + .iter() + .all(|mapping| mapping.prot.contains(Prot::READ)) + { + return Err(Error::new( + ErrorKind::PermissionDenied, + "No read access", + )); + } + + let iov = self + .as_ref() + .iter() + .map(|mapping| iovec { + iov_base: mapping.ptr.as_ptr() as *mut libc::c_void, + iov_len: mapping.len, + }) + .collect::>(); + + let written = unsafe { + libc::pwritev( + file.as_raw_fd(), + iov.as_ptr(), + iov.len() as libc::c_int, + offset, + ) + }; + if written == -1 { + return Err(Error::last_os_error()); + } + + Ok(written as usize) + } +} + #[cfg(test)] pub mod tests { use super::*; From 7104797119362d927c83709dcd8226d5bf774373 Mon Sep 17 00:00:00 2001 From: Luqman Aden Date: Thu, 2 Sep 2021 14:53:12 -0700 Subject: [PATCH 2/3] Add units to nvme dtrace script. --- scripts/nvme-trace.d | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/nvme-trace.d b/scripts/nvme-trace.d index ebc2e98ef..470d78651 100755 --- a/scripts/nvme-trace.d +++ b/scripts/nvme-trace.d @@ -40,7 +40,7 @@ propolis$target:::nvme_write_complete this->cid = args[0]; this->elapsed = timestamp - io[this->cid].ts; this->elapsed_us = this->elapsed / 1000; - @time[io[this->cid].op] = quantize(this->elapsed_us); + @time[strjoin(io[this->cid].op, " (us)")] = quantize(this->elapsed_us); printf("%s(cid=%u) %d blocks from LBA 0x%x in %uus\n", io[this->cid].op, this->cid, io[this->cid].nlb, io[this->cid].slba, this->elapsed_us); } From 80a5c456c49962c444784c3096b9770c8705e57c Mon Sep 17 00:00:00 2001 From: Luqman Aden Date: Fri, 3 Sep 2021 10:08:27 -0700 Subject: [PATCH 3/3] Use specific git rev for libc while we wait for a new release. --- propolis/Cargo.toml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/propolis/Cargo.toml b/propolis/Cargo.toml index efacaa906..564fcfa21 100644 --- a/propolis/Cargo.toml +++ b/propolis/Cargo.toml @@ -9,7 +9,9 @@ edition = "2018" [dependencies] #libc = "0.2" -libc = { git = "https://github.com/rust-lang/libc.git" } +# TODO: Switch back to "0.2" once a new release is published +# See https://github.com/rust-lang/libc/pull/2383 +libc = { git = "https://github.com/rust-lang/libc.git", rev = "796459785" } bitflags = "1.2" byteorder = "1" lazy_static = "1.4"