From b5fb182279e333caca9879ffe7208b70d62eb191 Mon Sep 17 00:00:00 2001 From: Frank Rehwinkel Date: Fri, 3 Feb 2023 10:25:59 -0500 Subject: [PATCH 01/13] Introduce fd variations: raw and fixed --- src/io/shared_fd.rs | 69 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 63 insertions(+), 6 deletions(-) diff --git a/src/io/shared_fd.rs b/src/io/shared_fd.rs index d573265f..22f38e49 100644 --- a/src/io/shared_fd.rs +++ b/src/io/shared_fd.rs @@ -21,7 +21,7 @@ pub(crate) struct SharedFd { struct Inner { // Open file descriptor - fd: RawFd, + fd: Fd, // Waker to notify when the close operation completes. state: RefCell, @@ -45,7 +45,7 @@ impl SharedFd { pub(crate) fn new(fd: RawFd) -> SharedFd { SharedFd { inner: Rc::new(Inner { - fd, + fd: Fd(fd), state: RefCell::new(State::Init), }), } @@ -53,7 +53,7 @@ impl SharedFd { /// Returns the RawFd pub(crate) fn raw_fd(&self) -> RawFd { - self.inner.fd + self.inner.fd.0 } /// An FD cannot be closed until all in-flight operation have completed. @@ -93,15 +93,15 @@ impl Inner { // // TODO: Should we warn? *state = match CONTEXT.try_with(|cx| cx.is_set()) { - Ok(true) => match Op::close(self.fd) { + Ok(true) => match Op::close(self.fd.0) { Ok(op) => State::Closing(op), Err(_) => { - let _ = unsafe { std::fs::File::from_raw_fd(self.fd) }; + let _ = unsafe { std::fs::File::from_raw_fd(self.fd.0) }; State::Closed } }, _ => { - let _ = unsafe { std::fs::File::from_raw_fd(self.fd) }; + let _ = unsafe { std::fs::File::from_raw_fd(self.fd.0) }; State::Closed } }; @@ -156,3 +156,60 @@ impl Drop for Inner { } } } + +// TODO maybe find a better file for this later. + +/// A file descriptor that has not been registered with io_uring. +#[derive(Debug, Clone, Copy)] +#[repr(transparent)] +pub struct Fd(pub RawFd); // TODO consider renaming to RawFd + +/// A file descriptor that has been registered with io_uring using +/// [`Submitter::register_files`](crate::Submitter::register_files) or [`Submitter::register_files_sparse`](crate::Submitter::register_files_sparse). +/// This can reduce overhead compared to using [`Fd`] in some cases. +#[derive(Debug, Clone, Copy)] +#[repr(transparent)] // TODO this probably isn't significant for this crate +pub struct Fixed(pub u32); // TODO consider renaming to Direct (but uring docs use both Fixed descriptor and Direct descriptor) + +pub(crate) mod sealed { + use super::{Fd, Fixed}; + use std::os::unix::io::RawFd; + + #[derive(Debug)] + // Note: io-uring crate names this Target + pub enum CommonFd { + Fd(RawFd), + Fixed(u32), + } + + // Note: io-uring crate names this UseFd + pub trait UseRawFd: Sized { + fn into(self) -> RawFd; + } + + // Note: ioo-uring crate names this UseFixed + pub trait UseCommonFd: Sized { + fn into(self) -> CommonFd; + } + + impl UseRawFd for Fd { + #[inline] + fn into(self) -> RawFd { + self.0 + } + } + + impl UseCommonFd for Fd { + #[inline] + fn into(self) -> CommonFd { + CommonFd::Fd(self.0) + } + } + + impl UseCommonFd for Fixed { + #[inline] + fn into(self) -> CommonFd { + CommonFd::Fixed(self.0) + } + } +} From a57f2ae16a1fcbc10060c0c161cbae75c7f2c767 Mon Sep 17 00:00:00 2001 From: Frank Rehwinkel Date: Fri, 3 Feb 2023 11:48:48 -0500 Subject: [PATCH 02/13] Adds the fixed descriptor capability to SharedFd A step in getting to the support of requests that have fixed descriptors created so that requests can then be written to make use of fixed descriptors. --- src/io/close.rs | 34 ++++++++++----- src/io/shared_fd.rs | 100 +++++++++++++++++++++++++++++++------------- 2 files changed, 93 insertions(+), 41 deletions(-) diff --git a/src/io/close.rs b/src/io/close.rs index b871d83b..6c6d16a6 100644 --- a/src/io/close.rs +++ b/src/io/close.rs @@ -1,24 +1,36 @@ +use crate::io::shared_fd::sealed::CommonFd; use crate::runtime::driver::op; use crate::runtime::driver::op::{Completable, Op}; use crate::runtime::CONTEXT; use std::io; -use std::os::unix::io::RawFd; -pub(crate) struct Close { - fd: RawFd, -} +pub(crate) struct Close {} impl Op { - pub(crate) fn close(fd: RawFd) -> io::Result> { + pub(crate) fn close(fd: CommonFd) -> io::Result> { use io_uring::{opcode, types}; - CONTEXT.with(|x| { - x.handle() - .expect("Not in a runtime context") - .submit_op(Close { fd }, |close| { - opcode::Close::new(types::Fd(close.fd)).build() + match fd { + CommonFd::Raw(raw) => CONTEXT.with(|x| { + x.handle() + .expect("Not in a runtime context") + .submit_op(Close {}, |_close| { + opcode::Close::new(types::Fd(raw)).build() + }) + }), + CommonFd::Fixed(_fixed) => { + /* TODO not yet implemented by io-uring + CONTEXT.with(|x| { + x.handle() + .expect("Not in a runtime context") + .submit_op(Close { fd }, |_close| { + opcode::Close_direct::new(_fixed).build() + }) }) - }) + */ + unreachable!("waiting for the ability to create a fixed descriptor for a request"); + } + } } } diff --git a/src/io/shared_fd.rs b/src/io/shared_fd.rs index 22f38e49..7b34e0ac 100644 --- a/src/io/shared_fd.rs +++ b/src/io/shared_fd.rs @@ -6,6 +6,7 @@ use std::os::unix::io::{FromRawFd, RawFd}; use std::rc::Rc; use std::task::Waker; +use crate::io::shared_fd::sealed::CommonFd; use crate::runtime::driver::op::Op; use crate::runtime::CONTEXT; @@ -21,7 +22,7 @@ pub(crate) struct SharedFd { struct Inner { // Open file descriptor - fd: Fd, + fd: CommonFd, // Waker to notify when the close operation completes. state: RefCell, @@ -45,15 +46,34 @@ impl SharedFd { pub(crate) fn new(fd: RawFd) -> SharedFd { SharedFd { inner: Rc::new(Inner { - fd: Fd(fd), + fd: CommonFd::Raw(fd), + state: RefCell::new(State::Init), + }), + } + } + // TODO once we implement a request that creates a fixed file descriptor, remove this 'allow'. + // It would be possible to create a fixed file using a `register` command to store a raw fd + // into the fixed table, but that's a whole other can of worms - do we track both, can either + // be closed while the other remains active and functional? + #[allow(dead_code)] + pub(crate) fn new_fixed(slot: u32) -> SharedFd { + SharedFd { + inner: Rc::new(Inner { + fd: CommonFd::Fixed(slot), state: RefCell::new(State::Init), }), } } - /// Returns the RawFd + /// Returns the RawFd. pub(crate) fn raw_fd(&self) -> RawFd { - self.inner.fd.0 + self.inner.raw_fd() + } + + /// Returns true if self represents a RawFd. + #[allow(dead_code)] + pub(crate) fn is_raw_fd(&self) -> bool { + self.inner.is_raw_fd() } /// An FD cannot be closed until all in-flight operation have completed. @@ -74,11 +94,35 @@ impl SharedFd { } impl Inner { + /// Returns the RawFd + pub(crate) fn raw_fd(&self) -> RawFd { + //self.inner.fd.0 + match self.fd { + CommonFd::Raw(raw) => raw, + CommonFd::Fixed(_fixed) => { + unreachable!(); // caller could have used is_raw_fd + } + } + } + + /// Returns true if self represents a RawFd. + pub(crate) fn is_raw_fd(&self) -> bool { + match self.fd { + CommonFd::Raw(_) => true, + CommonFd::Fixed(_) => false, + } + } /// If there are no in-flight operations, submit the operation. fn submit_close_op(&mut self) { - // Close the FD + // Close the file + let common_fd = self.fd; let state = RefCell::get_mut(&mut self.state); + match *state { + State::Closing(_) | State::Closed => return, + _ => {} + }; + // Submit a close operation // If either: // - runtime has already closed, or @@ -92,19 +136,17 @@ impl Inner { // dropping it. // // TODO: Should we warn? - *state = match CONTEXT.try_with(|cx| cx.is_set()) { - Ok(true) => match Op::close(self.fd.0) { - Ok(op) => State::Closing(op), - Err(_) => { - let _ = unsafe { std::fs::File::from_raw_fd(self.fd.0) }; - State::Closed - } - }, - _ => { - let _ = unsafe { std::fs::File::from_raw_fd(self.fd.0) }; - State::Closed + if let Ok(true) = CONTEXT.try_with(|cx| cx.is_set()) { + if let Ok(op) = Op::close(common_fd) { + *state = State::Closing(op); + return; } }; + + if let CommonFd::Raw(raw) = common_fd { + let _ = unsafe { std::fs::File::from_raw_fd(raw) }; + } + *state = State::Closed; } /// Completes when the FD has been closed. @@ -133,7 +175,7 @@ impl Inner { Poll::Pending } State::Closing(op) => { - // Nothing to do if the close opeation failed. + // Nothing to do if the close operation failed. let _ = ready!(Pin::new(op).poll(cx)); *state = State::Closed; Poll::Ready(()) @@ -157,52 +199,50 @@ impl Drop for Inner { } } -// TODO maybe find a better file for this later. +// Enum and traits copied from the io-uring crate. /// A file descriptor that has not been registered with io_uring. #[derive(Debug, Clone, Copy)] -#[repr(transparent)] -pub struct Fd(pub RawFd); // TODO consider renaming to RawFd +pub struct Raw(pub RawFd); // Note: io-uring names this Fd /// A file descriptor that has been registered with io_uring using /// [`Submitter::register_files`](crate::Submitter::register_files) or [`Submitter::register_files_sparse`](crate::Submitter::register_files_sparse). /// This can reduce overhead compared to using [`Fd`] in some cases. #[derive(Debug, Clone, Copy)] -#[repr(transparent)] // TODO this probably isn't significant for this crate pub struct Fixed(pub u32); // TODO consider renaming to Direct (but uring docs use both Fixed descriptor and Direct descriptor) pub(crate) mod sealed { - use super::{Fd, Fixed}; + use super::{Fixed, Raw}; use std::os::unix::io::RawFd; - #[derive(Debug)] - // Note: io-uring crate names this Target + #[derive(Debug, Clone, Copy)] + // Note: io-uring names this Target pub enum CommonFd { - Fd(RawFd), + Raw(RawFd), Fixed(u32), } - // Note: io-uring crate names this UseFd + // Note: io-uring names this UseFd pub trait UseRawFd: Sized { fn into(self) -> RawFd; } - // Note: ioo-uring crate names this UseFixed + // Note: io-uring names this UseFixed pub trait UseCommonFd: Sized { fn into(self) -> CommonFd; } - impl UseRawFd for Fd { + impl UseRawFd for Raw { #[inline] fn into(self) -> RawFd { self.0 } } - impl UseCommonFd for Fd { + impl UseCommonFd for Raw { #[inline] fn into(self) -> CommonFd { - CommonFd::Fd(self.0) + CommonFd::Raw(self.0) } } From 24b274922386ff660ebe8250927cc7128948c683 Mon Sep 17 00:00:00 2001 From: Frank Rehwinkel Date: Fri, 10 Feb 2023 12:25:39 -0500 Subject: [PATCH 03/13] async close fixed support --- src/io/close.rs | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/src/io/close.rs b/src/io/close.rs index 6c6d16a6..c350e287 100644 --- a/src/io/close.rs +++ b/src/io/close.rs @@ -10,27 +10,29 @@ impl Op { pub(crate) fn close(fd: CommonFd) -> io::Result> { use io_uring::{opcode, types}; - match fd { - CommonFd::Raw(raw) => CONTEXT.with(|x| { - x.handle() - .expect("Not in a runtime context") - .submit_op(Close {}, |_close| { - opcode::Close::new(types::Fd(raw)).build() - }) - }), - CommonFd::Fixed(_fixed) => { - /* TODO not yet implemented by io-uring - CONTEXT.with(|x| { - x.handle() - .expect("Not in a runtime context") - .submit_op(Close { fd }, |_close| { - opcode::Close_direct::new(_fixed).build() - }) - }) - */ - unreachable!("waiting for the ability to create a fixed descriptor for a request"); + let (raw, fixed) = match fd { + CommonFd::Raw(raw) => (raw, None), + CommonFd::Fixed(fixed) => { + let fixed = match types::DestinationSlot::try_from_slot_target(fixed) { + Ok(n) => n, + Err(n) => { + return Err(std::io::Error::new( + std::io::ErrorKind::Other, + format!("invalid fixed slot: {n}"), + )) + } + }; + (0, Some(fixed)) } - } + }; + + CONTEXT.with(|x| { + x.handle() + .expect("Not in a runtime context") + .submit_op(Close {}, |_close| { + opcode::Close::new(types::Fd(raw)).file_index(fixed).build() + }) + }) } } From 1baf98754b1621c2a729ac41147afd653dd40152 Mon Sep 17 00:00:00 2001 From: Frank Rehwinkel Date: Fri, 10 Feb 2023 12:45:15 -0500 Subject: [PATCH 04/13] File::read fixed support --- src/io/read.rs | 14 +++++++++++--- src/io/shared_fd.rs | 3 ++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/io/read.rs b/src/io/read.rs index c3395b40..b1fdfe3b 100644 --- a/src/io/read.rs +++ b/src/io/read.rs @@ -2,6 +2,7 @@ use crate::buf::BoundedBufMut; use crate::io::SharedFd; use crate::BufResult; +use crate::io::shared_fd::sealed::CommonFd; use crate::runtime::driver::op::{Completable, CqeResult, Op}; use crate::runtime::CONTEXT; use std::io; @@ -30,9 +31,16 @@ impl Op> { // Get raw buffer info let ptr = read.buf.stable_mut_ptr(); let len = read.buf.bytes_total(); - opcode::Read::new(types::Fd(fd.raw_fd()), ptr, len as _) - .offset(offset as _) - .build() + match read.fd.common_fd() { + CommonFd::Raw(raw) => opcode::Read::new(types::Fd(raw), ptr, len as _) + .offset(offset as _) + .build(), + CommonFd::Fixed(fixed) => { + opcode::Read::new(types::Fixed(fixed), ptr, len as _) + .offset(offset as _) + .build() + } + } }, ) }) diff --git a/src/io/shared_fd.rs b/src/io/shared_fd.rs index 0253c287..f1cc4d93 100644 --- a/src/io/shared_fd.rs +++ b/src/io/shared_fd.rs @@ -81,7 +81,8 @@ impl SharedFd { match self.inner.fd { CommonFd::Raw(raw) => raw, CommonFd::Fixed(_fixed) => { - unreachable!("fixed files aren't actually created yet"); + // TODO Introduce the fixed option for file read and write first. + unreachable!("fixed file support not yet added for this call stack"); } } } From c9e9cebe2928b1a5829bc3d5f2f760719285247f Mon Sep 17 00:00:00 2001 From: Frank Rehwinkel Date: Fri, 10 Feb 2023 13:03:35 -0500 Subject: [PATCH 05/13] File::write fixed support --- src/io/read.rs | 12 ++++++++---- src/io/write.rs | 18 +++++++++++++++--- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/io/read.rs b/src/io/read.rs index b1fdfe3b..01ac42d7 100644 --- a/src/io/read.rs +++ b/src/io/read.rs @@ -32,11 +32,15 @@ impl Op> { let ptr = read.buf.stable_mut_ptr(); let len = read.buf.bytes_total(); match read.fd.common_fd() { - CommonFd::Raw(raw) => opcode::Read::new(types::Fd(raw), ptr, len as _) - .offset(offset as _) - .build(), + CommonFd::Raw(raw) => { + let fd = types::Fd(raw); + opcode::Read::new(fd, ptr, len as _) + .offset(offset as _) + .build() + } CommonFd::Fixed(fixed) => { - opcode::Read::new(types::Fixed(fixed), ptr, len as _) + let fd = types::Fixed(fixed); + opcode::Read::new(fd, ptr, len as _) .offset(offset as _) .build() } diff --git a/src/io/write.rs b/src/io/write.rs index 9775f4fe..f26b3174 100644 --- a/src/io/write.rs +++ b/src/io/write.rs @@ -1,3 +1,4 @@ +use crate::io::shared_fd::sealed::CommonFd; use crate::runtime::driver::op::{Completable, CqeResult, Op}; use crate::runtime::CONTEXT; use crate::{buf::BoundedBuf, io::SharedFd, BufResult}; @@ -27,9 +28,20 @@ impl Op> { let ptr = write.buf.stable_ptr(); let len = write.buf.bytes_init(); - opcode::Write::new(types::Fd(fd.raw_fd()), ptr, len as _) - .offset(offset as _) - .build() + match write.fd.common_fd() { + CommonFd::Raw(raw) => { + let fd = types::Fd(raw); + opcode::Write::new(fd, ptr, len as _) + .offset(offset as _) + .build() + } + CommonFd::Fixed(fixed) => { + let fd = types::Fixed(fixed); + opcode::Write::new(fd, ptr, len as _) + .offset(offset as _) + .build() + } + } }, ) }) From ff699e2d0d067e7089f966dd1e8a3f2884a45825 Mon Sep 17 00:00:00 2001 From: Frank Rehwinkel Date: Sat, 11 Feb 2023 07:56:17 -0500 Subject: [PATCH 06/13] support dropping fixed file table descriptors --- src/io/shared_fd.rs | 36 ++++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/src/io/shared_fd.rs b/src/io/shared_fd.rs index f1cc4d93..e5204ac3 100644 --- a/src/io/shared_fd.rs +++ b/src/io/shared_fd.rs @@ -10,6 +10,7 @@ use std::{ use crate::io::shared_fd::sealed::CommonFd; use crate::runtime::driver::op::Op; +use crate::runtime::CONTEXT; // Tracks in-flight operations on a file descriptor. Ensures all in-flight // operations complete before submitting the close. @@ -213,8 +214,10 @@ impl Drop for SharedFd { impl Drop for Inner { fn drop(&mut self) { - // If the inner state isn't `Closed`, the user hasn't called close().await - // so do it synchronously. + // If the inner state isn't `Closed`, the user hasn't called close().await so close it now. + // At least for the case of a regular file descriptor we can do it synchronously. For the + // case of a fixed file table descriptor, we may already be out of the driver's context, + // but if we aren't we resort to the io_uring close operation - and spawn a task to do it. let state = self.state.borrow_mut(); @@ -222,14 +225,35 @@ impl Drop for Inner { return; } - // Perform one form of synchronous close or the other. + // Perform one form of close or the other. match self.fd { CommonFd::Raw(raw) => { let _ = unsafe { std::fs::File::from_raw_fd(raw) }; } - CommonFd::Fixed(_fixed) => { - // TODO replace with test for context and then use synchronous close - unreachable!(); + + CommonFd::Fixed(fixed) => { + // As there is no synchronous close for a fixed file table slot, we have to resort + // to the async close provided by the io_uring device. If we knew the fixed file + // table had been unregistered, this wouldn't be necessary either. + + match CONTEXT.try_with(|cx| cx.is_set()) { + Ok(true) => {} + // If the driver is gone, nothing to do. The fixed table has already been taken + // down by the device anyway. + _ => return, + } + + crate::spawn(async move { + if let Ok(true) = CONTEXT.try_with(|cx| cx.is_set()) { + let fd = CommonFd::Fixed(fixed); + if let Ok(op) = Op::close(fd) { + let _ = op.await; + } + // Else, should warn or panic if the Op::Close can't be built? It would + // mean the fixed value was out of reach which would not be expected at + // this point. + } + }); } } } From 9cbca8e281210e9caeca4c6fed952c40e8f36b98 Mon Sep 17 00:00:00 2001 From: Frank Rehwinkel Date: Sat, 11 Feb 2023 07:56:49 -0500 Subject: [PATCH 07/13] general comment cleanup --- src/io/shared_fd.rs | 55 +++++++++------------------------------------ 1 file changed, 11 insertions(+), 44 deletions(-) diff --git a/src/io/shared_fd.rs b/src/io/shared_fd.rs index e5204ac3..6ba96f59 100644 --- a/src/io/shared_fd.rs +++ b/src/io/shared_fd.rs @@ -50,13 +50,6 @@ impl SharedFd { Self::_new(CommonFd::Raw(fd)) } - // TODO once we implement a request that creates a fixed file descriptor, remove this 'allow'. - // It would be possible to create a fixed file using a `register` command to store a raw fd - // into the fixed table, but that's a whole other can of worms - do we track both, can either - // be closed while the other remains active and functional? - // So as a first step, we will likely be creating fixed file versions from fixed file opens. - // Further down the line, from fixed file multi-accept. - #[allow(dead_code)] pub(crate) fn new_fixed(slot: u32) -> SharedFd { Self::_new(CommonFd::Fixed(slot)) } @@ -77,25 +70,17 @@ impl SharedFd { */ /// Returns the RawFd. pub(crate) fn raw_fd(&self) -> RawFd { - // TODO remove self.inner.raw_fd() - match self.inner.fd { CommonFd::Raw(raw) => raw, CommonFd::Fixed(_fixed) => { - // TODO Introduce the fixed option for file read and write first. + // TODO remove this function completely once all the uring opcodes that accept + // a fixed file table slot have been modified. For now, we have to keep it to avoid + // too many file changes all at once. unreachable!("fixed file support not yet added for this call stack"); } } } - /* - * TODO remove this, it doesn't seem appropriate any longer. - /// Returns true if self represents a RawFd. - #[allow(dead_code)] - pub(crate) fn is_raw_fd(&self) -> bool { - self.inner.is_raw_fd() - } - */ // Returns the common fd, either a RawFd or the fixed fd slot number. #[allow(dead_code)] pub(crate) fn common_fd(&self) -> CommonFd { @@ -151,29 +136,6 @@ impl SharedFd { } impl Inner { - /* TODO remove - // Returns the RawFd but panics if called on a fixed fd. - #[allow(dead_code)] - pub(crate) fn raw_fd(&self) -> Option { - //self.inner.fd.0 - match self.fd { - CommonFd::Raw(raw) => Some(raw), - CommonFd::Fixed(_fixed) => None, - } - } - */ - - /* TODO remove - // Returns true if self represents a RawFd. - // Should be used before callinng - pub(crate) fn is_raw_fd(&self) -> bool { - match self.fd { - CommonFd::Raw(_) => true, - CommonFd::Fixed(_) => false, - } - } - */ - async fn async_close_op(&mut self) -> io::Result<()> { // &mut self implies there are no outstanding operations. // If state already closed, the user closed multiple times; simply return Ok. @@ -261,21 +223,24 @@ impl Drop for Inner { // Enum and traits copied from the io-uring crate. +/* + * TODO maybe these types aren't needed at all. /// A file descriptor that has not been registered with io_uring. #[derive(Debug, Clone, Copy)] -pub struct Raw(pub RawFd); // Note: io-uring names this Fd +pub(crate) struct Raw(pub RawFd); // Note: io-uring names this Fd /// A file descriptor that has been registered with io_uring using /// [`Submitter::register_files`](crate::Submitter::register_files) or [`Submitter::register_files_sparse`](crate::Submitter::register_files_sparse). /// This can reduce overhead compared to using [`Fd`] in some cases. #[derive(Debug, Clone, Copy)] -pub struct Fixed(pub u32); // TODO consider renaming to Direct (but uring docs use both Fixed descriptor and Direct descriptor) +pub(crate) struct Fixed(pub u32); // TODO consider renaming to Direct (but uring docs use both Fixed descriptor and Direct descriptor) + */ // TODO definitely not sure this should be sealed. But leaving it for now. Could easily decide // there is nothing here to seal as our API is fluid for a while yet. pub(crate) mod sealed { - use super::{Fixed, Raw}; + // use super::{Fixed, Raw}; use std::os::unix::io::RawFd; #[derive(Debug, Clone, Copy)] @@ -295,6 +260,7 @@ pub(crate) mod sealed { fn into(self) -> CommonFd; } + /* TODO maybe won't be needed at all. impl UseRawFd for Raw { #[inline] fn into(self) -> RawFd { @@ -315,4 +281,5 @@ pub(crate) mod sealed { CommonFd::Fixed(self.0) } } + */ } From 297477e783219ed9d1feee3e90dcec974956682e Mon Sep 17 00:00:00 2001 From: Frank Rehwinkel Date: Sat, 11 Feb 2023 23:26:04 -0500 Subject: [PATCH 08/13] comment idea for fixed slot drop --- src/io/shared_fd.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/io/shared_fd.rs b/src/io/shared_fd.rs index 6ba96f59..4a6fafd0 100644 --- a/src/io/shared_fd.rs +++ b/src/io/shared_fd.rs @@ -205,6 +205,11 @@ impl Drop for Inner { _ => return, } + // TODO Investigate the idea from the liburing team of replacing the one slot with + // a -1 by using the register/files_update synchronous command. If the current + // scheme that uses a spawn is initiallly acceptable, probably leave it like this + // for now and wait to be able to benchmark once we have streaming tcp sockets. + crate::spawn(async move { if let Ok(true) = CONTEXT.try_with(|cx| cx.is_set()) { let fd = CommonFd::Fixed(fixed); From 3debd20095a6a0c42a4e957c03c87e5f0a43ebad Mon Sep 17 00:00:00 2001 From: Frank Rehwinkel Date: Tue, 14 Feb 2023 06:58:18 -0500 Subject: [PATCH 09/13] opcode::Close uses Fixed(fd) now --- src/io/close.rs | 37 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/src/io/close.rs b/src/io/close.rs index c350e287..7dc28625 100644 --- a/src/io/close.rs +++ b/src/io/close.rs @@ -10,29 +10,24 @@ impl Op { pub(crate) fn close(fd: CommonFd) -> io::Result> { use io_uring::{opcode, types}; - let (raw, fixed) = match fd { - CommonFd::Raw(raw) => (raw, None), - CommonFd::Fixed(fixed) => { - let fixed = match types::DestinationSlot::try_from_slot_target(fixed) { - Ok(n) => n, - Err(n) => { - return Err(std::io::Error::new( - std::io::ErrorKind::Other, - format!("invalid fixed slot: {n}"), - )) - } - }; - (0, Some(fixed)) + match fd { + CommonFd::Raw(raw) => { + let fd = types::Fd(raw); + CONTEXT.with(|x| { + x.handle() + .expect("Not in a runtime context") + .submit_op(Close {}, |_close| opcode::Close::new(fd).build()) + }) } - }; - - CONTEXT.with(|x| { - x.handle() - .expect("Not in a runtime context") - .submit_op(Close {}, |_close| { - opcode::Close::new(types::Fd(raw)).file_index(fixed).build() + CommonFd::Fixed(fixed) => { + let fd = types::Fixed(fixed); + CONTEXT.with(|x| { + x.handle() + .expect("Not in a runtime context") + .submit_op(Close {}, |_close| opcode::Close::new(fd).build()) }) - }) + } + } } } From b81fce01c66997be252646c109787af675df57ac Mon Sep 17 00:00:00 2001 From: Frank Rehwinkel Date: Mon, 20 Feb 2023 07:14:42 -0500 Subject: [PATCH 10/13] bump io-uring dependency to 0.5.13 --- Cargo.toml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1afa97d3..8812537f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,8 +20,7 @@ keywords = ["async", "fs", "io-uring"] tokio = { version = "1.2", features = ["net", "rt", "sync"] } slab = "0.4.2" libc = "0.2.80" -#io-uring = { version = "0.5.12", features = ["unstable"] } -io-uring = { git = "https://github.com/frankreh/io-uring", branch = "frankreh/add-fixed-file-to-opcode-for-open-opcodes" } +io-uring = "0.5.13" socket2 = { version = "0.4.4", features = ["all"] } bytes = { version = "1.0", optional = true } futures = "0.3.26" From c80ae285ba7e9559d5ea86f90c35f71687cb9994 Mon Sep 17 00:00:00 2001 From: Frank Rehwinkel Date: Mon, 20 Feb 2023 07:26:27 -0500 Subject: [PATCH 11/13] remove allow(dead_code) --- src/io/shared_fd.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/io/shared_fd.rs b/src/io/shared_fd.rs index 4a6fafd0..358e3f9f 100644 --- a/src/io/shared_fd.rs +++ b/src/io/shared_fd.rs @@ -82,7 +82,6 @@ impl SharedFd { } // Returns the common fd, either a RawFd or the fixed fd slot number. - #[allow(dead_code)] pub(crate) fn common_fd(&self) -> CommonFd { self.inner.fd } From 3de20686cf67421c1ccd16ee3834f8d98d9527f8 Mon Sep 17 00:00:00 2001 From: Frank Rehwinkel Date: Mon, 20 Feb 2023 07:30:42 -0500 Subject: [PATCH 12/13] remove old commented-out code --- src/io/shared_fd.rs | 36 ------------------------------------ 1 file changed, 36 deletions(-) diff --git a/src/io/shared_fd.rs b/src/io/shared_fd.rs index 358e3f9f..57fa5a1b 100644 --- a/src/io/shared_fd.rs +++ b/src/io/shared_fd.rs @@ -227,19 +227,6 @@ impl Drop for Inner { // Enum and traits copied from the io-uring crate. -/* - * TODO maybe these types aren't needed at all. -/// A file descriptor that has not been registered with io_uring. -#[derive(Debug, Clone, Copy)] -pub(crate) struct Raw(pub RawFd); // Note: io-uring names this Fd - -/// A file descriptor that has been registered with io_uring using -/// [`Submitter::register_files`](crate::Submitter::register_files) or [`Submitter::register_files_sparse`](crate::Submitter::register_files_sparse). -/// This can reduce overhead compared to using [`Fd`] in some cases. -#[derive(Debug, Clone, Copy)] -pub(crate) struct Fixed(pub u32); // TODO consider renaming to Direct (but uring docs use both Fixed descriptor and Direct descriptor) - */ - // TODO definitely not sure this should be sealed. But leaving it for now. Could easily decide // there is nothing here to seal as our API is fluid for a while yet. @@ -263,27 +250,4 @@ pub(crate) mod sealed { pub(crate) trait UseCommonFd: Sized { fn into(self) -> CommonFd; } - - /* TODO maybe won't be needed at all. - impl UseRawFd for Raw { - #[inline] - fn into(self) -> RawFd { - self.0 - } - } - - impl UseCommonFd for Raw { - #[inline] - fn into(self) -> CommonFd { - CommonFd::Raw(self.0) - } - } - - impl UseCommonFd for Fixed { - #[inline] - fn into(self) -> CommonFd { - CommonFd::Fixed(self.0) - } - } - */ } From 2390971359e539902a5aa33362ce69745971411c Mon Sep 17 00:00:00 2001 From: Frank Rehwinkel Date: Mon, 20 Feb 2023 07:38:18 -0500 Subject: [PATCH 13/13] simplify shared_fd::CommonFd --- src/io/close.rs | 2 +- src/io/read.rs | 2 +- src/io/shared_fd.rs | 34 ++++++---------------------------- src/io/write.rs | 2 +- 4 files changed, 9 insertions(+), 31 deletions(-) diff --git a/src/io/close.rs b/src/io/close.rs index 7dc28625..3c42bc5f 100644 --- a/src/io/close.rs +++ b/src/io/close.rs @@ -1,4 +1,4 @@ -use crate::io::shared_fd::sealed::CommonFd; +use crate::io::shared_fd::CommonFd; use crate::runtime::driver::op; use crate::runtime::driver::op::{Completable, Op}; use crate::runtime::CONTEXT; diff --git a/src/io/read.rs b/src/io/read.rs index 01ac42d7..8a9b9b35 100644 --- a/src/io/read.rs +++ b/src/io/read.rs @@ -2,7 +2,7 @@ use crate::buf::BoundedBufMut; use crate::io::SharedFd; use crate::BufResult; -use crate::io::shared_fd::sealed::CommonFd; +use crate::io::shared_fd::CommonFd; use crate::runtime::driver::op::{Completable, CqeResult, Op}; use crate::runtime::CONTEXT; use std::io; diff --git a/src/io/shared_fd.rs b/src/io/shared_fd.rs index 57fa5a1b..9fbf23b2 100644 --- a/src/io/shared_fd.rs +++ b/src/io/shared_fd.rs @@ -8,7 +8,6 @@ use std::{ task::Waker, }; -use crate::io::shared_fd::sealed::CommonFd; use crate::runtime::driver::op::Op; use crate::runtime::CONTEXT; @@ -25,6 +24,12 @@ pub(crate) struct SharedFd { inner: Rc, } +#[derive(Debug, Clone, Copy)] +pub(crate) enum CommonFd { + Raw(RawFd), + Fixed(u32), +} + struct Inner { // Open file descriptor fd: CommonFd, @@ -224,30 +229,3 @@ impl Drop for Inner { } } } - -// Enum and traits copied from the io-uring crate. - -// TODO definitely not sure this should be sealed. But leaving it for now. Could easily decide -// there is nothing here to seal as our API is fluid for a while yet. - -pub(crate) mod sealed { - // use super::{Fixed, Raw}; - use std::os::unix::io::RawFd; - - #[derive(Debug, Clone, Copy)] - // Note: io-uring names this Target - pub(crate) enum CommonFd { - Raw(RawFd), - Fixed(u32), - } - - // Note: io-uring names this UseFd - pub(crate) trait UseRawFd: Sized { - fn into(self) -> RawFd; - } - - // Note: io-uring names this UseFixed - pub(crate) trait UseCommonFd: Sized { - fn into(self) -> CommonFd; - } -} diff --git a/src/io/write.rs b/src/io/write.rs index f26b3174..0606bc33 100644 --- a/src/io/write.rs +++ b/src/io/write.rs @@ -1,4 +1,4 @@ -use crate::io::shared_fd::sealed::CommonFd; +use crate::io::shared_fd::CommonFd; use crate::runtime::driver::op::{Completable, CqeResult, Op}; use crate::runtime::CONTEXT; use crate::{buf::BoundedBuf, io::SharedFd, BufResult};