Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deadlock possible with new allocator #40

Closed
f-bro opened this issue Jul 18, 2017 · 3 comments
Closed

Deadlock possible with new allocator #40

f-bro opened this issue Jul 18, 2017 · 3 comments
Assignees

Comments

@f-bro
Copy link
Member

f-bro commented Jul 18, 2017

The new allocator uses a spin-lock to synchronize the access to the ALLOCATOR(LockedHeap).

Following sequence can result in a deadlock:

  1. ISR1 is entering the lock.
  2. ISR1 is preempted. ISR2 has higher priority and tries to access the lock. Scine the lock is already entered by ISR1, ISR2 has to wait in a loop until ISR1 has left the lock.

ISR1 and ISR2 don't make progress => Deadlock

Solution: Use the PRIMASK register on cortex m processors to synchronize the critical section by disabling interrutps.

See pull request for CortexMHeap rust-embedded/embedded-alloc#6

@phil-opp
Copy link
Contributor

LockedHeap is a wrapper type around Heap that uses a spinlock to implement the Alloc trait for shared references (&LockedHeap). This is required to be able to use it as a global allocator (see the RFC).

Instead of LockedHeap, we should create another wrapper type around Heap that uses the PRIMASK register instead of a spinlock. This wrapper type could live in either this crate or directly in linked-list-allocator.

I'm on vacation for the next four weeks, but I will look into this afterwards. It should be straightforward to implement such a wrapper type though (it should be pretty similar to the implementation of LockedHeap, but with a PRIMASK Mutex instead of a spinlock Mutex).

@f-bro
Copy link
Member Author

f-bro commented Jul 26, 2017

I have already implemented a wrapper (CortexMHeap) and sent a PR to the alloc-cortex-m repo. I just wait until it is merged. When it is merged, I will switch the ALLOCATOR to the new CortexMHeap.

Enjoy your vacation :)

@phil-opp
Copy link
Contributor

Perfect, thanks!

@f-bro f-bro assigned f-bro and unassigned phil-opp Jul 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants