Merge pull request #1084 from cagatay-y/copyless-send-command

fuse: reduce copying and handle errors
hermit-os · Mar 13, 2024 · d118729 · d118729
2 parents 1e26482 + 21bb203
commit d118729
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 49 deletions.
diff --git a/src/drivers/fs/virtio_fs.rs b/src/drivers/fs/virtio_fs.rs
@@ -13,10 +13,9 @@ use crate::drivers::virtio::error::VirtioFsError;
 use crate::drivers::virtio::transport::mmio::{ComCfg, IsrStatus, NotifCfg};
 #[cfg(feature = "pci")]
 use crate::drivers::virtio::transport::pci::{ComCfg, IsrStatus, NotifCfg};
+use crate::drivers::virtio::virtqueue::error::VirtqError;
 use crate::drivers::virtio::virtqueue::split::SplitVq;
-use crate::drivers::virtio::virtqueue::{
-	AsSliceU8, BuffSpec, BufferToken, Bytes, Virtq, VqIndex, VqSize,
-};
+use crate::drivers::virtio::virtqueue::{AsSliceU8, BuffSpec, Bytes, Virtq, VqIndex, VqSize};
 use crate::fs::fuse::{self, FuseInterface};
 
 /// A wrapper struct for the raw configuration structure.
@@ -39,7 +38,6 @@ pub(crate) struct VirtioFsDriver {
 	pub(super) isr_stat: IsrStatus,
 	pub(super) notif_cfg: NotifCfg,
 	pub(super) vqueues: Vec<Rc<dyn Virtq>>,
-	pub(super) ready_queue: Vec<BufferToken>,
 	pub(super) irq: InterruptLine,
 }
 
@@ -138,36 +136,34 @@ impl VirtioFsDriver {
 			self.vqueues.push(Rc::new(vq));
 		}
 
-		let cmd_spec = Some(BuffSpec::Single(Bytes::new(64 * 1024 + 128).unwrap()));
-		let rsp_spec = Some(BuffSpec::Single(Bytes::new(64 * 1024 + 128).unwrap()));
-
-		if let Ok(buff_tkn) = self.vqueues[1].clone().prep_buffer(cmd_spec, rsp_spec) {
-			self.ready_queue.push(buff_tkn);
-			// At this point the device is "live"
-			self.com_cfg.drv_ok();
-		}
+		// At this point the device is "live"
+		self.com_cfg.drv_ok();
 
 		Ok(())
 	}
 }
 
 impl FuseInterface for VirtioFsDriver {
-	fn send_command<O: fuse::ops::Op>(&mut self, cmd: &fuse::Cmd<O>, rsp: &mut fuse::Rsp<O>) {
-		if let Some(mut buff_tkn) = self.ready_queue.pop() {
-			let cmd_len = Some(cmd.len());
-			let rsp_len = Some(rsp.len());
-			buff_tkn.restr_size(cmd_len, rsp_len).unwrap();
-
-			let transfer_tkn = buff_tkn.write(Some(cmd), None::<&fuse::Rsp<O>>).unwrap();
-			let transfer = transfer_tkn.dispatch_blocking().unwrap();
-			let (_, response) = transfer.ret_cpy().unwrap();
-			let tkn = transfer.reuse().unwrap();
-			self.ready_queue.push(tkn);
-
-			if let Some(response) = response {
-				rsp.as_slice_u8_mut()[..response.len()].copy_from_slice(response.as_ref());
-			}
-		}
+	fn send_command<O: fuse::ops::Op>(
+		&mut self,
+		cmd: &fuse::Cmd<O>,
+		rsp: &mut fuse::Rsp<O>,
+	) -> Result<(), VirtqError> {
+		let send = (
+			cmd.as_slice_u8(),
+			BuffSpec::Single(Bytes::new(cmd.len()).ok_or(VirtqError::BufferToLarge)?),
+		);
+		let rsp_len = rsp.len();
+		let recv = (
+			rsp.as_slice_u8_mut(),
+			BuffSpec::Single(Bytes::new(rsp_len).ok_or(VirtqError::BufferToLarge)?),
+		);
+		let transfer_tkn = self.vqueues[1]
+			.clone()
+			.prep_transfer_from_raw(Some(send), Some(recv))
+			.unwrap();
+		transfer_tkn.dispatch_blocking()?;
+		Ok(())
 	}
 
 	fn get_mount_point(&self) -> String {

diff --git a/src/drivers/fs/virtio_pci.rs b/src/drivers/fs/virtio_pci.rs
@@ -112,7 +112,6 @@ impl VirtioFsDriver {
 			isr_stat,
 			notif_cfg,
 			vqueues: Vec::new(),
-			ready_queue: Vec::new(),
 			irq: device.get_irq().unwrap(),
 		})
 	}

diff --git a/src/drivers/virtio/virtqueue/mod.rs b/src/drivers/virtio/virtqueue/mod.rs
@@ -3307,6 +3307,7 @@ impl From<DescrFlags> for u16 {
 /// virtqueue implementation, realized via the different enum variants.
 pub mod error {
 	use super::Transfer;
+	use crate::fd;
 
 	#[derive(Debug)]
 	// Internal Error Handling for Buffers
@@ -3387,4 +3388,10 @@ pub mod error {
             }
 		}
 	}
+
+	impl core::convert::From<VirtqError> for fd::IoError {
+		fn from(_: VirtqError) -> Self {
+			fd::IoError::EIO
+		}
+	}
 }
