Merge pull request #1843 from hermit-os/non_async-object_map

mkroening · web-flow · commit 53536a5c0db8 · 2025-07-24T15:42:10.000Z
feat(scheduler): don't lock `Task::object_map` asynchronously
diff --git a/src/arch/x86_64/kernel/mod.rs b/src/arch/x86_64/kernel/mod.rs
@@ -298,10 +298,9 @@ pub unsafe fn jump_to_user_land(entry_point: usize, code_size: usize, arg: &[&st
 	use x86_64::structures::paging::{PageSize, Size4KiB as BasePageSize};
 
 	use crate::arch::x86_64::kernel::scheduler::TaskStacks;
-	use crate::executor::block_on;
 
 	info!("Create new file descriptor table");
-	block_on(core_scheduler().recreate_objmap(), None).unwrap();
+	core_scheduler().recreate_objmap().unwrap();
 
 	let entry_point: usize = LOADER_START | entry_point;
 	let stack_pointer: usize = LOADER_START
diff --git a/src/fd/mod.rs b/src/fd/mod.rs
@@ -307,8 +307,7 @@ async fn poll_fds(fds: &mut [PollFd]) -> io::Result<u64> {
 		for i in &mut *fds {
 			let fd = i.fd;
 			i.revents = PollEvent::empty();
-			let mut pinned_obj = core::pin::pin!(core_scheduler().get_object(fd));
-			if let Ready(Ok(obj)) = pinned_obj.as_mut().poll(cx) {
+			if let Ok(obj) = core_scheduler().get_object(fd) {
 				let mut pinned = core::pin::pin!(obj.poll(i.events));
 				if let Ready(Ok(e)) = pinned.as_mut().poll(cx)
 					&& !e.is_empty()
@@ -374,33 +373,33 @@ pub fn fstat(fd: FileDescriptor) -> io::Result<FileAttr> {
 pub fn eventfd(initval: u64, flags: EventFlags) -> io::Result<FileDescriptor> {
 	let obj = self::eventfd::EventFd::new(initval, flags);
 
-	let fd = block_on(core_scheduler().insert_object(Arc::new(obj)), None)?;
+	let fd = core_scheduler().insert_object(Arc::new(obj))?;
 
 	Ok(fd)
 }
 
 pub(crate) fn get_object(fd: FileDescriptor) -> io::Result<Arc<dyn ObjectInterface>> {
-	block_on(core_scheduler().get_object(fd), None)
+	core_scheduler().get_object(fd)
 }
 
 pub(crate) fn insert_object(obj: Arc<dyn ObjectInterface>) -> io::Result<FileDescriptor> {
-	block_on(core_scheduler().insert_object(obj), None)
+	core_scheduler().insert_object(obj)
 }
 
 // The dup system call allocates a new file descriptor that refers
 // to the same open file description as the descriptor oldfd. The new
 // file descriptor number is guaranteed to be the lowest-numbered
 // file descriptor that was unused in the calling process.
 pub(crate) fn dup_object(fd: FileDescriptor) -> io::Result<FileDescriptor> {
-	block_on(core_scheduler().dup_object(fd), None)
+	core_scheduler().dup_object(fd)
 }
 
 pub(crate) fn dup_object2(fd1: FileDescriptor, fd2: FileDescriptor) -> io::Result<FileDescriptor> {
-	block_on(core_scheduler().dup_object2(fd1, fd2), None)
+	core_scheduler().dup_object2(fd1, fd2)
 }
 
 pub(crate) fn remove_object(fd: FileDescriptor) -> io::Result<Arc<dyn ObjectInterface>> {
-	block_on(core_scheduler().remove_object(fd), None)
+	core_scheduler().remove_object(fd)
 }
 
 pub(crate) fn isatty(fd: FileDescriptor) -> io::Result<bool> {
diff --git a/src/scheduler/mod.rs b/src/scheduler/mod.rs
@@ -7,13 +7,10 @@ use alloc::sync::Arc;
 #[cfg(feature = "smp")]
 use alloc::vec::Vec;
 use core::cell::RefCell;
-use core::future::{self, Future};
 use core::ptr;
 #[cfg(all(target_arch = "x86_64", feature = "smp"))]
 use core::sync::atomic::AtomicBool;
 use core::sync::atomic::{AtomicI32, AtomicU32, Ordering};
-use core::task::Poll::Ready;
-use core::task::ready;
 
 use ahash::RandomState;
 use crossbeam_utils::Backoff;
@@ -223,8 +220,7 @@ struct NewTask {
 	prio: Priority,
 	core_id: CoreId,
 	stacks: TaskStacks,
-	object_map:
-		Arc<async_lock::RwLock<HashMap<FileDescriptor, Arc<dyn ObjectInterface>, RandomState>>>,
+	object_map: Arc<RwSpinLock<HashMap<FileDescriptor, Arc<dyn ObjectInterface>, RandomState>>>,
 }
 
 impl From<NewTask> for Task {
@@ -461,157 +457,130 @@ impl PerCoreScheduler {
 	#[inline]
 	pub fn get_current_task_object_map(
 		&self,
-	) -> Arc<async_lock::RwLock<HashMap<FileDescriptor, Arc<dyn ObjectInterface>, RandomState>>> {
+	) -> Arc<RwSpinLock<HashMap<FileDescriptor, Arc<dyn ObjectInterface>, RandomState>>> {
 		without_interrupts(|| self.current_task.borrow().object_map.clone())
 	}
 
 	/// Map a file descriptor to their IO interface and returns
 	/// the shared reference
 	#[inline]
-	pub async fn get_object(&self, fd: FileDescriptor) -> io::Result<Arc<dyn ObjectInterface>> {
-		future::poll_fn(|cx| {
-			without_interrupts(|| {
-				let borrowed = self.current_task.borrow();
-				let mut pinned_obj = core::pin::pin!(borrowed.object_map.read());
-
-				let guard = ready!(pinned_obj.as_mut().poll(cx));
-				Ready(guard.get(&fd).cloned().ok_or(Errno::Badf))
-			})
-		})
-		.await
+	pub fn get_object(&self, fd: FileDescriptor) -> io::Result<Arc<dyn ObjectInterface>> {
+		let current_task = self.current_task.borrow();
+		let object_map = current_task.object_map.read();
+		object_map.get(&fd).cloned().ok_or(Errno::Badf)
 	}
 
 	/// Creates a new map between file descriptor and their IO interface and
 	/// clone the standard descriptors.
-	#[allow(dead_code)]
-	pub async fn recreate_objmap(&self) -> io::Result<()> {
+	#[cfg(feature = "common-os")]
+	#[cfg_attr(not(target_arch = "x86_64"), expect(dead_code))]
+	pub fn recreate_objmap(&self) -> io::Result<()> {
 		let mut map = HashMap::<FileDescriptor, Arc<dyn ObjectInterface>, RandomState>::with_hasher(
 			RandomState::with_seeds(0, 0, 0, 0),
 		);
 
-		future::poll_fn(|cx| {
-			without_interrupts(|| {
-				let borrowed = self.current_task.borrow();
-				let mut pinned_obj = core::pin::pin!(borrowed.object_map.read());
-
-				let guard = ready!(pinned_obj.as_mut().poll(cx));
-				// clone standard file descriptors
-				for i in 0..3 {
-					if let Some(obj) = guard.get(&i) {
-						map.insert(i, obj.clone());
-					}
-				}
+		without_interrupts(|| {
+			let mut current_task = self.current_task.borrow_mut();
+			let object_map = current_task.object_map.read();
 
-				Ready(io::Result::Ok(()))
-			})
-		})
-		.await?;
+			// clone standard file descriptors
+			for i in 0..3 {
+				if let Some(obj) = object_map.get(&i) {
+					map.insert(i, obj.clone());
+				}
+			}
 
-		without_interrupts(|| {
-			self.current_task.borrow_mut().object_map = Arc::new(async_lock::RwLock::new(map));
+			drop(object_map);
+			current_task.object_map = Arc::new(RwSpinLock::new(map));
 		});
 
 		Ok(())
 	}
 
 	/// Insert a new IO interface and returns a file descriptor as
 	/// identifier to this object
-	pub async fn insert_object(&self, obj: Arc<dyn ObjectInterface>) -> io::Result<FileDescriptor> {
-		future::poll_fn(|cx| {
-			without_interrupts(|| {
-				let borrowed = self.current_task.borrow();
-				let mut pinned_obj = core::pin::pin!(borrowed.object_map.write());
-
-				let mut guard = ready!(pinned_obj.as_mut().poll(cx));
-				let new_fd = || -> io::Result<FileDescriptor> {
-					let mut fd: FileDescriptor = 0;
-					loop {
-						if !guard.contains_key(&fd) {
-							break Ok(fd);
-						} else if fd == FileDescriptor::MAX {
-							break Err(Errno::Overflow);
-						}
-
-						fd = fd.saturating_add(1);
+	pub fn insert_object(&self, obj: Arc<dyn ObjectInterface>) -> io::Result<FileDescriptor> {
+		without_interrupts(|| {
+			let current_task = self.current_task.borrow();
+			let mut object_map = current_task.object_map.write();
+
+			let new_fd = || -> io::Result<FileDescriptor> {
+				let mut fd: FileDescriptor = 0;
+				loop {
+					if !object_map.contains_key(&fd) {
+						break Ok(fd);
+					} else if fd == FileDescriptor::MAX {
+						break Err(Errno::Overflow);
 					}
-				};
 
-				let fd = new_fd()?;
-				let _ = guard.insert(fd, obj.clone());
-				Ready(Ok(fd))
-			})
+					fd = fd.saturating_add(1);
+				}
+			};
+
+			let fd = new_fd()?;
+			let _ = object_map.insert(fd, obj.clone());
+			Ok(fd)
 		})
-		.await
 	}
 
 	/// Duplicate a IO interface and returns a new file descriptor as
 	/// identifier to the new copy
-	pub async fn dup_object(&self, fd: FileDescriptor) -> io::Result<FileDescriptor> {
-		future::poll_fn(|cx| {
-			without_interrupts(|| {
-				let borrowed = self.current_task.borrow();
-				let mut pinned_obj = core::pin::pin!(borrowed.object_map.write());
-
-				let mut guard = ready!(pinned_obj.as_mut().poll(cx));
-				let obj = (*(guard.get(&fd).ok_or(Errno::Inval)?)).clone();
-
-				let new_fd = || -> io::Result<FileDescriptor> {
-					let mut fd: FileDescriptor = 0;
-					loop {
-						if !guard.contains_key(&fd) {
-							break Ok(fd);
-						} else if fd == FileDescriptor::MAX {
-							break Err(Errno::Overflow);
-						}
-
-						fd = fd.saturating_add(1);
+	pub fn dup_object(&self, fd: FileDescriptor) -> io::Result<FileDescriptor> {
+		without_interrupts(|| {
+			let current_task = self.current_task.borrow();
+			let mut object_map = current_task.object_map.write();
+
+			let obj = (*(object_map.get(&fd).ok_or(Errno::Inval)?)).clone();
+
+			let new_fd = || -> io::Result<FileDescriptor> {
+				let mut fd: FileDescriptor = 0;
+				loop {
+					if !object_map.contains_key(&fd) {
+						break Ok(fd);
+					} else if fd == FileDescriptor::MAX {
+						break Err(Errno::Overflow);
 					}
-				};
 
-				let fd = new_fd()?;
-				if guard.try_insert(fd, obj).is_err() {
-					Ready(Err(Errno::Mfile))
-				} else {
-					Ready(Ok(fd))
+					fd = fd.saturating_add(1);
 				}
-			})
+			};
+
+			let fd = new_fd()?;
+			if object_map.try_insert(fd, obj).is_err() {
+				Err(Errno::Mfile)
+			} else {
+				Ok(fd)
+			}
 		})
-		.await
 	}
 
-	pub async fn dup_object2(
+	pub fn dup_object2(
 		&self,
 		fd1: FileDescriptor,
 		fd2: FileDescriptor,
 	) -> io::Result<FileDescriptor> {
-		future::poll_fn(|cx| {
-			without_interrupts(|| {
-				let borrowed = self.current_task.borrow();
-				let mut pinned_obj = core::pin::pin!(borrowed.object_map.write());
-				let mut guard = ready!(pinned_obj.as_mut().poll(cx));
-				let obj = guard.get(&fd1).cloned().ok_or(Errno::Badf)?;
+		without_interrupts(|| {
+			let current_task = self.current_task.borrow();
+			let mut object_map = current_task.object_map.write();
 
-				if guard.try_insert(fd2, obj).is_err() {
-					Ready(Err(Errno::Mfile))
-				} else {
-					Ready(Ok(fd2))
-				}
-			})
+			let obj = object_map.get(&fd1).cloned().ok_or(Errno::Badf)?;
+
+			if object_map.try_insert(fd2, obj).is_err() {
+				Err(Errno::Mfile)
+			} else {
+				Ok(fd2)
+			}
 		})
-		.await
 	}
 
 	/// Remove a IO interface, which is named by the file descriptor
-	pub async fn remove_object(&self, fd: FileDescriptor) -> io::Result<Arc<dyn ObjectInterface>> {
-		future::poll_fn(|cx| {
-			without_interrupts(|| {
-				let borrowed = self.current_task.borrow();
-				let mut pinned_obj = core::pin::pin!(borrowed.object_map.write());
-				let mut guard = ready!(pinned_obj.as_mut().poll(cx));
-				Ready(guard.remove(&fd).ok_or(Errno::Badf))
-			})
+	pub fn remove_object(&self, fd: FileDescriptor) -> io::Result<Arc<dyn ObjectInterface>> {
+		without_interrupts(|| {
+			let current_task = self.current_task.borrow();
+			let mut object_map = current_task.object_map.write();
+
+			object_map.remove(&fd).ok_or(Errno::Badf)
 		})
-		.await
 	}
 
 	#[inline]
diff --git a/src/scheduler/task.rs b/src/scheduler/task.rs
@@ -12,7 +12,7 @@ use core::{cmp, fmt};
 use ahash::RandomState;
 use crossbeam_utils::CachePadded;
 use hashbrown::HashMap;
-use hermit_sync::OnceCell;
+use hermit_sync::{OnceCell, RwSpinLock};
 use memory_addresses::VirtAddr;
 
 use crate::arch::core_local::*;
@@ -390,8 +390,7 @@ pub(crate) struct Task {
 	/// Stack of the task
 	pub stacks: TaskStacks,
 	/// Mapping between file descriptor and the referenced IO interface
-	pub object_map:
-		Arc<async_lock::RwLock<HashMap<FileDescriptor, Arc<dyn ObjectInterface>, RandomState>>>,
+	pub object_map: Arc<RwSpinLock<HashMap<FileDescriptor, Arc<dyn ObjectInterface>, RandomState>>>,
 	/// Task Thread-Local-Storage (TLS)
 	#[cfg(not(feature = "common-os"))]
 	pub tls: Option<Box<TaskTLS>>,
@@ -412,9 +411,7 @@ impl Task {
 		task_status: TaskStatus,
 		task_prio: Priority,
 		stacks: TaskStacks,
-		object_map: Arc<
-			async_lock::RwLock<HashMap<FileDescriptor, Arc<dyn ObjectInterface>, RandomState>>,
-		>,
+		object_map: Arc<RwSpinLock<HashMap<FileDescriptor, Arc<dyn ObjectInterface>, RandomState>>>,
 	) -> Task {
 		debug!("Creating new task {tid} on core {core_id}");
 
@@ -440,12 +437,12 @@ impl Task {
 
 		/// All cores use the same mapping between file descriptor and the referenced object
 		static OBJECT_MAP: OnceCell<
-			Arc<async_lock::RwLock<HashMap<FileDescriptor, Arc<dyn ObjectInterface>, RandomState>>>,
+			Arc<RwSpinLock<HashMap<FileDescriptor, Arc<dyn ObjectInterface>, RandomState>>>,
 		> = OnceCell::new();
 
 		if core_id == 0 {
 			OBJECT_MAP
-				.set(Arc::new(async_lock::RwLock::new(HashMap::<
+				.set(Arc::new(RwSpinLock::new(HashMap::<
 					FileDescriptor,
 					Arc<dyn ObjectInterface>,
 					RandomState,
@@ -455,7 +452,7 @@ impl Task {
 				.unwrap();
 			let objmap = OBJECT_MAP.get().unwrap().clone();
 			let _ = poll_on(async {
-				let mut guard = objmap.write().await;
+				let mut guard = objmap.write();
 				if env::is_uhyve() {
 					guard
 						.try_insert(STDIN_FILENO, Arc::new(UhyveStdin::new()))
