fix: Proper network wakers

This refactors our timer interrupt handling into a separate module that
all timer interrupts are configured through and handled by. It allows us
to track why a timer interrupt was fired.

We make use of this to now only conditionally wake the network task's
waker when necessary. This is either because we sent some network
packets, a network device driver received an interrupt, or because a
timer interrupt that we configured so we can poll smoltcp in the future
was handled.

Author: Gelbpunkt

src/arch/aarch64/kernel/interrupts.rs

@@ -24,7 +24,7 @@ use crate::drivers::pci::get_interrupt_handlers;
 use crate::drivers::{InterruptHandlerQueue, InterruptLine};
 use crate::kernel::serial::handle_uart_interrupt;
 use crate::mm::{PageAlloc, PageRangeAllocator};
-use crate::scheduler::{self, CoreId};
+use crate::scheduler::{self, CoreId, timer_interrupts};
 use crate::{core_id, core_scheduler, env};
 
 /// The ID of the first Private Peripheral Interrupt.
@@ -93,10 +93,7 @@ pub(crate) fn install_handlers() {
 
 	fn timer_handler() {
 		debug!("Handle timer interrupt");
-
-		// disable timer
-		CNTP_CVAL_EL0.set(0);
-		CNTP_CTL_EL0.write(CNTP_CTL_EL0::ENABLE::CLEAR);
+		timer_interrupts::clear_active_and_set_next();
 	}
 
 	for (key, value) in get_interrupt_handlers().into_iter() {

src/arch/riscv64/kernel/scheduler.rs

@@ -5,10 +5,10 @@ use free_list::{PageLayout, PageRange};
 use memory_addresses::{PhysAddr, VirtAddr};
 
 use crate::arch::riscv64::kernel::core_local::core_scheduler;
-use crate::arch::riscv64::kernel::processor::set_oneshot_timer;
 use crate::arch::riscv64::mm::paging::{BasePageSize, PageSize, PageTableEntryFlags};
 use crate::mm::{FrameAlloc, PageAlloc, PageRangeAllocator};
 use crate::scheduler::task::{Task, TaskFrame};
+use crate::scheduler::timer_interrupts;
 use crate::{DEFAULT_STACK_SIZE, KERNEL_STACK_SIZE};
 
 #[repr(C, packed)]
@@ -360,9 +360,9 @@ unsafe extern "C" fn task_start(func: extern "C" fn(usize), arg: usize, user_sta
 }
 
 pub fn timer_handler() {
-	//increment_irq_counter(apic::TIMER_INTERRUPT_NUMBER.into());
+	debug!("Handle timer interrupt");
+	timer_interrupts::clear_active_and_set_next();
 	core_scheduler().handle_waiting_tasks();
-	set_oneshot_timer(None);
 	core_scheduler().scheduler();
 }
 

src/arch/x86_64/kernel/scheduler.rs

@@ -16,8 +16,8 @@ use crate::arch::x86_64::mm::paging::{
 use crate::config::*;
 use crate::env;
 use crate::mm::{FrameAlloc, PageAlloc, PageRangeAllocator};
-use crate::scheduler::PerCoreSchedulerExt;
 use crate::scheduler::task::{Task, TaskFrame};
+use crate::scheduler::{PerCoreSchedulerExt, timer_interrupts};
 
 #[repr(C, packed)]
 struct State {
@@ -318,6 +318,10 @@ impl TaskFrame for Task {
 
 extern "x86-interrupt" fn timer_handler(_stack_frame: interrupts::ExceptionStackFrame) {
 	increment_irq_counter(apic::TIMER_INTERRUPT_NUMBER);
+
+	debug!("Handle timer interrupt");
+	timer_interrupts::clear_active_and_set_next();
+
 	core_scheduler().handle_waiting_tasks();
 	apic::eoi();
 	core_scheduler().reschedule();

src/drivers/net/gem.rs

@@ -32,6 +32,7 @@ use crate::drivers::net::{NetworkDriver, mtu};
 #[cfg(feature = "pci")]
 use crate::drivers::pci as hardware;
 use crate::drivers::{Driver, InterruptLine};
+use crate::executor::network::NETWORK_WAKER;
 use crate::mm::device_alloc::DeviceAlloc;
 use crate::{BasePageSize, PageSize};
 
@@ -276,6 +277,9 @@ impl NetworkDriver for GEMDriver {
 
 	fn handle_interrupt(&mut self) {
 		self.tx_fields.handle_interrupt();
+
+		trace!("Waking network waker");
+		NETWORK_WAKER.lock().wake();
 	}
 }
 

src/drivers/net/loopback.rs

@@ -8,6 +8,7 @@ use smoltcp::time::Instant;
 
 use crate::drivers::net::NetworkDriver;
 use crate::drivers::{Driver, InterruptLine};
+use crate::executor::network::NETWORK_WAKER;
 use crate::mm::device_alloc::DeviceAlloc;
 
 pub(crate) struct LoopbackDriver {
@@ -122,7 +123,8 @@ impl NetworkDriver for LoopbackDriver {
 	}
 
 	fn handle_interrupt(&mut self) {
-		// no-op
+		trace!("Waking network waker");
+		NETWORK_WAKER.lock().wake();
 	}
 }
 

src/drivers/net/rtl8139.rs

@@ -21,6 +21,7 @@ use crate::drivers::Driver;
 use crate::drivers::error::DriverError;
 use crate::drivers::net::{NetworkDriver, mtu};
 use crate::drivers::pci::PciDevice;
+use crate::executor::network::NETWORK_WAKER;
 use crate::mm::device_alloc::DeviceAlloc;
 
 /// size of the receive buffer
@@ -687,6 +688,9 @@ impl NetworkDriver for RTL8139Driver {
 		self.regs.as_mut_ptr().isr().write(le16::from(
 			isr_contents & (ISR_RXOVW | ISR_TER | ISR_RER | ISR_TOK | ISR_ROK),
 		));
+
+		trace!("Waking network waker");
+		NETWORK_WAKER.lock().wake();
 	}
 }
 

src/drivers/net/virtio/mod.rs

@@ -42,6 +42,7 @@ use crate::drivers::virtio::virtqueue::{
 	AvailBufferToken, BufferElem, BufferType, UsedBufferToken, VirtQueue, Virtq,
 };
 use crate::drivers::{Driver, InterruptLine};
+use crate::executor::network::NETWORK_WAKER;
 use crate::mm::device_alloc::DeviceAlloc;
 
 /// A wrapper struct for the raw configuration structure.
@@ -417,6 +418,9 @@ impl NetworkDriver for VirtioNetDriver<Init> {
 		}
 
 		self.isr_stat.acknowledge();
+
+		trace!("Waking network waker");
+		NETWORK_WAKER.lock().wake();
 	}
 }
 

src/executor/mod.rs

@@ -18,15 +18,11 @@ use core::time::Duration;
 
 use crossbeam_utils::Backoff;
 use hermit_sync::without_interrupts;
-#[cfg(feature = "net")]
-use smoltcp::time::Instant;
 
 use crate::arch::core_local;
 use crate::errno::Errno;
 use crate::executor::task::AsyncTask;
 use crate::io;
-#[cfg(feature = "net")]
-use crate::scheduler::PerCoreSchedulerExt;
 use crate::synch::futex::*;
 
 /// WakerRegistration is derived from smoltcp's
@@ -155,101 +151,26 @@ where
 
 		let now = crate::arch::kernel::systemtime::now_micros();
 		if let Poll::Ready(t) = result {
-			// allow network interrupts
-			#[cfg(feature = "net")]
-			{
-				if let Some(mut guard) = crate::executor::network::NIC.try_lock() {
-					let delay = if let Ok(nic) = guard.as_nic_mut() {
-						nic.set_polling_mode(false);
-
-						nic.poll_delay(Instant::from_micros_const(now.try_into().unwrap()))
-							.map(|d| d.total_micros())
-					} else {
-						None
-					};
-					core_local::core_scheduler().add_network_timer(
-						delay.map(|d| crate::arch::processor::get_timer_ticks() + d),
-					);
-				}
-			}
-
 			return t;
 		}
 
 		if let Some(duration) = timeout
 			&& Duration::from_micros(now - start) >= duration
 		{
-			// allow network interrupts
-			#[cfg(feature = "net")]
-			{
-				if let Some(mut guard) = crate::executor::network::NIC.try_lock() {
-					let delay = if let Ok(nic) = guard.as_nic_mut() {
-						nic.set_polling_mode(false);
-
-						nic.poll_delay(Instant::from_micros_const(now.try_into().unwrap()))
-							.map(|d| d.total_micros())
-					} else {
-						None
-					};
-					core_local::core_scheduler().add_network_timer(
-						delay.map(|d| crate::arch::processor::get_timer_ticks() + d),
-					);
-				}
-			}
-
 			return Err(Errno::Time);
 		}
 
-		#[cfg(feature = "net")]
 		if backoff.is_completed() {
-			let delay = if let Some(mut guard) = crate::executor::network::NIC.try_lock() {
-				if let Ok(nic) = guard.as_nic_mut() {
-					nic.set_polling_mode(false);
-
-					nic.poll_delay(Instant::from_micros_const(now.try_into().unwrap()))
-						.map(|d| d.total_micros())
-				} else {
-					None
-				}
-			} else {
-				None
-			};
-
-			if delay.unwrap_or(10_000_000) > 10_000 {
-				core_local::core_scheduler().add_network_timer(
-					delay.map(|d| crate::arch::processor::get_timer_ticks() + d),
-				);
-				let wakeup_time =
-					timeout.map(|duration| start + u64::try_from(duration.as_micros()).unwrap());
-
-				// switch to another task
-				task_notify.wait(wakeup_time);
-
-				// restore default values
-				if let Ok(nic) = crate::executor::network::NIC.lock().as_nic_mut() {
-					nic.set_polling_mode(true);
-				}
-
-				backoff.reset();
-			}
-		} else {
-			backoff.snooze();
-		}
-
-		#[cfg(not(feature = "net"))]
-		{
-			if backoff.is_completed() {
-				let wakeup_time =
-					timeout.map(|duration| start + u64::try_from(duration.as_micros()).unwrap());
+			let wakeup_time =
+				timeout.map(|duration| start + u64::try_from(duration.as_micros()).unwrap());
 
-				// switch to another task
-				task_notify.wait(wakeup_time);
+			// switch to another task
+			task_notify.wait(wakeup_time);
 
-				// restore default values
-				backoff.reset();
-			} else {
-				backoff.snooze();
-			}
+			// restore default values
+			backoff.reset();
+		} else {
+			backoff.snooze();
 		}
 	}
 }

src/executor/network.rs

@@ -26,10 +26,10 @@ use crate::arch;
 use crate::drivers::net::{NetworkDevice, NetworkDriver};
 #[cfg(feature = "dns")]
 use crate::errno::Errno;
-use crate::executor::spawn;
+use crate::executor::{WakerRegistration, spawn};
 #[cfg(feature = "dns")]
 use crate::io;
-use crate::scheduler::PerCoreSchedulerExt;
+use crate::scheduler::timer_interrupts::{Source, create_timer};
 
 pub(crate) enum NetworkState<'a> {
 	Missing,
@@ -189,14 +189,33 @@ async fn dhcpv4_run() {
 	.await;
 }
 
+pub(crate) static NETWORK_WAKER: InterruptTicketMutex<WakerRegistration> =
+	InterruptTicketMutex::new(WakerRegistration::new());
+
 async fn network_run() {
 	future::poll_fn(|cx| {
 		if let Some(mut guard) = NIC.try_lock() {
 			match &mut *guard {
 				NetworkState::Initialized(nic) => {
-					nic.poll_common(now());
-					// FIXME: only wake when progress can be made
-					cx.waker().wake_by_ref();
+					let now = now();
+
+					match nic.poll_common(now) {
+						PollResult::SocketStateChanged => {
+							// Progress was made
+							cx.waker().wake_by_ref();
+						}
+						PollResult::None => {
+							// Very likely no progress can be made, so set up a timer interrupt to wake the waker
+							NETWORK_WAKER.lock().register(cx.waker());
+							nic.set_polling_mode(false);
+							if let Some(wakeup_time) = nic.poll_delay(now).map(|d| d.total_micros())
+							{
+								create_timer(Source::Network, wakeup_time);
+								trace!("Configured an interrupt for {wakeup_time:?}");
+							}
+						}
+					}
+
 					Poll::Pending
 				}
 				_ => Poll::Ready(()),
@@ -254,14 +273,7 @@ pub(crate) fn init() {
 
 	*guard = NetworkInterface::create();
 
-	if let NetworkState::Initialized(nic) = &mut *guard {
-		let time = now();
-		nic.poll_common(time);
-		let wakeup_time = nic
-			.poll_delay(time)
-			.map(|d| crate::arch::processor::get_timer_ticks() + d.total_micros());
-		crate::core_scheduler().add_network_timer(wakeup_time);
-
+	if let NetworkState::Initialized(_) = &mut *guard {
 		spawn(network_run());
 		#[cfg(feature = "dhcpv4")]
 		spawn(dhcpv4_run());

src/fd/socket/tcp.rs

@@ -13,7 +13,7 @@ use smoltcp::wire::{IpEndpoint, Ipv4Address, Ipv6Address};
 
 use crate::errno::Errno;
 use crate::executor::block_on;
-use crate::executor::network::{Handle, NIC};
+use crate::executor::network::{Handle, NETWORK_WAKER, NIC};
 use crate::fd::{self, Endpoint, ListenEndpoint, ObjectInterface, PollEvent, SocketOption};
 use crate::syscalls::socket::Af;
 use crate::{DEFAULT_KEEP_ALIVE_INTERVAL, io};
@@ -64,14 +64,18 @@ impl Socket {
 	fn with<R>(&self, f: impl FnOnce(&mut tcp::Socket<'_>) -> R) -> R {
 		let mut guard = NIC.lock();
 		let nic = guard.as_nic_mut().unwrap();
-		f(nic.get_mut_socket::<tcp::Socket<'_>>(*self.handle.first().unwrap()))
+		let r = f(nic.get_mut_socket::<tcp::Socket<'_>>(*self.handle.first().unwrap()));
+		NETWORK_WAKER.lock().wake();
+		r
 	}
 
 	fn with_context<R>(&self, f: impl FnOnce(&mut tcp::Socket<'_>, &mut iface::Context) -> R) -> R {
 		let mut guard = NIC.lock();
 		let nic = guard.as_nic_mut().unwrap();
 		let (s, cx) = nic.get_socket_and_context::<tcp::Socket<'_>>(*self.handle.first().unwrap());
-		f(s, cx)
+		let r = f(s, cx);
+		NETWORK_WAKER.lock().wake();
+		r
 	}
 
 	async fn close(&self) -> io::Result<()> {