michaelh/zephyr
1
0
Fork 0
Code Releases Activity

kernel/work_q: Fix locking across multiple queues

Browse source 
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 <andrew.j.ross@intel.com>
This commit is contained in:
Andy Ross 2019-03-11 10:53:44 -07:00 • committed by Anas Nashif
commit c0183fdedd
2 changed files with 7 additions and 7 deletions
Download patch file Download diff file Expand all files Collapse all files

1
include/kernel.h
View file

@ -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 {

13
kernel/work_q.c
View file

@ -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;
}