impl Mutex on all shared resources

document how to write generic code that operates on resources

book/src/by-example/tips.md

@@ -1,5 +1,25 @@
 # Tips & tricks
 
+## Generics
+
+Resources shared between two or more tasks implement the `Mutex` trait in *all*
+contexts, even on those where a critical section is not required to access the
+data. This lets you easily write generic code that operates on resources and can
+be called from different tasks. Here's one such example:
+
+``` rust
+{{#include ../../../examples/generics.rs}}
+```
+
+``` console
+$ cargo run --example generics
+{{#include ../../../ci/expected/generics.run}}```
+
+This also lets you change the static priorities of tasks without having to
+rewrite code. If you consistently use `lock`s to access the data behind shared
+resources then your code will continue to compile when you change the priority
+of tasks.
+
 ## Running tasks from RAM
 
 The main goal of moving the specification of RTFM applications to attributes in

ci/expected/generics.run

@@ -0,0 +1,6 @@
+UART1(STATE = 0)
+SHARED: 0 -> 1
+UART0(STATE = 0)
+SHARED: 1 -> 2
+UART1(STATE = 1)
+SHARED: 2 -> 4

ci/script.sh

@@ -57,6 +57,7 @@ main() {
                 not-send
                 not-sync
 
+                generics
                 ramfunc
             )
 

examples/capacity.rs

