From 69ca4918dc1fa7b63c3e11f9709f96dd522191a3 Mon Sep 17 00:00:00 2001 From: jtnunley Date: Wed, 17 Aug 2022 14:05:18 -0700 Subject: [PATCH 1/3] fix unneeded try_into --- Cargo.toml | 1 + src/lib.rs | 16 ++++-- src/util.rs | 137 +++++++++++++++++++++++++++++++++++++++------------- 3 files changed, 116 insertions(+), 38 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 6683fa63..ba4be1f3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,6 +20,7 @@ members = [ "io-uring-test", "io-uring-bench" ] unstable = [] overwrite = [ "bindgen" ] direct-syscall = [ "sc" ] +io_safety = [] [dependencies] bitflags = "1" diff --git a/src/lib.rs b/src/lib.rs index 3177132e..4766ff46 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -14,11 +14,13 @@ mod submit; mod sys; pub mod types; -use std::convert::TryInto; use std::mem::ManuallyDrop; use std::os::unix::io::{AsRawFd, RawFd}; use std::{cmp, io, mem}; +#[cfg(feature = "io_safety")] +use std::os::unix::io::{AsFd, BorrowedFd}; + pub use cqueue::CompletionQueue; pub use register::Probe; pub use squeue::SubmissionQueue; @@ -122,9 +124,8 @@ impl IoUring { } let fd: Fd = unsafe { - sys::io_uring_setup(entries, &mut p) - .try_into() - .map_err(|_| io::Error::last_os_error())? + let fd = sys::io_uring_setup(entries, &mut p); + Fd::from_raw(fd).ok_or_else(io::Error::last_os_error)? }; let (mm, sq, cq) = unsafe { setup_queue(&fd, &p)? }; @@ -451,3 +452,10 @@ impl AsRawFd for IoUring { self.fd.as_raw_fd() } } + +#[cfg(feature = "io_safety")] +impl AsFd for IoUring { + fn as_fd(&self) -> BorrowedFd<'_> { + self.fd.as_fd() + } +} \ No newline at end of file diff --git a/src/util.rs b/src/util.rs index bce73be8..5ff72108 100644 --- a/src/util.rs +++ b/src/util.rs @@ -1,7 +1,6 @@ -use std::convert::TryFrom; -use std::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; +use std::os::unix::io::AsRawFd; use std::sync::atomic; -use std::{io, mem, ptr}; +use std::{io, ptr}; /// A region of memory mapped using `mmap(2)`. pub struct Mmap { @@ -60,49 +59,119 @@ impl Drop for Mmap { } } -/// An owned file descriptor. -pub struct Fd(pub RawFd); +pub use fd::Fd; -impl TryFrom for Fd { - type Error = (); +#[cfg(feature = "io_safety")] +mod fd { + use std::os::unix::io::{AsRawFd, RawFd, IntoRawFd, FromRawFd, OwnedFd, AsFd, BorrowedFd}; - #[inline] - fn try_from(value: RawFd) -> Result { - if value >= 0 { - Ok(Fd(value)) - } else { - Err(()) + /// An owned file descriptor. + pub struct Fd(pub OwnedFd); + + impl Fd { + /// Try to convert from a `RawFd` to an `Fd`. + /// + /// # Safety + /// + /// Must be a valid file descriptor. + pub unsafe fn from_raw(value: RawFd) -> Option { + if value >= 0 { + // SAFETY: not -1, and this is a valid FD + Some(Fd(OwnedFd::from_raw_fd(value))) + } else { + None + } + } + } + + impl AsRawFd for Fd { + fn as_raw_fd(&self) -> RawFd { + self.0.as_raw_fd() } } -} -impl AsRawFd for Fd { - #[inline] - fn as_raw_fd(&self) -> RawFd { - self.0 + impl IntoRawFd for Fd { + fn into_raw_fd(self) -> RawFd { + self.0.into_raw_fd() + } } -} -impl IntoRawFd for Fd { - #[inline] - fn into_raw_fd(self) -> RawFd { - let fd = self.0; - mem::forget(self); - fd + impl FromRawFd for Fd { + unsafe fn from_raw_fd(fd: RawFd) -> Self { + Fd(OwnedFd::from_raw_fd(fd)) + } } -} -impl FromRawFd for Fd { - #[inline] - unsafe fn from_raw_fd(fd: RawFd) -> Fd { - Fd(fd) + impl AsFd for Fd { + fn as_fd(&self) -> BorrowedFd<'_> { + self.0.as_fd() + } + } + + impl From for Fd { + fn from(fd: OwnedFd) -> Self { + Fd(fd) + } + } + + impl From for OwnedFd { + fn from(fd: Fd) -> Self { + fd.0 + } } } -impl Drop for Fd { - fn drop(&mut self) { - unsafe { - libc::close(self.0); +#[cfg(not(feature = "io_safety"))] +mod fd { + use std::mem; + use std::os::unix::io::{AsRawFd, RawFd, IntoRawFd, FromRawFd}; + + /// An owned file descriptor. + pub struct Fd(pub RawFd); + + impl Fd { + /// Try to convert from a `RawFd` to an `Fd`. + /// + /// # Safety + /// + /// Must be a valid file descriptor. + pub unsafe fn from_raw(value: RawFd) -> Option { + if value >= 0 { + Some(Fd(value)) + } else { + None + } + } + } + + impl AsRawFd for Fd { + #[inline] + fn as_raw_fd(&self) -> RawFd { + self.0 + } + } + + impl IntoRawFd for Fd { + #[inline] + fn into_raw_fd(self) -> RawFd { + let fd = self.0; + mem::forget(self); + fd + } + } + + impl FromRawFd for Fd { + #[inline] + unsafe fn from_raw_fd(fd: RawFd) -> Fd { + Fd(fd) + } + } + + impl Drop for Fd { + fn drop(&mut self) { + unsafe { + libc::close(self.0); + } } } } From ab281ea6bc16e950c5f2e9380c4cbf935325dcb8 Mon Sep 17 00:00:00 2001 From: jtnunley Date: Mon, 22 Aug 2022 11:12:17 -0700 Subject: [PATCH 2/3] use OwnedFd polyfill --- src/lib.rs | 16 +++++---- src/submit.rs | 6 ++-- src/util.rs | 92 ++++++--------------------------------------------- 3 files changed, 24 insertions(+), 90 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 4766ff46..b2f76a9c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -15,7 +15,7 @@ mod sys; pub mod types; use std::mem::ManuallyDrop; -use std::os::unix::io::{AsRawFd, RawFd}; +use std::os::unix::io::{AsRawFd, FromRawFd, RawFd}; use std::{cmp, io, mem}; #[cfg(feature = "io_safety")] @@ -25,13 +25,13 @@ pub use cqueue::CompletionQueue; pub use register::Probe; pub use squeue::SubmissionQueue; pub use submit::Submitter; -use util::{Fd, Mmap}; +use util::{OwnedFd, Mmap}; /// IoUring instance pub struct IoUring { sq: squeue::Inner, cq: cqueue::Inner, - fd: Fd, + fd: OwnedFd, params: Parameters, memory: ManuallyDrop, } @@ -85,7 +85,7 @@ impl IoUring { // I really hope that Rust can safely use self-reference types. #[inline] unsafe fn setup_queue( - fd: &Fd, + fd: &OwnedFd, p: &sys::io_uring_params, ) -> io::Result<(MemoryMap, squeue::Inner, cqueue::Inner)> { let sq_len = p.sq_off.array as usize + p.sq_entries as usize * mem::size_of::(); @@ -123,9 +123,13 @@ impl IoUring { } } - let fd: Fd = unsafe { + let fd: OwnedFd = unsafe { let fd = sys::io_uring_setup(entries, &mut p); - Fd::from_raw(fd).ok_or_else(io::Error::last_os_error)? + if fd >= 0 { + OwnedFd::from_raw_fd(fd) + } else { + return Err(io::Error::last_os_error()); + } }; let (mm, sq, cq) = unsafe { setup_queue(&fd, &p)? }; diff --git a/src/submit.rs b/src/submit.rs index daf5919d..54146277 100644 --- a/src/submit.rs +++ b/src/submit.rs @@ -4,7 +4,7 @@ use std::{io, ptr}; use crate::register::{execute, Probe}; use crate::sys; -use crate::util::{cast_ptr, Fd}; +use crate::util::{cast_ptr, OwnedFd}; use crate::Parameters; #[cfg(feature = "unstable")] @@ -19,7 +19,7 @@ use crate::types; /// io_uring supports both directly performing I/O on buffers and file descriptors and registering /// them beforehand. Registering is slow, but it makes performing the actual I/O much faster. pub struct Submitter<'a> { - fd: &'a Fd, + fd: &'a OwnedFd, params: &'a Parameters, sq_head: *const atomic::AtomicU32, @@ -30,7 +30,7 @@ pub struct Submitter<'a> { impl<'a> Submitter<'a> { #[inline] pub(crate) const fn new( - fd: &'a Fd, + fd: &'a OwnedFd, params: &'a Parameters, sq_head: *const atomic::AtomicU32, sq_tail: *const atomic::AtomicU32, diff --git a/src/util.rs b/src/util.rs index 5ff72108..b4e30d2c 100644 --- a/src/util.rs +++ b/src/util.rs @@ -10,7 +10,7 @@ pub struct Mmap { impl Mmap { /// Map `len` bytes starting from the offset `offset` in the file descriptor `fd` into memory. - pub fn new(fd: &Fd, offset: libc::off_t, len: usize) -> io::Result { + pub fn new(fd: &OwnedFd, offset: libc::off_t, len: usize) -> io::Result { unsafe { match libc::mmap( ptr::null_mut(), @@ -59,66 +59,11 @@ impl Drop for Mmap { } } -pub use fd::Fd; +pub use fd::OwnedFd; #[cfg(feature = "io_safety")] mod fd { - use std::os::unix::io::{AsRawFd, RawFd, IntoRawFd, FromRawFd, OwnedFd, AsFd, BorrowedFd}; - - /// An owned file descriptor. - pub struct Fd(pub OwnedFd); - - impl Fd { - /// Try to convert from a `RawFd` to an `Fd`. - /// - /// # Safety - /// - /// Must be a valid file descriptor. - pub unsafe fn from_raw(value: RawFd) -> Option { - if value >= 0 { - // SAFETY: not -1, and this is a valid FD - Some(Fd(OwnedFd::from_raw_fd(value))) - } else { - None - } - } - } - - impl AsRawFd for Fd { - fn as_raw_fd(&self) -> RawFd { - self.0.as_raw_fd() - } - } - - impl IntoRawFd for Fd { - fn into_raw_fd(self) -> RawFd { - self.0.into_raw_fd() - } - } - - impl FromRawFd for Fd { - unsafe fn from_raw_fd(fd: RawFd) -> Self { - Fd(OwnedFd::from_raw_fd(fd)) - } - } - - impl AsFd for Fd { - fn as_fd(&self) -> BorrowedFd<'_> { - self.0.as_fd() - } - } - - impl From for Fd { - fn from(fd: OwnedFd) -> Self { - Fd(fd) - } - } - - impl From for OwnedFd { - fn from(fd: Fd) -> Self { - fd.0 - } - } + pub use std::os::unix::io::OwnedFd; } #[cfg(not(feature = "io_safety"))] @@ -126,32 +71,17 @@ mod fd { use std::mem; use std::os::unix::io::{AsRawFd, RawFd, IntoRawFd, FromRawFd}; - /// An owned file descriptor. - pub struct Fd(pub RawFd); - - impl Fd { - /// Try to convert from a `RawFd` to an `Fd`. - /// - /// # Safety - /// - /// Must be a valid file descriptor. - pub unsafe fn from_raw(value: RawFd) -> Option { - if value >= 0 { - Some(Fd(value)) - } else { - None - } - } - } + /// API-compatible with the `OwnedFd` type in the Rust stdlib. + pub struct OwnedFd(RawFd); - impl AsRawFd for Fd { + impl AsRawFd for OwnedFd { #[inline] fn as_raw_fd(&self) -> RawFd { self.0 } } - impl IntoRawFd for Fd { + impl IntoRawFd for OwnedFd { #[inline] fn into_raw_fd(self) -> RawFd { let fd = self.0; @@ -160,14 +90,14 @@ mod fd { } } - impl FromRawFd for Fd { + impl FromRawFd for OwnedFd { #[inline] - unsafe fn from_raw_fd(fd: RawFd) -> Fd { - Fd(fd) + unsafe fn from_raw_fd(fd: RawFd) -> OwnedFd { + OwnedFd(fd) } } - impl Drop for Fd { + impl Drop for OwnedFd { fn drop(&mut self) { unsafe { libc::close(self.0); From 2adc773ad4812ba617cd48c73489105ee0c4e53c Mon Sep 17 00:00:00 2001 From: jtnunley Date: Tue, 23 Aug 2022 13:01:11 -0700 Subject: [PATCH 3/3] fmt --- src/lib.rs | 4 ++-- src/util.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index b2f76a9c..b8bb748f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -25,7 +25,7 @@ pub use cqueue::CompletionQueue; pub use register::Probe; pub use squeue::SubmissionQueue; pub use submit::Submitter; -use util::{OwnedFd, Mmap}; +use util::{Mmap, OwnedFd}; /// IoUring instance pub struct IoUring { @@ -462,4 +462,4 @@ impl AsFd for IoUring { fn as_fd(&self) -> BorrowedFd<'_> { self.fd.as_fd() } -} \ No newline at end of file +} diff --git a/src/util.rs b/src/util.rs index b4e30d2c..a557ccb8 100644 --- a/src/util.rs +++ b/src/util.rs @@ -69,7 +69,7 @@ mod fd { #[cfg(not(feature = "io_safety"))] mod fd { use std::mem; - use std::os::unix::io::{AsRawFd, RawFd, IntoRawFd, FromRawFd}; + use std::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; /// API-compatible with the `OwnedFd` type in the Rust stdlib. pub struct OwnedFd(RawFd);