Skip to content
Permalink
Browse files

kernel/sched: Don't reschedule inside a nested lock

The internal "reschedule" API has always understood the idea that it
might run in a ISR context where it can't swap.  But it has always
been happy to swap away when in thread mode, even when the environment
contains an outer lock that would NOT be expecting to swap!  As it
happened, the way irq locks are implemented (they store flag state
that can be restored without context) this would "work" even though it
was completely breaking the synchronization promise made by the outer
lock.

But now, with spinlocks, the error gets detected (albeit in a clumsy
way) in debug builds.  The unexpected swap triggers SPIN_VALIDATE
failures in later threads (this gets reported as a "recursive" lock,
but what actually happened is that another thread got to run before
the lock was released and tried to grab the same lock).

Fix this so that swap can only be called in a situation where the irq
lock key it was passed would have the effect of unmasking interrupts.
Note that this is a real behavioral change that affects when swaps
occur: it's not impossible that there is code out there that actually
relies on this "lock breaking reschedule" for correct behavior.  But
our previous implementation was irredeemably broken and I don't know
how to address that.

Fixes #16273

Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
  • Loading branch information...
andyross authored and andrewboie committed May 24, 2019
1 parent 92ce767 commit 312b43f14510754f18c8c6cf2cbabbd23f26484f
Showing with 4 additions and 4 deletions.
  1. +4 −4 kernel/sched.c
@@ -502,7 +502,7 @@ void z_thread_priority_set(struct k_thread *thread, int prio)
}
}

static inline int resched(void)
static inline int resched(u32_t key)
{
#ifdef CONFIG_SMP
if (!_current_cpu->swap_ok) {
@@ -511,12 +511,12 @@ static inline int resched(void)
_current_cpu->swap_ok = 0;
#endif

return !z_is_in_isr();
return z_arch_irq_unlocked(key) && !z_is_in_isr();
}

void z_reschedule(struct k_spinlock *lock, k_spinlock_key_t key)
{
if (resched()) {
if (resched(key.key)) {
z_swap(lock, key);
} else {
k_spin_unlock(lock, key);
@@ -525,7 +525,7 @@ void z_reschedule(struct k_spinlock *lock, k_spinlock_key_t key)

void z_reschedule_irqlock(u32_t key)
{
if (resched()) {
if (resched(key)) {
z_swap_irqlock(key);
} else {
irq_unlock(key);

0 comments on commit 312b43f

Please sign in to comment.
You can’t perform that action at this time.