diff --git a/src/fs/fuse.rs b/src/fs/fuse.rs
@@ -18,6 +18,7 @@ use crate::alloc::string::ToString;
 use crate::arch::kernel::mmio::get_filesystem_driver;
 #[cfg(feature = "pci")]
 use crate::drivers::pci::get_filesystem_driver;
+use crate::drivers::virtio::virtqueue::error::VirtqError;
 use crate::drivers::virtio::virtqueue::AsSliceU8;
 use crate::executor::block_on;
 use crate::fd::{IoError, PollEvent};
@@ -39,7 +40,11 @@ const S_IFLNK: u32 = 40960;
 const S_IFMT: u32 = 61440;
 
 pub(crate) trait FuseInterface {
-	fn send_command<O: ops::Op>(&mut self, cmd: &Cmd<O>, rsp: &mut Rsp<O>);
+	fn send_command<O: ops::Op>(
+		&mut self,
+		cmd: &Cmd<O>,
+		rsp: &mut Rsp<O>,
+	) -> Result<(), VirtqError>;
 
 	fn get_mount_point(&self) -> String;
 }
@@ -603,7 +608,8 @@ fn lookup(name: &str) -> Option<u64> {
 	get_filesystem_driver()
 		.unwrap()
 		.lock()
-		.send_command(cmd.as_ref(), rsp.as_mut());
+		.send_command(cmd.as_ref(), rsp.as_mut())
+		.ok()?;
 	if unsafe { rsp.out_header.assume_init_ref().error } == 0 {
 		Some(unsafe { rsp.op_header.assume_init_ref().nodeid })
 	} else {
@@ -617,7 +623,7 @@ fn readlink(nid: u64) -> Result<String, IoError> {
 	get_filesystem_driver()
 		.unwrap()
 		.lock()
-		.send_command(cmd.as_ref(), rsp.as_mut());
+		.send_command(cmd.as_ref(), rsp.as_mut())?;
 	let len: usize = if unsafe { rsp.out_header.assume_init_ref().len } as usize
 		- ::core::mem::size_of::<fuse_abi::OutHeader>()
 		- ::core::mem::size_of::<fuse_abi::ReadlinkOut>()
@@ -662,7 +668,7 @@ impl FuseFileHandleInner {
 				get_filesystem_driver()
 					.ok_or(IoError::ENOSYS)?
 					.lock()
-					.send_command(cmd.as_ref(), rsp.as_mut());
+					.send_command(cmd.as_ref(), rsp.as_mut())?;
 
 				if unsafe { rsp.out_header.assume_init_ref().error } < 0 {
 					Poll::Ready(Err(IoError::EIO))
@@ -702,7 +708,7 @@ impl FuseFileHandleInner {
 			get_filesystem_driver()
 				.ok_or(IoError::ENOSYS)?
 				.lock()
-				.send_command(cmd.as_ref(), rsp.as_mut());
+				.send_command(cmd.as_ref(), rsp.as_mut())?;
 			let len: usize = if (unsafe { rsp.out_header.assume_init_ref().len } as usize)
 				- ::core::mem::size_of::<fuse_abi::OutHeader>()
 				- ::core::mem::size_of::<fuse_abi::ReadOut>()
@@ -743,7 +749,7 @@ impl FuseFileHandleInner {
 			get_filesystem_driver()
 				.ok_or(IoError::ENOSYS)?
 				.lock()
-				.send_command(cmd.as_ref(), rsp.as_mut());
+				.send_command(cmd.as_ref(), rsp.as_mut())?;
 
 			if unsafe { rsp.out_header.assume_init_ref().error } < 0 {
 				return Err(IoError::EIO);
@@ -771,7 +777,7 @@ impl FuseFileHandleInner {
 			get_filesystem_driver()
 				.ok_or(IoError::ENOSYS)?
 				.lock()
-				.send_command(cmd.as_ref(), rsp.as_mut());
+				.send_command(cmd.as_ref(), rsp.as_mut())?;
 
 			if unsafe { rsp.out_header.assume_init_ref().error } < 0 {
 				return Err(IoError::EIO);
@@ -794,7 +800,8 @@ impl Drop for FuseFileHandleInner {
 			get_filesystem_driver()
 				.unwrap()
 				.lock()
-				.send_command(cmd.as_ref(), rsp.as_mut());
+				.send_command(cmd.as_ref(), rsp.as_mut())
+				.unwrap();
 		}
 	}
 }
@@ -871,7 +878,7 @@ impl VfsNode for FuseDirectory {
 		get_filesystem_driver()
 			.ok_or(IoError::ENOSYS)?
 			.lock()
-			.send_command(cmd.as_ref(), rsp.as_mut());
+			.send_command(cmd.as_ref(), rsp.as_mut())?;
 		let fuse_fh = unsafe { rsp.op_header.assume_init_ref().fh };
 
 		debug!("FUSE readdir: {}", path);
@@ -886,7 +893,7 @@ impl VfsNode for FuseDirectory {
 		get_filesystem_driver()
 			.ok_or(IoError::ENOSYS)?
 			.lock()
-			.send_command(cmd.as_ref(), rsp.as_mut());
+			.send_command(cmd.as_ref(), rsp.as_mut())?;
 
 		let len: usize = if unsafe { rsp.out_header.assume_init_ref().len } as usize
 			- ::core::mem::size_of::<fuse_abi::OutHeader>()
@@ -931,7 +938,7 @@ impl VfsNode for FuseDirectory {
 		get_filesystem_driver()
 			.unwrap()
 			.lock()
-			.send_command(cmd.as_ref(), rsp.as_mut());
+			.send_command(cmd.as_ref(), rsp.as_mut())?;
 
 		Ok(entries)
 	}
@@ -954,7 +961,7 @@ impl VfsNode for FuseDirectory {
 		get_filesystem_driver()
 			.unwrap()
 			.lock()
-			.send_command(cmd.as_ref(), rsp.as_mut());
+			.send_command(cmd.as_ref(), rsp.as_mut())?;
 
 		if unsafe { rsp.out_header.assume_init_ref().error } != 0 {
 			// TODO: Correct error handling
@@ -990,7 +997,7 @@ impl VfsNode for FuseDirectory {
 		get_filesystem_driver()
 			.unwrap()
 			.lock()
-			.send_command(cmd.as_ref(), rsp.as_mut());
+			.send_command(cmd.as_ref(), rsp.as_mut())?;
 
 		let attr = unsafe { rsp.op_header.assume_init().attr };
 		Ok(FileAttr::from(attr))
@@ -1036,7 +1043,7 @@ impl VfsNode for FuseDirectory {
 			get_filesystem_driver()
 				.ok_or(IoError::ENOSYS)?
 				.lock()
-				.send_command(cmd.as_ref(), rsp.as_mut());
+				.send_command(cmd.as_ref(), rsp.as_mut())?;
 			file_guard.fuse_fh = Some(unsafe { rsp.op_header.assume_init_ref().fh });
 		} else {
 			// Create file (opens implicitly, returns results from both lookup and open calls)
@@ -1045,7 +1052,7 @@ impl VfsNode for FuseDirectory {
 			get_filesystem_driver()
 				.ok_or(IoError::ENOSYS)?
 				.lock()
-				.send_command(cmd.as_ref(), rsp.as_mut());
+				.send_command(cmd.as_ref(), rsp.as_mut())?;
 
 			let inner = unsafe { rsp.op_header.assume_init() };
 			file_guard.fuse_nid = Some(inner.entry.nodeid);
@@ -1072,7 +1079,7 @@ impl VfsNode for FuseDirectory {
 		get_filesystem_driver()
 			.ok_or(IoError::ENOSYS)?
 			.lock()
-			.send_command(cmd.as_ref(), rsp.as_mut());
+			.send_command(cmd.as_ref(), rsp.as_mut())?;
 		trace!("unlink answer {:?}", rsp);
 
 		Ok(())
@@ -1093,7 +1100,7 @@ impl VfsNode for FuseDirectory {
 		get_filesystem_driver()
 			.ok_or(IoError::ENOSYS)?
 			.lock()
-			.send_command(cmd.as_ref(), rsp.as_mut());
+			.send_command(cmd.as_ref(), rsp.as_mut())?;
 		trace!("rmdir answer {:?}", rsp);
 
 		Ok(())
@@ -1118,7 +1125,7 @@ impl VfsNode for FuseDirectory {
 		get_filesystem_driver()
 			.ok_or(IoError::ENOSYS)?
 			.lock()
-			.send_command(cmd.as_ref(), rsp.as_mut());
+			.send_command(cmd.as_ref(), rsp.as_mut())?;
 		if unsafe { rsp.out_header.assume_init_ref().error } == 0 {
 			Ok(())
 		} else {
@@ -1135,7 +1142,10 @@ pub(crate) fn init() {
 
 	if let Some(driver) = get_filesystem_driver() {
 		let (cmd, mut rsp) = ops::Init::create();
-		driver.lock().send_command(cmd.as_ref(), rsp.as_mut());
+		driver
+			.lock()
+			.send_command(cmd.as_ref(), rsp.as_mut())
+			.unwrap();
 		trace!("fuse init answer: {:?}", rsp);
 
 		let mount_point = format!("/{}", driver.lock().get_mount_point());

diff --git a/src/scheduler/mod.rs b/src/scheduler/mod.rs
@@ -506,7 +506,7 @@ impl PerCoreScheduler {
 					}
 				}
 
-				Ready(Ok(()))
+				Ready(Ok::<(), IoError>(()))
 			})
 		})
 		.await?;
-Original file line number
+Diff line change
@@ Expand Up / @@ -506,7 +506,7 @@ impl PerCoreScheduler { @@
     					}
     				}
-    				Ready(Ok(()))
+    				Ready(Ok::<(), IoError>(()))
     			})
     		})
     		.await?;
@@ Expand Down @@