Index: kernel/generic/src/synch/workqueue.c =================================================================== --- kernel/generic/src/synch/workqueue.c (revision 8a64e81e7083acc3dc69cd56c21cca72dd37ca13) +++ kernel/generic/src/synch/workqueue.c (revision 0d567128e8779f20e00b0f9912a328a8bfdaf195) @@ -9,4 +9,7 @@ #include +#define WORKQ_MAGIC 0xf00c1333U +#define WORK_ITEM_MAGIC 0xfeec1777U + struct work_queue { @@ -46,4 +49,9 @@ link_t nb_link; + +#ifdef CONFIG_DEBUG + /* Magic cookie for integrity checks. Immutable. Accessed without lock. */ + uint32_t cookie; +#endif }; @@ -79,5 +87,4 @@ - /* Fwd decl. */ static void workq_preinit(struct work_queue *workq, const char *name); @@ -87,4 +94,5 @@ static int _workq_enqueue(struct work_queue *workq, work_t *work_item, work_func_t func, bool can_block); +static void init_work_item(work_t *work_item, work_func_t func); static signal_op_t signal_worker_logic(struct work_queue *workq, bool can_block); static void worker_thread(void *arg); @@ -93,4 +101,7 @@ static void cv_wait(struct work_queue *workq); static void nonblock_init(void); +static bool workq_corrupted(struct work_queue *workq); +static bool work_item_corrupted(work_t *work_item); + /** Creates worker thread for the system-wide worker queue. */ @@ -138,4 +149,5 @@ if (workq) { if (workq_init(workq, name)) { + ASSERT(!workq_corrupted(workq)); return workq; } @@ -150,5 +162,5 @@ void workq_destroy(struct work_queue *workq) { - ASSERT(workq); + ASSERT(!workq_corrupted(workq)); irq_spinlock_lock(&workq->lock, true); @@ -163,4 +175,8 @@ } +#ifdef CONFIG_DEBUG + workq->cookie = 0; +#endif + free(workq); } @@ -169,4 +185,8 @@ static void workq_preinit(struct work_queue *workq, const char *name) { +#ifdef CONFIG_DEBUG + workq->cookie = WORKQ_MAGIC; +#endif + irq_spinlock_initialize(&workq->lock, name); condvar_initialize(&workq->activate_worker); @@ -202,4 +222,6 @@ static bool add_worker(struct work_queue *workq) { + ASSERT(!workq_corrupted(workq)); + thread_t *thread = thread_create(worker_thread, workq, TASK, THREAD_FLAG_NONE, workq->name); @@ -270,4 +292,6 @@ void workq_stop(struct work_queue *workq) { + ASSERT(!workq_corrupted(workq)); + interrupt_workers(workq); wait_for_workers(workq); @@ -285,5 +309,4 @@ /* Respect lock ordering - do not hold workq->lock during broadcast. */ irq_spinlock_unlock(&workq->lock, true); - condvar_broadcast(&workq->activate_worker); @@ -408,9 +431,8 @@ work_func_t func, bool can_block) { + ASSERT(!workq_corrupted(workq)); + bool success = true; signal_op_t signal_op = NULL; - - link_initialize(&work_item->queue_link); - work_item->func = func; irq_spinlock_lock(&workq->lock, true); @@ -419,4 +441,5 @@ success = false; } else { + init_work_item(work_item, func); list_append(&work_item->queue_link, &workq->queue); ++workq->item_cnt; @@ -442,4 +465,15 @@ } +/** Prepare an item to be added to the work item queue. */ +static void init_work_item(work_t *work_item, work_func_t func) +{ +#ifdef CONFIG_DEBUG + work_item->cookie = WORK_ITEM_MAGIC; +#endif + + link_initialize(&work_item->queue_link); + work_item->func = func; +} + /** Returns the number of workers running work func() that are not blocked. */ static size_t active_workers_now(struct work_queue *workq) @@ -497,4 +531,6 @@ static void signal_worker_op(struct work_queue *workq) { + ASSERT(!workq_corrupted(workq)); + condvar_signal(&workq->activate_worker); @@ -515,4 +551,5 @@ static signal_op_t signal_worker_logic(struct work_queue *workq, bool can_block) { + ASSERT(!workq_corrupted(workq)); ASSERT(irq_spinlock_locked(&workq->lock)); @@ -610,5 +647,5 @@ /* Copy the func field so func() can safely free work_item. */ work_func_t func = work_item->func; - + func(work_item); } @@ -618,4 +655,6 @@ static bool dequeue_work(struct work_queue *workq, work_t **pwork_item) { + ASSERT(!workq_corrupted(workq)); + irq_spinlock_lock(&workq->lock, true); @@ -646,4 +685,9 @@ link_t *work_link = list_first(&workq->queue); *pwork_item = list_get_instance(work_link, work_t, queue_link); + +#ifdef CONFIG_DEBUG + ASSERT(!work_item_corrupted(*pwork_item)); + (*pwork_item)->cookie = 0; +#endif list_remove(work_link); --workq->item_cnt; @@ -694,4 +738,5 @@ &workq->lock, SYNCH_NO_TIMEOUT, SYNCH_FLAGS_NONE); + ASSERT(!workq_corrupted(workq)); ASSERT(irq_spinlock_locked(&workq->lock)); @@ -711,6 +756,7 @@ /* Must be blocked in user work func() and not be waiting for work. */ ASSERT(!thread->workq_idling); + ASSERT(thread->state == Sleeping); ASSERT(THREAD != thread); - ASSERT(thread->state == Sleeping); + ASSERT(!workq_corrupted(thread->workq)); /* Protected by thread->lock */ @@ -732,4 +778,6 @@ if (THREAD->workq && THREAD->state == Sleeping && !THREAD->workq_idling) { ASSERT(!THREAD->workq_blocked); + ASSERT(!workq_corrupted(THREAD->workq)); + THREAD->workq_blocked = true; @@ -765,10 +813,9 @@ (0 < workq->activate_pending) ? "increasing" : "stable"; - irq_spinlock_unlock(&workq->lock, true); printf( - "Configured with: max_worker_cnt=%zu, min_worker_cnt=%zu,\n" - " max_concurrent_workers=%zu, max_items_per_worker=%zu\n" + "Configuration: max_worker_cnt=%zu, min_worker_cnt=%zu,\n" + " max_concurrent_workers=%zu, max_items_per_worker=%zu\n" "Workers: %zu\n" "Active: %zu (workers currently processing work)\n" @@ -813,4 +860,6 @@ *pworkq = list_get_instance(list_first(&info->work_queues), struct work_queue, nb_link); + + ASSERT(!workq_corrupted(*pworkq)); list_remove(&(*pworkq)->nb_link); @@ -835,10 +884,10 @@ static void nonblock_init(void) { - irq_spinlock_initialize(&nonblock_adder.lock, "workq:nb.lock"); + irq_spinlock_initialize(&nonblock_adder.lock, "kworkq-nb.lock"); condvar_initialize(&nonblock_adder.req_cv); list_initialize(&nonblock_adder.work_queues); nonblock_adder.thread = thread_create(thr_nonblock_add_worker, - &nonblock_adder, TASK, THREAD_FLAG_NONE, "workq:nb"); + &nonblock_adder, TASK, THREAD_FLAG_NONE, "kworkq-nb"); if (nonblock_adder.thread) { @@ -849,7 +898,38 @@ * sleep, but at least boot the system. */ - printf("Failed to create workq:nb. Sleeping work may stall the workq.\n"); - } -} - - + printf("Failed to create kworkq-nb. Sleeping work may stall the workq.\n"); + } +} + +/** Returns true if the workq is definitely corrupted; false if not sure. + * + * Can be used outside of any locks. + */ +static bool workq_corrupted(struct work_queue *workq) +{ +#ifdef CONFIG_DEBUG + /* + * Needed to make the most current cookie value set by workq_preinit() + * visible even if we access the workq right after it is created but + * on a different cpu. Otherwise, workq_corrupted() would not work + * outside a lock. + */ + memory_barrier(); + return NULL == workq || workq->cookie != WORKQ_MAGIC; +#else + return false; +#endif +} + +/** Returns true if the work_item is definitely corrupted; false if not sure. + * + * Must be used with the work queue protecting spinlock locked. + */ +static bool work_item_corrupted(work_t *work_item) +{ +#ifdef CONFIG_DEBUG + return NULL == work_item || work_item->cookie != WORK_ITEM_MAGIC; +#else + return false; +#endif +}