From c57c5867ae9511f2f2d9ba170f493bbab73a2eee Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Mon, 11 Dec 2023 15:53:16 +0000 Subject: [PATCH 1/4] read: be generic over the type of fd --- tokio-epoll-uring/src/lib.rs | 1 + tokio-epoll-uring/src/ops/read.rs | 30 ++++++--- .../src/system/lifecycle/handle.rs | 8 +-- uring-common/src/io_fd.rs | 62 +++++++++++++++++++ uring-common/src/lib.rs | 2 + 5 files changed, 91 insertions(+), 12 deletions(-) create mode 100644 uring-common/src/io_fd.rs diff --git a/tokio-epoll-uring/src/lib.rs b/tokio-epoll-uring/src/lib.rs index 3154c0a..07ac681 100644 --- a/tokio-epoll-uring/src/lib.rs +++ b/tokio-epoll-uring/src/lib.rs @@ -81,6 +81,7 @@ pub use system::lifecycle::System; pub use system::submission::op_fut::Error as SystemError; pub use uring_common::buf::{IoBuf, IoBufMut}; +pub use uring_common::io_fd::IoFd; pub(crate) mod util; diff --git a/tokio-epoll-uring/src/ops/read.rs b/tokio-epoll-uring/src/ops/read.rs index 29e7515..f24834f 100644 --- a/tokio-epoll-uring/src/ops/read.rs +++ b/tokio-epoll-uring/src/ops/read.rs @@ -1,31 +1,45 @@ -use std::os::fd::{AsRawFd, OwnedFd}; +use std::os::fd::AsRawFd; -use uring_common::{buf::IoBufMut, io_uring}; +use uring_common::{buf::IoBufMut, io_fd::IoFd, io_uring}; use crate::system::submission::op_fut::Op; -pub struct ReadOp +pub struct ReadOp where + F: IoFd + Send, B: IoBufMut + Send, { - pub(crate) file: OwnedFd, + pub(crate) file: F, pub(crate) offset: u64, pub(crate) buf: B, } -impl crate::sealed::Sealed for ReadOp where B: IoBufMut + Send {} +impl crate::sealed::Sealed for ReadOp +where + F: IoFd + Send, + B: IoBufMut + Send, +{ +} -impl Op for ReadOp +impl Op for ReadOp where + F: IoFd + Send, B: IoBufMut + Send, { - type Resources = (OwnedFd, B); + type Resources = (F, B); type Success = usize; type Error = std::io::Error; fn make_sqe(&mut self) -> io_uring::squeue::Entry { io_uring::opcode::Read::new( - io_uring::types::Fd(self.file.as_raw_fd()), + io_uring::types::Fd( + // SAFETY: we hold `AsFd` in self, and if `self` is dropped, hand the fd to the + // `System` to keep it live until the operation completes. + #[allow(unused_unsafe)] + unsafe { + self.file.as_fd().as_raw_fd() + }, + ), self.buf.stable_mut_ptr(), self.buf.bytes_total() as _, ) diff --git a/tokio-epoll-uring/src/system/lifecycle/handle.rs b/tokio-epoll-uring/src/system/lifecycle/handle.rs index 24baa26..fdc269d 100644 --- a/tokio-epoll-uring/src/system/lifecycle/handle.rs +++ b/tokio-epoll-uring/src/system/lifecycle/handle.rs @@ -2,7 +2,7 @@ use futures::FutureExt; use std::{os::fd::OwnedFd, path::Path, task::ready}; -use uring_common::buf::IoBufMut; +use uring_common::{buf::IoBufMut, io_fd::IoFd}; use crate::{ ops::{open_at::OpenAtOp, read::ReadOp}, @@ -104,14 +104,14 @@ impl crate::SystemHandle { let inner = self.inner.as_ref().unwrap(); execute_op(op, inner.submit_side.weak(), None) } - pub fn read( + pub fn read( &self, - file: OwnedFd, + file: F, offset: u64, buf: B, ) -> impl std::future::Future< Output = ( - (OwnedFd, B), + (F, B), Result>, ), > { diff --git a/uring-common/src/io_fd.rs b/uring-common/src/io_fd.rs new file mode 100644 index 0000000..3ff908d --- /dev/null +++ b/uring-common/src/io_fd.rs @@ -0,0 +1,62 @@ +use std::os::fd::{AsRawFd, OwnedFd, RawFd}; + +/// An `io-uring` compatible file file-descriptor-wrapping struct. +/// +/// This trait is implemented by structs that hold ownership of a file descriptor. +/// +/// Think of this as [`crate::buf::IoBuf`], but for file descriptors. +pub trait IoFd: Unpin + 'static { + /// # Safety + /// + /// The implementation must ensure that, while the runtime + /// owns the value, the fd returned by this method + /// 1. remains valid (i.e., is not closed), + /// 2. points to the same kernel resource. + unsafe fn as_fd(&self) -> RawFd; +} + +impl IoFd for OwnedFd { + unsafe fn as_fd(&self) -> RawFd { + // SAFETY: `OwnedFd` is the definition of the requirements in the trait method. + self.as_raw_fd() + } +} + +impl IoFd for std::fs::File { + unsafe fn as_fd(&self) -> RawFd { + // SAFETY: `File` is the definition of the requirements in the trait method. + self.as_raw_fd() + } +} + +pub trait IoFdMut: IoFd { + /// # Safety + /// + /// In addition to the requirements in [`IoFd::as_fd`], + /// the implementation must ensure that the state of the kernel resource + /// referenced by the file descriptor does not change. + /// + /// The reason for this requirement is that the runtime may be + /// using the file descriptor in an operation the modifies the kernel resource, + /// and the higher-level APIs of the runtime may use `&mut`-ness to prevent race conditions. + /// + /// For example, the higher-level API may require `&mut self` for `seek` because `seek` + /// modifies the cursor position of the underlying file kernel resource. + /// This ensures through the type system that it's not possible to `seek` and `read` + /// concurrently. + unsafe fn as_fd_mut(&mut self) -> RawFd; +} + +impl IoFdMut for OwnedFd { + unsafe fn as_fd_mut(&mut self) -> RawFd { + // Safety: If we have a `&mut` ref to an `OwnedFd`, Rust type system guarantees this is the only reference. + self.as_raw_fd() + } +} + +impl IoFdMut for std::fs::File { + unsafe fn as_fd_mut(&mut self) -> RawFd { + // Safety: If we have a `&mut` ref to an `std::fs::File`, Rust type system guarantees this is the only reference. + self.as_raw_fd() + } +} diff --git a/uring-common/src/lib.rs b/uring-common/src/lib.rs index 4925c24..c997965 100644 --- a/uring-common/src/lib.rs +++ b/uring-common/src/lib.rs @@ -2,4 +2,6 @@ pub mod buf; pub mod open_options; pub mod open_options_io_uring_ext; +pub mod io_fd; + pub use io_uring; From eaf3cc957a12ba1079458f0506d314f31717831f Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Mon, 11 Dec 2023 17:09:36 +0000 Subject: [PATCH 2/4] fixup examples --- tokio-epoll-uring/examples/thread_local.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tokio-epoll-uring/examples/thread_local.rs b/tokio-epoll-uring/examples/thread_local.rs index 4b18740..a95fd4d 100644 --- a/tokio-epoll-uring/examples/thread_local.rs +++ b/tokio-epoll-uring/examples/thread_local.rs @@ -19,7 +19,7 @@ async fn main() { let system = tokio_epoll_uring::thread_local_system().await; let buf = vec![0; 2048]; - let ((_file, buf), res) = system.read(file.into(), 512, buf).await; + let ((_file, buf), res) = system.read(file, 512, buf).await; let read = res.unwrap(); assert_eq!(read, 2048, "not expecting short read"); assert_eq!(&buf[0..512], &[23u8; 512]); From a31b2283c30061634f72e0e7e7f95e24500c75c3 Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Tue, 12 Dec 2023 10:02:26 +0000 Subject: [PATCH 3/4] improve comments --- uring-common/src/io_fd.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/uring-common/src/io_fd.rs b/uring-common/src/io_fd.rs index 3ff908d..a785f7c 100644 --- a/uring-common/src/io_fd.rs +++ b/uring-common/src/io_fd.rs @@ -1,6 +1,6 @@ use std::os::fd::{AsRawFd, OwnedFd, RawFd}; -/// An `io-uring` compatible file file-descriptor-wrapping struct. +/// An `io-uring` compatible file descriptor, or wrapper thereof. /// /// This trait is implemented by structs that hold ownership of a file descriptor. /// @@ -29,6 +29,12 @@ impl IoFd for std::fs::File { } } +/// A mutable `io-uring` compatible file descriptor, or wrapper thereof. +/// +/// This trait is implemented by structs that hold ownership of a file descriptor. +/// +/// Think of this as [`crate::buf::IoBufMut`], but for file descriptors. +/// See also [`IoFd`]. pub trait IoFdMut: IoFd { /// # Safety /// From fe30c3e8cf8197bd7655e79906d346e5605d96b6 Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Tue, 12 Dec 2023 10:03:48 +0000 Subject: [PATCH 4/4] improve safety comment --- tokio-epoll-uring/src/ops/read.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tokio-epoll-uring/src/ops/read.rs b/tokio-epoll-uring/src/ops/read.rs index f24834f..1562ac8 100644 --- a/tokio-epoll-uring/src/ops/read.rs +++ b/tokio-epoll-uring/src/ops/read.rs @@ -33,7 +33,7 @@ where fn make_sqe(&mut self) -> io_uring::squeue::Entry { io_uring::opcode::Read::new( io_uring::types::Fd( - // SAFETY: we hold `AsFd` in self, and if `self` is dropped, hand the fd to the + // SAFETY: we hold `F` in self, and if `self` is dropped, we hand the fd to the // `System` to keep it live until the operation completes. #[allow(unused_unsafe)] unsafe {