From c0183fdeddf48479d5698da7f8f9fa8bf6f540d4 Mon Sep 17 00:00:00 2001 From: Andy Ross Date: Mon, 11 Mar 2019 10:53:44 -0700 Subject: [PATCH] kernel/work_q: Fix locking across multiple queues There was a detected user error in the code where racing insertions of k_delayed_work items into different queues would be detected and flagged as an error (honestly I don't see much value there -- Zephyr doesn't as a general rule protect against errors like this, and work_q's are inherently kernel things that don't require userspace-style checking). This got broken with spinlockification, where each work_q object got its own lock, so the single lock wouldn't protect against the other insert function any more. As it happens, that was needless. The core synchronization on a work_q is in the internal k_queue object anyway -- the lock in this file was only ever used for (very fast, noncontending) delayed work insertion. So go back to a global lock to preserve the original behavior. Fixes #14104 Signed-off-by: Andy Ross --- include/kernel.h | 1 - kernel/work_q.c | 13 +++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/include/kernel.h b/include/kernel.h index 22e03e28852..ac3a3f05714 100644 --- a/include/kernel.h +++ b/include/kernel.h @@ -2533,7 +2533,6 @@ typedef void (*k_work_handler_t)(struct k_work *work); struct k_work_q { struct k_queue queue; struct k_thread thread; - struct k_spinlock lock; }; enum { diff --git a/kernel/work_q.c b/kernel/work_q.c index 6ecd613e522..9f21b8d290f 100644 --- a/kernel/work_q.c +++ b/kernel/work_q.c @@ -19,6 +19,8 @@ #define WORKQUEUE_THREAD_NAME "workqueue" +static struct k_spinlock lock; + extern void z_work_q_main(void *work_q_ptr, void *p2, void *p3); void k_work_q_start(struct k_work_q *work_q, k_thread_stack_t *stack, @@ -73,7 +75,7 @@ int k_delayed_work_submit_to_queue(struct k_work_q *work_q, struct k_delayed_work *work, s32_t delay) { - k_spinlock_key_t key = k_spin_lock(&work_q->lock); + k_spinlock_key_t key = k_spin_lock(&lock); int err = 0; /* Work cannot be active in multiple queues */ @@ -97,7 +99,7 @@ int k_delayed_work_submit_to_queue(struct k_work_q *work_q, * blocking operation, so release the lock first. */ if (!delay) { - k_spin_unlock(&work_q->lock, key); + k_spin_unlock(&lock, key); k_work_submit_to_queue(work_q, &work->work); return 0; } @@ -107,7 +109,7 @@ int k_delayed_work_submit_to_queue(struct k_work_q *work_q, _TICK_ALIGN + z_ms_to_ticks(delay)); done: - k_spin_unlock(&work_q->lock, key); + k_spin_unlock(&lock, key); return err; } @@ -117,11 +119,10 @@ int k_delayed_work_cancel(struct k_delayed_work *work) return -EINVAL; } - struct k_spinlock *lock = &work->work_q->lock; - k_spinlock_key_t key = k_spin_lock(lock); + k_spinlock_key_t key = k_spin_lock(&lock); int ret = work_cancel(work); - k_spin_unlock(lock, key); + k_spin_unlock(&lock, key); return ret; }