Changeset 0d56712 in mainline for kernel/generic/src/synch/workqueue.c

Timestamp:

2012-07-07T00:27:01Z (11 years ago)

Author:

Adam Hraska <adam.hraska+hos@…>

Branches:

lfn, master, serial

Children:

ff90f5f

Parents:

8a64e81e

Message:

workq: Added magic cookie integrity checks. Fixed test workqueue1.

File:

• kernel/generic/src/synch/workqueue.c (modified) (27 diffs)

kernel/generic/src/synch/workqueue.c

@@ -9 +9 @@
 #include <macros.h>
 
+#define WORKQ_MAGIC      0xf00c1333U
+#define WORK_ITEM_MAGIC  0xfeec1777U
+
 
 struct work_queue {
@@ -46 +49 @@
         
         link_t nb_link;
+        
+#ifdef CONFIG_DEBUG
+        /* Magic cookie for integrity checks. Immutable. Accessed without lock. */
+        uint32_t cookie;
+#endif 
 };
 
@@ -79 +87 @@
 
 
-
 /* Fwd decl. */
 static void workq_preinit(struct work_queue *workq, const char *name);
@@ -87 +94 @@
 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 +101 @@
 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 +149 @@
         if (workq) {
                 if (workq_init(workq, name)) {
+                        ASSERT(!workq_corrupted(workq));
                         return workq;
                 }
@@ -150 +162 @@
 void workq_destroy(struct work_queue *workq)
 {
-        ASSERT(workq);
+        ASSERT(!workq_corrupted(workq));
         
         irq_spinlock_lock(&workq->lock, true);
@@ -163 +175 @@
         }
         
+#ifdef CONFIG_DEBUG
+        workq->cookie = 0;
+#endif 
+        
         free(workq);
 }
@@ -169 +185 @@
 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 +222 @@
 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 +292 @@
 void workq_stop(struct work_queue *workq)
 {
+        ASSERT(!workq_corrupted(workq));
+        
         interrupt_workers(workq);
         wait_for_workers(workq);
@@ -285 +309 @@
         /* Respect lock ordering - do not hold workq->lock during broadcast. */
         irq_spinlock_unlock(&workq->lock, true);
-        
         
         condvar_broadcast(&workq->activate_worker);
@@ -408 +431 @@
         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 +441 @@
                 success = false;
         } else {
+                init_work_item(work_item, func);
                 list_append(&work_item->queue_link, &workq->queue);
                 ++workq->item_cnt;
@@ -442 +465 @@
 }
 
+/** 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 +531 @@
 static void signal_worker_op(struct work_queue *workq)
 {
+        ASSERT(!workq_corrupted(workq));
+
         condvar_signal(&workq->activate_worker);
         
@@ -515 +551 @@
 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 +647 @@
                 /* Copy the func field so func() can safely free work_item. */
                 work_func_t func = work_item->func;
-                
+
                 func(work_item);
         }
@@ -618 +655 @@
 static bool dequeue_work(struct work_queue *workq, work_t **pwork_item)
 {
+        ASSERT(!workq_corrupted(workq));
+        
         irq_spinlock_lock(&workq->lock, true);
         
@@ -646 +685 @@
                 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 +738 @@
                 &workq->lock, SYNCH_NO_TIMEOUT, SYNCH_FLAGS_NONE);
 
+        ASSERT(!workq_corrupted(workq));
         ASSERT(irq_spinlock_locked(&workq->lock));
         
@@ -711 +756 @@
                 /* 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 +778 @@
         if (THREAD->workq && THREAD->state == Sleeping && !THREAD->workq_idling) {
                 ASSERT(!THREAD->workq_blocked);
+                ASSERT(!workq_corrupted(THREAD->workq));
+                
                 THREAD->workq_blocked = true;
                 
@@ -765 +813 @@
                 (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 +860 @@
                 *pworkq = list_get_instance(list_first(&info->work_queues), 
                         struct work_queue, nb_link);
+
+                ASSERT(!workq_corrupted(*pworkq));
                 
                 list_remove(&(*pworkq)->nb_link);
@@ -835 +884 @@
 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 +898 @@
                  * 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
+}