Fixes reset_alarm_abs()
authorBarret Rhoden <brho@cs.berkeley.edu>
Fri, 15 Nov 2013 04:59:55 +0000 (20:59 -0800)
committerBarret Rhoden <brho@cs.berkeley.edu>
Thu, 16 Jan 2014 19:45:47 +0000 (11:45 -0800)
It couldn't handle an alarm that had never been set, since has_fired was
initially 0.  Renaming has_fired clarifies its use, fixes the init
problem, and makes the code clearer.

kern/include/alarm.h
kern/src/alarm.c
kern/src/rendez.c

index d2a153c..8f52320 100644 (file)
@@ -63,7 +63,7 @@ struct alarm_waiter {
        struct semaphore                        sem;                    /* kthread will sleep on this */
        void                                            *data;
        TAILQ_ENTRY(alarm_waiter)       next;
-       bool                                            has_fired;
+       bool                                            on_tchain;
 };
 TAILQ_HEAD(awaiters_tailq, alarm_waiter);              /* ideally not a LL */
 
index 29db9d9..4baf2a9 100644 (file)
@@ -54,7 +54,7 @@ void init_awaiter(struct alarm_waiter *waiter,
        waiter->func = func;
        if (!func)
                sem_init_irqsave(&waiter->sem, 0);
-       waiter->has_fired = FALSE;      /* so we can check this before arming */
+       waiter->on_tchain = FALSE;
 }
 
 /* Give this the absolute time.  For now, abs_time is the TSC time that you want
@@ -108,8 +108,8 @@ static void reset_tchain_interrupt(struct timer_chain *tchain)
  * will wake up.  o/w, it will call the func ptr stored in the awaiter. */
 static void wake_awaiter(struct alarm_waiter *waiter)
 {
-       waiter->has_fired = TRUE;
-       cmb();  /* enforce the has_fired write before the handlers */
+       waiter->on_tchain = FALSE;
+       cmb();  /* enforce the on_tchain write before the handlers */
        if (waiter->func)
                waiter->func(waiter);
        else
@@ -156,8 +156,7 @@ static bool __insert_awaiter(struct timer_chain *tchain,
        struct alarm_waiter *i, *temp;
        /* This will fail if you don't set a time */
        assert(waiter->wake_up_time != ALARM_POISON_TIME);
-       /* has_fired tells us if it is on the tchain or not */
-       waiter->has_fired = FALSE;
+       waiter->on_tchain = TRUE;
        /* Either the list is empty, or not. */
        if (TAILQ_EMPTY(&tchain->waiters)) {
                tchain->earliest_time = waiter->wake_up_time;
@@ -240,12 +239,10 @@ static bool __remove_awaiter(struct timer_chain *tchain,
 bool unset_alarm(struct timer_chain *tchain, struct alarm_waiter *waiter)
 {
        spin_lock_irqsave(&tchain->lock);
-       if (waiter->has_fired) {
+       if (!waiter->on_tchain) {
                /* the alarm has already gone off.  its not even on this tchain's list,
-                * though the concurrent change to has_fired (specifically, the setting
-                * of it to TRUE), happens under the tchain's lock.  As a side note, the
-                * code that sets it to FALSE is called when the waiter is on no chain,
-                * so there is no race on that. */
+                * though the concurrent change to on_tchain (specifically, the setting
+                * of it to FALSE), happens under the tchain's lock. */
                spin_unlock_irqsave(&tchain->lock);
                return FALSE;
        }
@@ -264,12 +261,12 @@ void reset_alarm_abs(struct timer_chain *tchain, struct alarm_waiter *waiter,
 {
        bool reset_int = FALSE;         /* whether or not to reset the interrupt */
        spin_lock_irqsave(&tchain->lock);
-       /* We only need to remove/unset when the alarm has not fired yet.  If it
-        * has, it's like a fresh insert */
-       if (!waiter->has_fired)
+       /* We only need to remove/unset when the alarm has not fired yet (is still
+        * on the tchain).  If it has fired, it's like a fresh insert */
+       if (waiter->on_tchain)
                reset_int = __remove_awaiter(tchain, waiter);
        set_awaiter_abs(waiter, abs_time);
-       /* regardless, we need to be reinserted, which will handle has_fired */
+       /* regardless, we need to be reinserted */
        if (__insert_awaiter(tchain, waiter) || reset_int)
                reset_tchain_interrupt(tchain);
        spin_unlock_irqsave(&tchain->lock);
index 472e706..b1797b1 100644 (file)
@@ -49,7 +49,7 @@ void rendez_sleep_timeout(struct rendez *rv, int (*cond)(void*), void *arg,
 
        assert((int)msec > 0);
        /* The handler will call rendez_wake, but won't mess with the condition
-        * state.  It's enough to break us out of cv_wait() to see .has_fired. */
+        * state.  It's enough to break us out of cv_wait() to see .on_tchain. */
        init_awaiter(&awaiter, rendez_alarm_handler);
        awaiter.data = rv;
        set_awaiter_rel(&awaiter, msec);
@@ -62,7 +62,7 @@ void rendez_sleep_timeout(struct rendez *rv, int (*cond)(void*), void *arg,
         * condition (and we should exit), other alarms with different timeouts (and
         * we should go back to sleep), etc.  Note it is possible for our alarm to
         * fire immediately upon setting it: before we even cv_lock. */
-       while (!cond(arg) && !awaiter.has_fired) {
+       while (!cond(arg) && awaiter.on_tchain) {
                cv_wait(&rv->cv);
                cpu_relax();
        }