Lephenixnoir
/
gint
10
14
Fork 8
Code Issues 4 Pull Requests Releases Wiki Activity

cpu: fix a data race occurring on the cpu_atomic lock 

The function was designed with multi-threaded concurrency in mind,
where threads can take over while the lock is held and simply block
trying to acquire it, which allows the lock holder to proceed.

However interrupt handlers are different; they have priority, so once
they start they must complete immediately. The cannot afford to block
on the lock as the program would simply freeze. In exchange, they clean
up before they leave, so there are some guarantees on the execution
state even when interrupted.

The correct protection is therefore not a lock but a temporary block on
interrupts. There is no data race on the value of the saved IMASK
because it is preserved during interrupt handling.
This commit is contained in:
Lephe 2021-05-07 09:59:05 +02:00
parent 95dbec17ab
commit 7aa86235a3
Signed by: Lephenixnoir
GPG Key ID: 1BBA026E13FC0495
1 changed files with 12 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
src/cpu/atomic.c
View File

@ -6,41 +6,33 @@
/* Value of IMASK when atomic mode is entered */
static int saved_IMASK = 0;
/* Number of atomic mode levels (sort of mutex) */
/* Number of atomic mode levels */
static unsigned int atomic_level = 0;
/* Lock on (atomic_level) */
static char atomic_level_lock = 0;
void cpu_atomic_start(void)
{
/* Get the lock on (atomic_level) */
while(__atomic_test_and_set(&atomic_level_lock, __ATOMIC_RELAXED)) {}
if(atomic_level == 0) {
cpu_sr_t SR = cpu_getSR();
saved_IMASK = SR.IMASK;
SR.IMASK = 15;
cpu_setSR(SR);
}
/* There is no access problem to IMASK here because interrupts must
preserve and restore it */
cpu_sr_t SR = cpu_getSR();
cpu_sr_t SR2 = SR;
SR2.IMASK = 15;
cpu_setSR(SR2);
/* Now that we're alone, atomically update the atomic level */
if(atomic_level == 0) saved_IMASK = SR.IMASK;
atomic_level++;
/* Release the lock */
__atomic_clear(&atomic_level_lock, __ATOMIC_RELAXED);
}
void cpu_atomic_end(void)
{
while(__atomic_test_and_set(&atomic_level_lock, __ATOMIC_RELAXED)) {}
cpu_sr_t SR = cpu_getSR();
/* Update atomic_level before restoring interrupts */
atomic_level--;
if(atomic_level == 0) {
cpu_sr_t SR = cpu_getSR();
SR.IMASK = saved_IMASK;
saved_IMASK = 0;
cpu_setSR(SR);
}
__atomic_clear(&atomic_level_lock, __ATOMIC_RELAXED);
cpu_setSR(SR);
}