zephyr/kernel/work_q.c
Andy Ross 03c1d28e6e work_q: Correctly clear pending flag in delayed work queue, update docs
As discovered in https://github.com/zephyrproject-rtos/zephyr/issues/5952

...a duplicate call to k_delayed_work_submit_to_queue() on a work item
whose timeout had expired but which had not yet executed (i.e. it was
pending in the queue for the active work queue thread) would fail,
because the cancellation step wouldn't clear the PENDING bit, causing
the resubmission to see the object in an invalid state.  Trivially
fixed by adding a bit clear.

It also turns out that the behavior of the code doesn't match the
docs, which state that a PENDING work item is not supposed to be
cancelled at all.  Fix the docs to remove that.

And on yet further review, it turns out that there's no way to make a
test like the one in the linked bug threadsafe.  The work queue does
no synchronization by design, so if the user code does no external
synchronization it might very well clobber the running handler.  Added
a sentence to the docs to reflect this gotcha.

Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
2018-02-13 18:08:57 -05:00

146 lines
2.9 KiB
C

/*
* Copyright (c) 2016 Intel Corporation
* Copyright (c) 2016 Wind River Systems, Inc.
*
* SPDX-License-Identifier: Apache-2.0
*/
/**
* @file
*
* Workqueue support functions
*/
#include <kernel_structs.h>
#include <wait_q.h>
#include <errno.h>
static void work_q_main(void *work_q_ptr, void *p2, void *p3)
{
struct k_work_q *work_q = work_q_ptr;
ARG_UNUSED(p2);
ARG_UNUSED(p3);
while (1) {
struct k_work *work;
k_work_handler_t handler;
work = k_queue_get(&work_q->queue, K_FOREVER);
if (!work) {
continue;
}
handler = work->handler;
/* Reset pending state so it can be resubmitted by handler */
if (atomic_test_and_clear_bit(work->flags,
K_WORK_STATE_PENDING)) {
handler(work);
}
/* Make sure we don't hog up the CPU if the FIFO never (or
* very rarely) gets empty.
*/
k_yield();
}
}
void k_work_q_start(struct k_work_q *work_q, k_thread_stack_t *stack,
size_t stack_size, int prio)
{
k_queue_init(&work_q->queue);
k_thread_create(&work_q->thread, stack, stack_size, work_q_main,
work_q, 0, 0, prio, 0, 0);
_k_object_init(work_q);
}
#ifdef CONFIG_SYS_CLOCK_EXISTS
static void work_timeout(struct _timeout *t)
{
struct k_delayed_work *w = CONTAINER_OF(t, struct k_delayed_work,
timeout);
/* submit work to workqueue */
k_work_submit_to_queue(w->work_q, &w->work);
}
void k_delayed_work_init(struct k_delayed_work *work, k_work_handler_t handler)
{
k_work_init(&work->work, handler);
_init_timeout(&work->timeout, work_timeout);
work->work_q = NULL;
_k_object_init(work);
}
int k_delayed_work_submit_to_queue(struct k_work_q *work_q,
struct k_delayed_work *work,
s32_t delay)
{
int key = irq_lock();
int err;
/* Work cannot be active in multiple queues */
if (work->work_q && work->work_q != work_q) {
err = -EADDRINUSE;
goto done;
}
/* Cancel if work has been submitted */
if (work->work_q == work_q) {
err = k_delayed_work_cancel(work);
if (err < 0) {
goto done;
}
}
/* Attach workqueue so the timeout callback can submit it */
work->work_q = work_q;
if (!delay) {
/* Submit work if no ticks is 0 */
k_work_submit_to_queue(work_q, &work->work);
} else {
/* Add timeout */
_add_timeout(NULL, &work->timeout, NULL,
_TICK_ALIGN + _ms_to_ticks(delay));
}
err = 0;
done:
irq_unlock(key);
return err;
}
int k_delayed_work_cancel(struct k_delayed_work *work)
{
int key = irq_lock();
if (!work->work_q) {
irq_unlock(key);
return -EINVAL;
}
if (k_work_pending(&work->work)) {
/* Remove from the queue if already submitted */
if (!k_queue_remove(&work->work_q->queue, &work->work)) {
irq_unlock(key);
return -EINVAL;
}
} else {
_abort_timeout(&work->timeout);
}
/* Detach from workqueue */
work->work_q = NULL;
atomic_clear_bit(work->work.flags, K_WORK_STATE_PENDING);
irq_unlock(key);
return 0;
}
#endif /* CONFIG_SYS_CLOCK_EXISTS */