Merge pull request tock#4254 from Ioan-Cristian/scheduler

kernel: scheduler: Use NonZeroU32 instead of u32 for `start()` and `get_remaining_us()`

arch/cortex-m/src/systick.rs

@@ -8,6 +8,8 @@ use kernel::utilities::registers::interfaces::{Readable, Writeable};
 use kernel::utilities::registers::{register_bitfields, FieldValue, ReadOnly, ReadWrite};
 use kernel::utilities::StaticRef;
 
+use core::num::NonZeroU32;
+
 #[repr(C)]
 struct SystickRegisters {
     syst_csr: ReadWrite<u32, ControlAndStatus::Register>,
@@ -121,13 +123,13 @@ impl SysTick {
 }
 
 impl kernel::platform::scheduler_timer::SchedulerTimer for SysTick {
-    fn start(&self, us: u32) {
+    fn start(&self, us: NonZeroU32) {
         let reload = {
             // We need to convert from microseconds to native tics, which could overflow in 32-bit
             // arithmetic. So we convert to 64-bit. 64-bit division is an expensive subroutine, but
             // if `us` is a power of 10 the compiler will simplify it with the 1_000_000 divisor
             // instead.
-            let us = us as u64;
+            let us = us.get() as u64;
             let hertz = self.hertz() as u64;
 
             hertz * us / 1_000_000
@@ -198,14 +200,14 @@ impl kernel::platform::scheduler_timer::SchedulerTimer for SysTick {
             .write(ControlAndStatus::TICKINT::CLEAR + ControlAndStatus::ENABLE::SET + clock_source);
     }
 
-    fn get_remaining_us(&self) -> Option<u32> {
+    fn get_remaining_us(&self) -> Option<NonZeroU32> {
         // use u64 in case of overflow when multiplying by 1,000,000
         let tics = SYSTICK_BASE.syst_cvr.read(CurrentValue::CURRENT) as u64;
         if SYSTICK_BASE.syst_csr.is_set(ControlAndStatus::COUNTFLAG) {
             None
         } else {
             let hertz = self.hertz() as u64;
-            Some(((tics * 1_000_000) / hertz) as u32)
+            NonZeroU32::new(((tics * 1_000_000) / hertz) as u32)
         }
     }
 }

chips/sifive/src/clint.rs

@@ -5,6 +5,7 @@
 //! Create a timer using the Machine Timer registers.
 
 use core::marker::PhantomData;
+use core::num::NonZeroU32;
 
 use kernel::hil::time::{self, Alarm, ConvertTicks, Frequency, Ticks, Ticks64, Time};
 use kernel::utilities::cells::OptionalCell;
@@ -103,13 +104,13 @@ impl<'a, F: Frequency> time::Alarm<'a> for Clint<'a, F> {
 /// hardware timer can be used to provide alarms to capsules and userspace. This
 /// implementation will not work alongside other uses of the machine timer.
 impl<F: Frequency> kernel::platform::scheduler_timer::SchedulerTimer for Clint<'_, F> {
-    fn start(&self, us: u32) {
+    fn start(&self, us: NonZeroU32) {
         let now = self.now();
-        let tics = self.ticks_from_us(us);
+        let tics = self.ticks_from_us(us.get());
         self.set_alarm(now, tics);
     }
 
-    fn get_remaining_us(&self) -> Option<u32> {
+    fn get_remaining_us(&self) -> Option<NonZeroU32> {
         // We need to convert from native tics to us, multiplication could overflow in 32-bit
         // arithmetic. So we convert to 64-bit.
         let diff = self.get_alarm().wrapping_sub(self.now()).into_u64();
@@ -122,11 +123,11 @@ impl<F: Frequency> kernel::platform::scheduler_timer::SchedulerTimer for Clint<'
         // However, if the alarm frequency is slow enough relative to the cpu frequency, it is
         // possible this will be evaluated while now() == get_alarm(), so we special case that
         // result where the alarm has fired but the subtraction has not overflowed
-        if diff >= <Self as Time>::Frequency::frequency() as u64 || diff == 0 {
+        if diff >= <Self as Time>::Frequency::frequency() as u64 {
             None
         } else {
             let hertz = <Self as Time>::Frequency::frequency() as u64;
-            Some(((diff * 1_000_000) / hertz) as u32)
+            NonZeroU32::new(((diff * 1_000_000) / hertz) as u32)
         }
     }
 

kernel/src/kernel.rs

@@ -9,6 +9,7 @@
 //! etc.) is defined in the `scheduler` subcrate and selected by a board.
 
 use core::cell::Cell;
+use core::num::NonZeroU32;
 
 use crate::capabilities;
 use crate::config;
@@ -476,7 +477,7 @@ impl Kernel {
         chip: &C,
         process: &dyn process::Process,
         ipc: Option<&crate::ipc::IPC<NUM_PROCS>>,
-        timeslice_us: Option<u32>,
+        timeslice_us: Option<NonZeroU32>,
     ) -> (process::StoppedExecutingReason, Option<u32>) {
         // We must use a dummy scheduler timer if the process should be executed
         // without any timeslice restrictions. Note, a chip may not provide a
@@ -508,7 +509,7 @@ impl Kernel {
         // timeslice.
         loop {
             let stop_running = match scheduler_timer.get_remaining_us() {
-                Some(us) => us <= MIN_QUANTA_THRESHOLD_US,
+                Some(us) => us.get() <= MIN_QUANTA_THRESHOLD_US,
                 None => true,
             };
             if stop_running {
@@ -709,11 +710,11 @@ impl Kernel {
             // first.
             if return_reason == process::StoppedExecutingReason::TimesliceExpired {
                 // used the whole timeslice
-                timeslice
+                timeslice.get()
             } else {
                 match scheduler_timer.get_remaining_us() {
-                    Some(remaining) => timeslice - remaining,
-                    None => timeslice, // used whole timeslice
+                    Some(remaining) => timeslice.get() - remaining.get(),
+                    None => timeslice.get(), // used whole timeslice
                 }
             }
         });

kernel/src/platform/scheduler_timer.rs

@@ -9,6 +9,8 @@
 
 use crate::hil::time::{self, Frequency, Ticks};
 
+use core::num::NonZeroU32;
+
 /// Interface for the system scheduler timer.
 ///
 /// A system scheduler timer provides a countdown timer to enforce process
@@ -71,7 +73,7 @@ pub trait SchedulerTimer {
     /// peripheral, increments of 10ms are most accurate thanks to additional
     /// hardware support for this value. ARM SysTick supports intervals up to
     /// 400ms.
-    fn start(&self, us: u32);
+    fn start(&self, us: NonZeroU32);
 
     /// Reset the SchedulerTimer.
     ///
@@ -122,7 +124,7 @@ pub trait SchedulerTimer {
     /// called again after returning `None` without an intervening call to
     /// `start()`, the return value is unspecified and implementations may
     /// return whatever they like.
-    fn get_remaining_us(&self) -> Option<u32>;
+    fn get_remaining_us(&self) -> Option<NonZeroU32>;
 }
 
 /// A dummy `SchedulerTimer` implementation in which the timer never expires.
@@ -132,14 +134,14 @@ pub trait SchedulerTimer {
 impl SchedulerTimer for () {
     fn reset(&self) {}
 
-    fn start(&self, _: u32) {}
+    fn start(&self, _: NonZeroU32) {}
 
     fn disarm(&self) {}
 
     fn arm(&self) {}
 
-    fn get_remaining_us(&self) -> Option<u32> {
-        Some(10000) // chose arbitrary large value
+    fn get_remaining_us(&self) -> Option<NonZeroU32> {
+        NonZeroU32::new(10000) // choose arbitrary large value
     }
 }
 
@@ -169,13 +171,13 @@ impl<A: 'static + time::Alarm<'static>> SchedulerTimer for VirtualSchedulerTimer
         let _ = self.alarm.disarm();
     }
 
-    fn start(&self, us: u32) {
+    fn start(&self, us: NonZeroU32) {
         let tics = {
             // We need to convert from microseconds to native tics, which could overflow in 32-bit
             // arithmetic. So we convert to 64-bit. 64-bit division is an expensive subroutine, but
             // if `us` is a power of 10 the compiler will simplify it with the 1_000_000 divisor
             // instead.
-            let us = us as u64;
+            let us = us.get() as u64;
             let hertz = A::Frequency::frequency() as u64;
 
             (hertz * us / 1_000_000) as u32
@@ -193,7 +195,7 @@ impl<A: 'static + time::Alarm<'static>> SchedulerTimer for VirtualSchedulerTimer
         //self.alarm.disarm();
     }
 
-    fn get_remaining_us(&self) -> Option<u32> {
+    fn get_remaining_us(&self) -> Option<NonZeroU32> {
         // We need to convert from native tics to us, multiplication could overflow in 32-bit
         // arithmetic. So we convert to 64-bit.
 
@@ -211,11 +213,11 @@ impl<A: 'static + time::Alarm<'static>> SchedulerTimer for VirtualSchedulerTimer
         // However, if the alarm frequency is slow enough relative to the cpu frequency, it is
         // possible this will be evaluated while now() == get_alarm(), so we special case that
         // result where the alarm has fired but the subtraction has not overflowed
-        if diff >= A::Frequency::frequency() as u64 || diff == 0 {
+        if diff >= A::Frequency::frequency() as u64 {
             None
         } else {
             let hertz = A::Frequency::frequency() as u64;
-            Some(((diff * 1_000_000) / hertz) as u32)
+            NonZeroU32::new(((diff * 1_000_000) / hertz) as u32)
         }
     }
 }

kernel/src/scheduler.rs

@@ -14,6 +14,8 @@ use crate::platform::chip::Chip;
 use crate::process::ProcessId;
 use crate::process::StoppedExecutingReason;
 
+use core::num::NonZeroU32;
+
 /// Trait which any scheduler must implement.
 pub trait Scheduler<C: Chip> {
     /// Decide which process to run next.
@@ -91,7 +93,7 @@ pub enum SchedulingDecision {
     /// Tell the kernel to run the specified process with the passed timeslice.
     /// If `None` is passed as a timeslice, the process will be run
     /// cooperatively.
-    RunProcess((ProcessId, Option<u32>)),
+    RunProcess((ProcessId, Option<NonZeroU32>)),
 
     /// Tell the kernel to go to sleep. Notably, if the scheduler asks the
     /// kernel to sleep when kernel tasks are ready, the kernel will not sleep,

kernel/src/scheduler/mlfq.rs

@@ -22,6 +22,7 @@
 //!           topmost queue.
 
 use core::cell::Cell;
+use core::num::NonZeroU32;
 
 use crate::collections::list::{List, ListLink, ListNode};
 use crate::hil::time::{self, ConvertTicks, Ticks};
@@ -156,7 +157,7 @@ impl<A: 'static + time::Alarm<'static>, C: Chip> Scheduler<C> for MLFQSched<'_,
         self.last_queue_idx.set(queue_idx);
         self.last_timeslice.set(timeslice);
 
-        SchedulingDecision::RunProcess((next, Some(timeslice)))
+        SchedulingDecision::RunProcess((next, NonZeroU32::new(timeslice)))
     }
 
     fn result(&self, result: StoppedExecutingReason, execution_time_us: Option<u32>) {

kernel/src/scheduler/round_robin.rs

@@ -19,6 +19,7 @@
 //! interrupted.
 
 use core::cell::Cell;
+use core::num::NonZeroU32;
 
 use crate::collections::list::{List, ListLink, ListNode};
 use crate::platform::chip::Chip;
@@ -120,9 +121,10 @@ impl<C: Chip> Scheduler<C> for RoundRobinSched<'_> {
             self.time_remaining.set(self.timeslice_length);
             self.timeslice_length
         };
-        assert!(timeslice != 0);
+        // Why should this panic?
+        let non_zero_timeslice = NonZeroU32::new(timeslice).unwrap();
 
-        SchedulingDecision::RunProcess((next, Some(timeslice)))
+        SchedulingDecision::RunProcess((next, Some(non_zero_timeslice)))
     }
 
     fn result(&self, result: StoppedExecutingReason, execution_time_us: Option<u32>) {