Skip to content

Commit

Permalink
kernel/mutex: Fix races, make unlock rescheduling
Browse files Browse the repository at this point in the history
The k_mutex is a priority-inheriting mutex, so on unlock it's possible
that a thread's priority will be lowered.  Make this a reschedule
point so that reasoning about thread priorities is easier (possibly at
the cost of performance): most users are going to expect that the
priority elevation stops at exactly the moment of unlock.

Note that this also reorders the code to fix what appear to be obvious
race conditions.  After the call to z_ready_thread(), that thread may
be run (e.g. by an interrupt preemption or on another SMP core), yet
the return value and mutex weren't correctly set yet.  The spinlock
was also prematurely released.

Fixes #20802

Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
  • Loading branch information
Andy Ross authored and dleach02 committed Dec 3, 2019
1 parent 84f8235 commit 7022000
Showing 1 changed file with 3 additions and 6 deletions.
9 changes: 3 additions & 6 deletions kernel/mutex.c
Original file line number Diff line number Diff line change
Expand Up @@ -233,18 +233,15 @@ void z_impl_k_mutex_unlock(struct k_mutex *mutex)
mutex, new_owner, new_owner ? new_owner->base.prio : -1000);

if (new_owner != NULL) {
z_ready_thread(new_owner);

k_spin_unlock(&lock, key);

arch_thread_return_value_set(new_owner, 0);

/*
* new owner is already of higher or equal prio than first
* waiter since the wait queue is priority-based: no need to
* ajust its priority
*/
mutex->owner_orig_prio = new_owner->base.prio;
arch_thread_return_value_set(new_owner, 0);
z_ready_thread(new_owner);
z_reschedule(&lock, key);
} else {
mutex->lock_count = 0U;
k_spin_unlock(&lock, key);
Expand Down

0 comments on commit 7022000

Please sign in to comment.