kernel/sched: protect thread sched_lock with compiler barriers

This has not bitten us yet, but it was a ticking timebomb.

This is similar to the issue that was found with irq_lock/irq_unlock
implementations on several architectures. Having a volatile variable is
not the way to force the sched_lock variable to be
incremented/decremented around the accesses to data it protects.
Instead, a compiler barrier must prevent the compiler from reordering
the memory accesses around setting of sched_lock. Needed in the inline
implementations _sched_lock()/_sched_unlock_no_reschedule(), which
resolve to simple decrement/increment of the per-thread sched_lock
variable.

Change-Id: I06f5b3524889f193efe69caa947118404b1be0b5
Signed-off-by: Benjamin Walsh <walsh.benj@gmail.com>
This commit is contained in:
Benjamin Walsh 2017-02-11 10:50:27 -05:00 committed by Anas Nashif
commit 8d7c274e55
3 changed files with 15 additions and 4 deletions

View file

@ -93,11 +93,11 @@ struct _thread_base {
union {
struct {
#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
volatile uint8_t sched_locked;
uint8_t sched_locked;
int8_t prio;
#else /* LITTLE and PDP */
int8_t prio;
volatile uint8_t sched_locked;
uint8_t sched_locked;
#endif
};
uint16_t preempt;

View file

@ -1,5 +1,5 @@
/*
* Copyright (c) 2016 Wind River Systems, Inc.
* Copyright (c) 2016-2017 Wind River Systems, Inc.
*
* SPDX-License-Identifier: Apache-2.0
*/
@ -239,6 +239,8 @@ static inline void _sched_lock(void)
--_current->base.sched_locked;
compiler_barrier();
K_DEBUG("scheduler locked (%p:%d)\n",
_current, _current->base.sched_locked);
#endif
@ -256,6 +258,8 @@ static ALWAYS_INLINE void _sched_unlock_no_reschedule(void)
__ASSERT(!_is_in_isr(), "");
__ASSERT(_current->base.sched_locked != 0, "");
compiler_barrier();
++_current->base.sched_locked;
#endif
}

View file

@ -1,5 +1,5 @@
/*
* Copyright (c) 2016 Wind River Systems, Inc.
* Copyright (c) 2016-2017 Wind River Systems, Inc.
*
* SPDX-License-Identifier: Apache-2.0
*/
@ -142,6 +142,11 @@ void k_sched_lock(void)
--_current->base.sched_locked;
/* Probably not needed since we're in a real function,
* but it doesn't hurt.
*/
compiler_barrier();
K_DEBUG("scheduler locked (%p:%d)\n",
_current, _current->base.sched_locked);
#endif
@ -155,6 +160,8 @@ void k_sched_unlock(void)
int key = irq_lock();
/* compiler_barrier() not needed, comes from irq_lock() */
++_current->base.sched_locked;
K_DEBUG("scheduler unlocked (%p:%d)\n",