Changeset 9fe9d296 in mainline

Timestamp:

2012-11-16T17:36:23Z (11 years ago)

Author:

Adam Hraska <adam.hraska+hos@…>

Branches:

lfn, master, serial, ticket/834-toolchain-update, topic/msim-upgrade, topic/simplify-dev-export

Children:

853d613

Parents:

497bd656

Message:

Fix: waitq_sleep_timeout() waits for its waitq_wakeup() to complete unconditionally. No need to wait explicitly via waitq_complete_wakeup() anymore.

Location:

kernel/generic

Files:

• include/synch/waitq.h (modified) (1 diff)
• src/synch/rcu.c (modified) (1 diff)
• src/synch/spinlock.c (modified) (1 diff)
• src/synch/waitq.c (modified) (6 diffs)

kernel/generic/include/synch/waitq.h

@@ -77 +77 @@
 extern void waitq_sleep_finish(waitq_t *, int, ipl_t);
 extern void waitq_wakeup(waitq_t *, wakeup_mode_t);
-extern void waitq_complete_wakeup(waitq_t *);
 extern void _waitq_wakeup_unsafe(waitq_t *, wakeup_mode_t);
 extern void waitq_interrupt_sleep(struct thread *);

kernel/generic/src/synch/rcu.c

@@ -492 +492 @@
         _rcu_call(expedite, &completion.rcu_item, synch_complete);
         waitq_sleep(&completion.wq);
-        waitq_complete_wakeup(&completion.wq);
 }
 

kernel/generic/src/synch/spinlock.c

@@ -199 +199 @@
  *
  * @param lock    IRQ spinlock to be locked.
- * @param irq_dis If true, interrupts are actually disabled
- *                prior locking the spinlock. If false, interrupts
- *                are expected to be already disabled.
+ * @param irq_dis If true, disables interrupts before locking the spinlock.
+ *                If false, interrupts are expected to be already disabled.
  *
  */

kernel/generic/src/synch/waitq.c

@@ -57 +57 @@
 
 static void waitq_sleep_timed_out(void *);
+static void waitq_complete_wakeup(waitq_t *);
+
 
 /** Initialize wait queue
@@ -330 +332 @@
                 break;
         default:
+                /* 
+                 * Wait for a waitq_wakeup() or waitq_unsleep() to complete
+                 * before returning from waitq_sleep() to the caller. Otherwise
+                 * the caller might expect that the wait queue is no longer used 
+                 * and deallocate it (although the wakeup on a another cpu has 
+                 * not yet completed and is using the wait queue). 
+                 * 
+                 * Note that we have to do this for ESYNCH_OK_BLOCKED and
+                 * ESYNCH_INTERRUPTED, but not necessarily for ESYNCH_TIMEOUT
+                 * where the timeout handler stops using the waitq before waking 
+                 * us up. To be on the safe side, ensure the waitq is not in use 
+                 * anymore in this case as well.
+                 */
+                waitq_complete_wakeup(wq);
                 break;
         }
@@ -357 +373 @@
         } else {
                 if (PARAM_NON_BLOCKING(flags, usec)) {
-                        /* Return immediatelly instead of going to sleep */
+                        /* Return immediately instead of going to sleep */
                         return ESYNCH_WOULD_BLOCK;
                 }
@@ -448 +464 @@
  * exits. It returns immediately if there are no concurrent wakeups 
  * at the time.
+ * 
+ * Interrupts must be disabled.
  * 
  * Example usage:
@@ -466 +484 @@
  *     // callback() completed its work, but it may still be accessing 
  *     // wq in waitq_wakeup(). Therefore it is not yet safe to return 
- *     // or it would clobber up our stack (where wq is stored).
- *     waitq_complete_wakeup(&wq);
- *     // waitq_wakeup() is complete, it is safe to free wq.
+ *     // from waitq_sleep() or it would clobber up our stack (where wq 
+ *     // is stored). waitq_sleep() ensures the wait queue is no longer
+ *     // in use by invoking waitq_complete_wakeup() internally.
+ *     
+ *     // waitq_sleep() returned, it is safe to free wq.
  * }
  * @endcode
@@ -474 +494 @@
  * @param wq  Pointer to a wait queue.
  */
-void waitq_complete_wakeup(waitq_t *wq)
-{
-        irq_spinlock_lock(&wq->lock, true);
-        irq_spinlock_unlock(&wq->lock, true);
+static void waitq_complete_wakeup(waitq_t *wq)
+{
+        ASSERT(interrupts_disabled());
+        
+        irq_spinlock_lock(&wq->lock, false);
+        irq_spinlock_unlock(&wq->lock, false);
 }