diff options
author | Mulin Chao <mlchao@nuvoton.com> | 2017-01-03 10:40:58 +0800 |
---|---|---|
committer | chrome-bot <chrome-bot@chromium.org> | 2017-01-05 11:01:54 -0800 |
commit | 266b36d54b00de514030b76364cf94977bf02cd0 (patch) | |
tree | 58edcfa86be895aae9beaa7e36eae71d8f705c9b | |
parent | 1523e8b3ef04894d17dec57babed3da02f52fc6c (diff) | |
download | chrome-ec-266b36d54b00de514030b76364cf94977bf02cd0.tar.gz |
core: Fix bug will cause tasks sleep forever by mutex_unlock in task.c.
If there's a task switching occurred between loading waiter and
unlocking the lock, the task with higher priority won't wake up since
the local variable, waiter, doesn't contain its ID bit before task
switching. In this situation, the higher priority task only can be
awakened when the other tasks execute mutex_unlock() again.
But consider the following conditions: (For example, the driver of
charger bd9995x.)
1. There are more than one mutex for the usage path of i2c port.
2. There are more than one task access this usage path of i2c port and
one of these tasks, task A, met the situation above.
3. The other tasks have no chance to execute mutex_unlock() of i2c since
the task A still occupied the mutex of charger.
All the tasks used the same i2c port or the other hardware will sleep
forever. This CL makes loading waiter and unlocking the lock as atomic
to solve this issue.
BRANCH=none
BUG=chrome-os-partner:60617
TEST=make BOARD=snappy; make BOARD=oak; Executed charger factory test on
4 units of snappy for 3 days and no symptom occurred.
Change-Id: Id976fc47955b33ca83bb2182b197d9f2781c341b
Signed-off-by: Mulin Chao <mlchao@nuvoton.com>
Reviewed-on: https://chromium-review.googlesource.com/423285
Commit-Ready: Aaron Durbin <adurbin@chromium.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
-rw-r--r-- | core/cortex-m/task.c | 14 | ||||
-rw-r--r-- | core/cortex-m0/task.c | 14 |
2 files changed, 18 insertions, 10 deletions
diff --git a/core/cortex-m/task.c b/core/cortex-m/task.c index 112c32299d..f23b8142ff 100644 --- a/core/cortex-m/task.c +++ b/core/cortex-m/task.c @@ -539,11 +539,15 @@ void mutex_unlock(struct mutex *mtx) uint32_t waiters; task_ *tsk = current_task; - __asm__ __volatile__(" ldr %0, [%2]\n" - " str %3, [%1]\n" - : "=&r" (waiters) - : "r" (&mtx->lock), "r" (&mtx->waiters), "r" (0) - : "cc"); + /* + * Add a critical section to keep the unlock and the snapshotting of + * waiters atomic in case a task switching occurs between them. + */ + interrupt_disable(); + waiters = mtx->waiters; + mtx->lock = 0; + interrupt_enable(); + while (waiters) { task_id_t id = __fls(waiters); waiters &= ~(1 << id); diff --git a/core/cortex-m0/task.c b/core/cortex-m0/task.c index 3bbee70cae..8796983127 100644 --- a/core/cortex-m0/task.c +++ b/core/cortex-m0/task.c @@ -530,11 +530,15 @@ void mutex_unlock(struct mutex *mtx) uint32_t waiters; task_ *tsk = current_task; - __asm__ __volatile__(" ldr %0, [%2]\n" - " str %3, [%1]\n" - : "=&r" (waiters) - : "r" (&mtx->lock), "r" (&mtx->waiters), "r" (0) - : "cc"); + /* + * Add a critical section to keep the unlock and the snapshotting of + * waiters atomic in case a task switching occurs between them. + */ + interrupt_disable(); + waiters = mtx->waiters; + mtx->lock = 0; + interrupt_enable(); + while (waiters) { task_id_t id = __fls(waiters); waiters &= ~(1 << id); |