alarm: do not wait for unset when resetting an alarm
authorBarret Rhoden <brho@cs.berkeley.edu>
Fri, 30 Aug 2019 19:17:00 +0000 (15:17 -0400)
committerBarret Rhoden <brho@cs.berkeley.edu>
Tue, 1 Oct 2019 14:17:01 +0000 (10:17 -0400)
reset_alarm_* are racy.  Concurrent resetters can both unset, then both
set, and set_alarm can't be called on an alarm that is already set.

Right now, the only thing going for reset_alarm is that it makes sure
the alarm has stopped before we set it again.  That seems to prevent the
race between reset_alarm for the handler and an external function, but
does nothing for the race between two random functions.

But actually, the handlers can't call reset_alarm (prior to this
commit); unset will block on the handler finishing, which is a deadlock.
(cv_wait->can_block will catch this).

Basically, handlers needed to use set_alarm to rearm, since they know
they aren't on the tchain anymore.  Hokey.  set_alarm works outside a
handler too (and not the old __set_alarm vs set_alarm nonsense), but
that's only for when you know a handler isn't armed.

A lot of code wants to use reset_alarm, which is similar to Linux's
mod_timer.  It's better to have all of those functions callable in any
context - particularly the most useful one.

This commit doesn't fix the race between unset and set yet.  Instead,
has us unset without waiting for the handler to finish before setting.
This makes it usable in alarm handlers.  You may then worry - can't this
alarm get rearmed and run on another tchain (cpu) concurrently, and then
we're racing on alarm state?  No - if the alarm was currently running,
it was presumably on the same tchain that we're putting it on -
otherwise the caller was bugged.  If it's on the same tchain, then we're
OK, since we know a given tchain processes alarms in order.

A note on naming issues: nosync vs sync, and what's the default can get
sorted out in a future commit.

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
kern/include/alarm.h
kern/src/alarm.c

index 7510d1c..7f63470 100644 (file)
@@ -103,6 +103,9 @@ void set_alarm(struct timer_chain *tchain, struct alarm_waiter *waiter);
 /* Unset and reset may block if the alarm is not IRQ.  Do not call from within a
  * handler.  Returns TRUE if you stopped the alarm from firing. */
 bool unset_alarm(struct timer_chain *tchain, struct alarm_waiter *waiter);
+/* Will not block; handler may still be running. */
+bool unset_alarm_nosync(struct timer_chain *tchain,
+                       struct alarm_waiter *waiter);
 /* Convenience wrappers for unset, then set.  Slower, but easier than just
  * setting, since you don't need to know if it fired.  Returns TRUE if the alarm
  * did not fire before your reset. */
index 6ee32e8..3ff6534 100644 (file)
@@ -277,12 +277,26 @@ bool unset_alarm(struct timer_chain *tchain, struct alarm_waiter *waiter)
        }
 }
 
+bool unset_alarm_nosync(struct timer_chain *tchain, struct alarm_waiter *waiter)
+{
+       bool ret = false;
+
+       spin_lock_irqsave(&tchain->lock);
+       if (waiter->on_tchain) {
+               if (__remove_awaiter(tchain, waiter))
+                       reset_tchain_interrupt(tchain);
+               ret = true;
+       }
+       spin_unlock_irqsave(&tchain->lock);
+       return ret;
+}
+
 bool reset_alarm_abs(struct timer_chain *tchain, struct alarm_waiter *waiter,
                      uint64_t abs_time)
 {
        bool ret;
 
-       ret = unset_alarm(tchain, waiter);
+       ret = unset_alarm_nosync(tchain, waiter);
        set_awaiter_abs(waiter, abs_time);
        set_alarm(tchain, waiter);
        return ret;
@@ -293,7 +307,7 @@ bool reset_alarm_rel(struct timer_chain *tchain, struct alarm_waiter *waiter,
 {
        bool ret;
 
-       ret = unset_alarm(tchain, waiter);
+       ret = unset_alarm_nosync(tchain, waiter);
        set_awaiter_rel(waiter, usleep);
        set_alarm(tchain, waiter);
        return ret;