@@ -23,7 +23,7 @@ macro_rules! println {
 
 #[app(device = lm3s6965)]
 const APP: () = {
-    #[init(spawn = [foo])]
+    #[init]
     fn init() {
         rtfm::pend(Interrupt::UART0);
     }

examples/generics.rs

@@ -0,0 +1,74 @@
+//! examples/generics.rs
+
+#![deny(unsafe_code)]
+#![deny(warnings)]
+#![no_main]
+#![no_std]
+
+extern crate panic_semihosting;
+
+use cortex_m_semihosting::debug;
+use lm3s6965::Interrupt;
+use rtfm::{app, Mutex};
+
+// NOTE: This convenience macro will appear in all the other examples and
+// will always look the same
+macro_rules! println {
+    ($($tt:tt)*) => {
+        if let Ok(mut stdout) = cortex_m_semihosting::hio::hstdout() {
+            use core::fmt::Write;
+
+            writeln!(stdout, $($tt)*).ok();
+        }
+    };
+}
+
+#[app(device = lm3s6965)]
+const APP: () = {
+    static mut SHARED: u32 = 0;
+
+    #[init]
+    fn init() {
+        rtfm::pend(Interrupt::UART0);
+        rtfm::pend(Interrupt::UART1);
+    }
+
+    #[interrupt(resources = [SHARED])]
+    fn UART0() {
+        static mut STATE: u32 = 0;
+
+        println!("UART0(STATE = {})", *STATE);
+
+        advance(STATE, resources.SHARED);
+
+        rtfm::pend(Interrupt::UART1);
+
+        debug::exit(debug::EXIT_SUCCESS);
+    }
+
+    #[interrupt(priority = 2, resources = [SHARED])]
+    fn UART1() {
+        static mut STATE: u32 = 0;
+
+        println!("UART1(STATE = {})", *STATE);
+
+        // just to show that `SHARED` can be accessed directly and ..
+        *resources.SHARED += 0;
+        // .. also through a (no-op) `lock`
+        resources.SHARED.lock(|shared| *shared += 0);
+
+        advance(STATE, resources.SHARED);
+    }
+};
+
+fn advance(state: &mut u32, mut shared: impl Mutex<T = u32>) {
+    *state += 1;
+
+    let (old, new) = shared.lock(|shared| {
+        let old = *shared;
+        *shared += *state;
+        (old, *shared)
+    });
+
+    println!("SHARED: {} -> {}", old, new);
+}

examples/types.rs

@@ -8,7 +8,7 @@
 extern crate panic_semihosting;
 
 use cortex_m_semihosting::debug;
-use rtfm::{app, Instant};
+use rtfm::{app, Exclusive, Instant};
 
 #[app(device = lm3s6965)]
 const APP: () = {
@@ -43,6 +43,7 @@ const APP: () = {
     #[task(priority = 2, resources = [SHARED], schedule = [foo], spawn = [foo])]
     fn foo() {
         let _: Instant = scheduled;
+        let _: Exclusive<u32> = resources.SHARED;
         let _: foo::Resources = resources;
         let _: foo::Schedule = schedule;
         let _: foo::Spawn = spawn;

macros/src/analyze.rs

@@ -30,13 +30,14 @@ pub struct Analysis {
 pub enum Ownership {
     // NOTE priorities and ceilings are "logical" (0 = lowest priority, 255 = highest priority)
     Owned { priority: u8 },
+    CoOwned { priority: u8 },
     Shared { ceiling: u8 },
 }
 
 impl Ownership {
     pub fn needs_lock(&self, priority: u8) -> bool {
         match *self {
-            Ownership::Owned { .. } => false,
+            Ownership::Owned { .. } | Ownership::CoOwned { .. } => false,
             Ownership::Shared { ceiling } => {
                 debug_assert!(ceiling >= priority);
 
@@ -44,6 +45,13 @@ impl Ownership {
             }
         }
     }
+
+    pub fn is_owned(&self) -> bool {
+        match *self {
+            Ownership::Owned { .. } => true,
+            _ => false,
+        }
+    }
 }
 
 pub struct Dispatcher {
@@ -72,18 +80,24 @@ pub fn app(app: &App) -> Analysis {
     for (priority, res) in app.resource_accesses() {
         if let Some(ownership) = ownerships.get_mut(res) {
             match *ownership {
-                Ownership::Owned { priority: ceiling } | Ownership::Shared { ceiling } => {
+                Ownership::Owned { priority: ceiling }
-                    if priority != ceiling {
+                | Ownership::CoOwned { priority: ceiling }
-                        *ownership = Ownership::Shared {
+                | Ownership::Shared { ceiling }
-                            ceiling: cmp::max(ceiling, priority),
+                    if priority != ceiling =>
-                        };
+                {
+                    *ownership = Ownership::Shared {
+                        ceiling: cmp::max(ceiling, priority),
+                    };
 
-                        let res = &app.resources[res];
+                    let res = &app.resources[res];
-                        if res.mutability.is_none() {
+                    if res.mutability.is_none() {
-                            assert_sync.insert(res.ty.clone());
+                        assert_sync.insert(res.ty.clone());
-                        }
                     }
                 }
+                Ownership::Owned { priority: ceiling } if ceiling == priority => {
+                    *ownership = Ownership::CoOwned { priority };
+                }
+                _ => {}
             }
 
             continue;

macros/src/codegen.rs

@@ -676,6 +676,7 @@ fn prelude(
                 }
             } else {
                 let ownership = &analysis.ownerships[name];
+                let mut exclusive = false;
 
                 if ownership.needs_lock(logical_prio) {
                     may_call_lock = true;
@@ -710,28 +711,53 @@ fn prelude(
                             exprs.push(quote!(#name: <#name as owned_singleton::Singleton>::new()));
                         } else {
                             needs_unsafe = true;
-                            defs.push(quote!(pub #name: &'a mut #name));
+                            if ownership.is_owned() {
-                            exprs.push(
+                                defs.push(quote!(pub #name: &'a mut #name));
-                                quote!(#name: &mut <#name as owned_singleton::Singleton>::new()),
+                                exprs.push(quote!(
-                            );
+                                    #name: &mut <#name as owned_singleton::Singleton>::new()
+                                ));
+                            } else {
+                                may_call_lock = true;
+                                defs.push(quote!(pub #name: rtfm::Exclusive<'a, #name>));
+                                exprs.push(quote!(
+                                    #name: rtfm::Exclusive(
+                                        &mut <#name as owned_singleton::Singleton>::new()
+                                    )
+                                ));
+                            }
                         }
                         continue;
                     } else {
-                        defs.push(quote!(pub #name: &#lt #mut_ #ty));
+                        if ownership.is_owned() || mut_.is_none() {
+                            defs.push(quote!(pub #name: &#lt #mut_ #ty));
+                        } else {
+                            exclusive = true;
+                            may_call_lock = true;
+                            defs.push(quote!(pub #name: rtfm::Exclusive<#lt, #ty>));
+                        }
                     }
                 }
 
                 let alias = &ctxt.statics[name];
                 needs_unsafe = true;
                 if initialized {
-                    exprs.push(quote!(#name: &#mut_ #alias));
+                    if exclusive {
+                        exprs.push(quote!(#name: rtfm::Exclusive(&mut #alias)));
+                    } else {
+                        exprs.push(quote!(#name: &#mut_ #alias));
+                    }
                 } else {
                     let method = if mut_.is_some() {
                         quote!(get_mut)
                     } else {
                         quote!(get_ref)
                     };
-                    exprs.push(quote!(#name: #alias.#method() ));
+
+                    if exclusive {
+                        exprs.push(quote!(#name: rtfm::Exclusive(#alias.#method()) ));
+                    } else {
+                        exprs.push(quote!(#name: #alias.#method() ));
+                    }
                 }
             }
         }
@@ -1655,19 +1681,23 @@ fn mk_resource(
     };
 
     items.push(quote!(
-        unsafe impl<'a> rtfm::Mutex for #path<'a> {
+        impl<'a> rtfm::Mutex for #path<'a> {
-            const CEILING: u8 = #ceiling;
+            type T = #ty;
-            const NVIC_PRIO_BITS: u8 = #device::NVIC_PRIO_BITS;
-            type Data = #ty;
 
-            #[inline(always)]
+            #[inline]
-            unsafe fn priority(&self) -> &core::cell::Cell<u8> {
+            fn lock<R, F>(&mut self, f: F) -> R
-                &self.#priority
+            where
-            }
+                F: FnOnce(&mut Self::T) -> R,
-
+            {
-            #[inline(always)]
+                unsafe {
-            fn ptr(&self) -> *mut Self::Data {
+                    rtfm::export::claim(
-                unsafe { #ptr }
+                        #ptr,
+                        &self.#priority,
+                        #ceiling,
+                        #device::NVIC_PRIO_BITS,
+                        f,
+                    )
+                }
             }
         }
     ));

src/export.rs

@@ -1,5 +1,8 @@
-/// IMPLEMENTATION DETAILS. DO NOT USE ANYTHING IN THIS MODULE
+//! IMPLEMENTATION DETAILS. DO NOT USE ANYTHING IN THIS MODULE
-use core::{hint, ptr};
+
+#[cfg(not(debug_assertions))]
+use core::hint;
+use core::{cell::Cell, ptr, u8};
 
 #[cfg(armv7m)]
 use cortex_m::register::basepri;
@@ -52,7 +55,13 @@ impl<T> MaybeUninit<T> {
         if let Some(x) = self.value.as_ref() {
             x
         } else {
-            hint::unreachable_unchecked()
+            match () {
+                // Try to catch UB when compiling in release with debug assertions enabled
+                #[cfg(debug_assertions)]
+                () => unreachable!(),
+                #[cfg(not(debug_assertions))]
+                () => hint::unreachable_unchecked(),
+            }
         }
     }
 
@@ -60,12 +69,19 @@ impl<T> MaybeUninit<T> {
         if let Some(x) = self.value.as_mut() {
             x
         } else {
-            hint::unreachable_unchecked()
+            match () {
+                // Try to catch UB when compiling in release with debug assertions enabled
+                #[cfg(debug_assertions)]
+                () => unreachable!(),
+                #[cfg(not(debug_assertions))]
+                () => hint::unreachable_unchecked(),
+            }
         }
     }
 
     pub fn set(&mut self, val: T) {
-        unsafe { ptr::write(&mut self.value, Some(val)) }
+        // NOTE(volatile) we have observed UB when this uses a plain `ptr::write`
+        unsafe { ptr::write_volatile(&mut self.value, Some(val)) }
     }
 }
 
@@ -82,3 +98,66 @@ where
     T: Sync,
 {
 }
+
+#[cfg(armv7m)]
+#[inline(always)]
+pub unsafe fn claim<T, R, F>(
+    ptr: *mut T,
+    priority: &Cell<u8>,
+    ceiling: u8,
+    nvic_prio_bits: u8,
+    f: F,
+) -> R
+where
+    F: FnOnce(&mut T) -> R,
+{
+    let current = priority.get();
+
+    if priority.get() < ceiling {
+        if ceiling == (1 << nvic_prio_bits) {
+            priority.set(u8::MAX);
+            let r = interrupt::free(|_| f(&mut *ptr));
+            priority.set(current);
+            r
+        } else {
+            priority.set(ceiling);
+            basepri::write(logical2hw(ceiling, nvic_prio_bits));
+            let r = f(&mut *ptr);
+            basepri::write(logical2hw(current, nvic_prio_bits));
+            priority.set(current);
+            r
+        }
+    } else {
+        f(&mut *ptr)
+    }
+}
+
+#[cfg(not(armv7m))]
+#[inline(always)]
+pub unsafe fn claim<T, R, F>(
+    ptr: *mut T,
+    priority: &Cell<u8>,
+    ceiling: u8,
+    _nvic_prio_bits: u8,
+    f: F,
+) -> R
+where
+    F: FnOnce(&mut T) -> R,
+{
+    let current = priority.get();
+
+    if priority.get() < ceiling {
+        priority.set(u8::MAX);
+        let r = interrupt::free(|_| f(&mut *ptr));
+        priority.set(current);
+        r
+    } else {
+        f(&mut *ptr)
+    }
+}
+
+#[cfg(armv7m)]
+#[inline]
+fn logical2hw(logical: u8, nvic_prio_bits: u8) -> u8 {
+    ((1 << nvic_prio_bits) - logical) << (8 - nvic_prio_bits)
+}

src/lib.rs

@@ -26,16 +26,14 @@
 #![deny(warnings)]
 #![no_std]
 
-use core::{cell::Cell, u8};
 #[cfg(feature = "timer-queue")]
-use core::{cmp::Ordering, ops};
+use core::cmp::Ordering;
+use core::{fmt, ops};
 
 #[cfg(not(feature = "timer-queue"))]
 use cortex_m::peripheral::SYST;
-#[cfg(armv7m)]
-use cortex_m::register::basepri;
 use cortex_m::{
-    interrupt::{self, Nr},
+    interrupt::Nr,
     peripheral::{CBP, CPUID, DCB, DWT, FPB, FPU, ITM, MPU, NVIC, SCB, TPIU},
 };
 pub use cortex_m_rtfm_macros::app;
@@ -253,81 +251,76 @@ impl U32Ext for u32 {
 /// [BASEPRI]) of the current context.
 ///
 /// [BASEPRI]: https://developer.arm.com/products/architecture/cpu-architecture/m-profile/docs/100701/latest/special-purpose-mask-registers
-pub unsafe trait Mutex {
+pub trait Mutex {
-    /// IMPLEMENTATION DETAIL. DO NOT USE THIS CONSTANT
-    #[doc(hidden)]
-    const CEILING: u8;
-
-    /// IMPLEMENTATION DETAIL. DO NOT USE THIS CONSTANT
-    #[doc(hidden)]
-    const NVIC_PRIO_BITS: u8;
-
     /// Data protected by the mutex
-    type Data: Send;
+    type T;
-
-    /// IMPLEMENTATION DETAIL. DO NOT USE THIS METHOD
-    #[doc(hidden)]
-    unsafe fn priority(&self) -> &Cell<u8>;
-
-    /// IMPLEMENTATION DETAIL. DO NOT USE THIS METHOD
-    #[doc(hidden)]
-    fn ptr(&self) -> *mut Self::Data;
 
     /// Creates a critical section and grants temporary access to the protected data
-    #[inline(always)]
-    #[cfg(armv7m)]
     fn lock<R, F>(&mut self, f: F) -> R
     where
-        F: FnOnce(&mut Self::Data) -> R,
+        F: FnOnce(&mut Self::T) -> R;
-    {
+}
-        unsafe {
-            let current = self.priority().get();
 
-            if self.priority().get() < Self::CEILING {
+impl<'a, M> Mutex for &'a mut M
-                if Self::CEILING == (1 << Self::NVIC_PRIO_BITS) {
+where
-                    self.priority().set(u8::MAX);
+    M: Mutex,
-                    let r = interrupt::free(|_| f(&mut *self.ptr()));
+{
-                    self.priority().set(current);
+    type T = M::T;
-                    r
-                } else {
-                    self.priority().set(Self::CEILING);
-                    basepri::write(logical2hw(Self::CEILING, Self::NVIC_PRIO_BITS));
-                    let r = f(&mut *self.ptr());
-                    basepri::write(logical2hw(current, Self::NVIC_PRIO_BITS));
-                    self.priority().set(current);
-                    r
-                }
-            } else {
-                f(&mut *self.ptr())
-            }
-        }
-    }
 
-    /// Creates a critical section and grants temporary access to the protected data
-    #[cfg(not(armv7m))]
     fn lock<R, F>(&mut self, f: F) -> R
     where
-        F: FnOnce(&mut Self::Data) -> R,
+        F: FnOnce(&mut Self::T) -> R,
     {
-        unsafe {
+        (**self).lock(f)
-            let current = self.priority().get();
-
-            if self.priority().get() < Self::CEILING {
-                self.priority().set(u8::MAX);
-                let r = interrupt::free(|_| f(&mut *self.ptr()));
-                self.priority().set(current);
-                r
-            } else {
-                f(&mut *self.ptr())
-            }
-        }
     }
 }
 
-#[cfg(armv7m)]
+/// Newtype over `&'a mut T` that implements the `Mutex` trait
-#[inline]
+///
-fn logical2hw(logical: u8, nvic_prio_bits: u8) -> u8 {
+/// The `Mutex` implementation for this type is a no-op, no critical section is created
-    ((1 << nvic_prio_bits) - logical) << (8 - nvic_prio_bits)
+pub struct Exclusive<'a, T>(pub &'a mut T);
+
+impl<'a, T> Mutex for Exclusive<'a, T> {
+    type T = T;
+
+    fn lock<R, F>(&mut self, f: F) -> R
+    where
+        F: FnOnce(&mut Self::T) -> R,
+    {
+        f(self.0)
+    }
+}
+
+impl<'a, T> fmt::Debug for Exclusive<'a, T>
+where
+    T: fmt::Debug,
+{
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        (**self).fmt(f)
+    }
+}
+
+impl<'a, T> fmt::Display for Exclusive<'a, T>
+where
+    T: fmt::Display,
+{
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        (**self).fmt(f)
+    }
+}
+
+impl<'a, T> ops::Deref for Exclusive<'a, T> {
+    type Target = T;
+
+    fn deref(&self) -> &T {
+        self.0
+    }
+}
+
+impl<'a, T> ops::DerefMut for Exclusive<'a, T> {
+    fn deref_mut(&mut self) -> &mut T {
+        self.0
+    }
 }
 
 /// Sets the given `interrupt` as pending

src/tq.rs

@@ -91,7 +91,7 @@ where
 #[inline(always)]
 pub fn isr<TQ, T, N, F>(mut tq: TQ, mut f: F)
 where
-    TQ: Mutex<Data = TimerQueue<T, N>>,
+    TQ: Mutex<T = TimerQueue<T, N>>,
     T: Copy + Send,
     N: ArrayLength<NotReady<T>>,
     F: FnMut(T, u8),