rust-embedded/cortex-m

Implement a More Ergonomic & Efficient Mutex for the Embedded

Congyuwang opened this issue · 2 comments

Mutexes are mostly used for static variables, and also a lot of times the variable needs to be initialized later.

The current Mutex implementation doesn't feel very ergonomic:

  1. to allow mutable access + late initialization to a variable, we need to use Mutex<RefCell<Option<T>>> with var.borrow(cs).borrow_mut().replace(value) for initialization and var.borrow(cs).borrow_mut().as_mut().unwrap() for mutable access. These are very long expressions for simple purposes.
  2. RefCell comes with an extra isize 32bit space with Option adding another u8 4bit overhead. Each time accessing a mutable reference to a variable basically involves two unwraps, once for checking uniqueness and once for checking initialization, which is unnecessary.

I am thinking of an easier to use + more efficient implementation:

pub struct Mutex<T>(UnsafeCell<MutexInner<T>>);

struct MutexInner<T> {
    state: MutexInnerState,
    value: MaybeUninit<T>,
}

pub struct LockGuard<'cs, T>(&'cs mut MutexInner<T>);

We can use a single enum to keep track of the state of the mutex cell. So the value can be

  • uninitialized (None),
  • locked (borrowed),
  • unlock (free to borrow).
enum MutexInnerState {
    Locked,
    Uinit,
    Unlock,
}

The mutex can be either initialized with a value or not.

impl<T> Mutex<T> {
    /// Creates a new mutex.
    pub const fn new(value: T) -> Self {
        Self(UnsafeCell::new(MutexInner {
            state: MutexInnerState::Unlock,
            value: MaybeUninit::new(value),
        }))
    }

    /// Creates a new unit mutex.
    pub const fn new_uinit() -> Self {
        Self(UnsafeCell::new(MutexInner {
            state: MutexInnerState::Uinit,
            value: MaybeUninit::uninit(),
        }))
    }
}

Value can be initialized once if it was not initialized (otherwise panic).

impl<T> Mutex<T> {
    /// Value initialization.
    ///
    /// panic if already initialized.
    pub fn init<'cs>(&'cs self, _cs: &'cs CriticalSection, value: T) {
        let inner = unsafe { &mut *self.0.get() };
        if let MutexInnerState::Uinit = inner.state {
            inner.state = MutexInnerState::Unlock;
            inner.value = MaybeUninit::new(value);
        } else {
            panic!()
        }
    }
}

Locking the mutex returns None if try_lock fails or if the value is uninitialized.

impl<T> Mutex<T> {
    /// Try to lock the mutex.
    pub fn try_lock<'cs>(&'cs self, _cs: &'cs CriticalSection) -> Option<LockGuard<'cs, T>> {
        let inner = unsafe { &mut *self.0.get() };
        match inner.state {
            MutexInnerState::Uinit | MutexInnerState::Locked => None,
            MutexInnerState::Unlock => {
                inner.state = MutexInnerState::Locked;
                Some(LockGuard(inner))
            }
        }
    }
}

The LockGuard restore lock state back to Unlock on drop.

impl<'cs, T> Drop for LockGuard<'cs, T> {
    fn drop(&mut self) {
        self.0.state = MutexInnerState::Unlock;
    }
}

Miscellaneous.

impl<T> Drop for Mutex<T> {
    fn drop(&mut self) {
        let inner = unsafe { &mut *self.0.get() };
        if let MutexInnerState::Unlock | MutexInnerState::Locked = inner.state {
            unsafe { inner.value.assume_init_drop() }
        }
    }
}

impl<'cs, T> Deref for LockGuard<'cs, T> {
    type Target = T;

    #[inline]
    fn deref(&self) -> &Self::Target {
        unsafe { self.0.value.assume_init_ref() }
    }
}

impl<'cs, T> DerefMut for LockGuard<'cs, T> {
    #[inline]
    fn deref_mut(&mut self) -> &mut Self::Target {
        unsafe { self.0.value.assume_init_mut() }
    }
}

unsafe impl<T> Sync for Mutex<T> where T: Send {}

Usage

Now we can write like

// compared to `Mutex <RefCell<Option<Rtc<RTC0>>>>`
static RTC: Mutex<Rtc<RTC0>> = Mutex::new_uinit();

// initialization
cortex_m::interrupt::free(|cs| RTC.init(cs, rtc0));

// access
#[interrupt]
fn RTC0() {
    cortex_m::interrupt::free(|cs| {
        if let (Some(mut rtc), Some(mut other)) = (RTC.try_lock(cs), OTHER.try_lock(cs)) {
            other.handle_interrupt(&mut rtc);
        }
    });
}

Of course we should not use the name Mutex directly since we need to have backward compatibility. I am thinking of naming it MutexCell or perhaps MutexOption. If anyone finds it useful maybe we can PR it into the library.

Related issues #224 #359.

Moved to critical-section repository issue.