Return bool from reset_alarm_* apis.
authorGodfrey van der Linden <gvdl@google.com>
Tue, 17 Feb 2015 22:02:35 +0000 (14:02 -0800)
committerBarret Rhoden <brho@cs.berkeley.edu>
Sun, 1 Mar 2015 14:30:44 +0000 (09:30 -0500)
In the presence of kernel pre-emption or multi-core scheduling leak free
clean up is a challenge. This idiom is the best I've found but it must
have support from the underlying apis.

Say a hot plugging device driver is dynamically unloadable and it has a
kreffed context and after a command completes we now have scheduled the
next I/O and have to reset the timeout alarm.

struct context {
bool is_active;  // Set to false on unloading
}

void start_next_io()
{
[snipped]

// Reschedule an alarm

// Always take a reference for the alarm function’s context
kref_get(context, 1);
if (reset_alarm_abs(...)) {
// We caught the alarm in time, i.e. the reference from the original
// set_awaiter_*  is still good, clear our reference.
kref_put(context);
}

[snipped]
}

void alarm_func(context)
{
 if (context->is_active) {  // Various atomic ops can go here
  // Do work referring to context
 }
 kref_put(context);  // Potentially releasing the context now
}

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

index 0fb2bf7..6bfc0fe 100644 (file)
@@ -103,13 +103,13 @@ void set_awaiter_inc(struct alarm_waiter *waiter, uint64_t usleep);
 void __set_alarm(struct timer_chain *tchain, struct alarm_waiter *waiter);
 void set_alarm(struct timer_chain *tchain, struct alarm_waiter *waiter);
 bool unset_alarm(struct timer_chain *tchain, struct alarm_waiter *waiter);
-void __reset_alarm_abs(struct timer_chain *tchain, struct alarm_waiter *waiter,
+bool __reset_alarm_abs(struct timer_chain *tchain, struct alarm_waiter *waiter,
                        uint64_t abs_time);
-void __reset_alarm_rel(struct timer_chain *tchain, struct alarm_waiter *waiter,
+bool __reset_alarm_rel(struct timer_chain *tchain, struct alarm_waiter *waiter,
                        uint64_t usleep);
-void reset_alarm_abs(struct timer_chain *tchain, struct alarm_waiter *waiter,
+bool reset_alarm_abs(struct timer_chain *tchain, struct alarm_waiter *waiter,
                      uint64_t abs_time);
-void reset_alarm_rel(struct timer_chain *tchain, struct alarm_waiter *waiter,
+bool reset_alarm_rel(struct timer_chain *tchain, struct alarm_waiter *waiter,
                      uint64_t usleep);
 
 /* Blocks on the alarm waiter */
index ec1dc04..0ab20f7 100644 (file)
@@ -275,61 +275,63 @@ 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->on_tchain) {
-               /* the alarm has already gone off.  its not even on this tchain's list,
-                * 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;
-       }
-       if (__remove_awaiter(tchain, waiter))
+       bool ret = waiter->on_tchain;
+       if (ret && __remove_awaiter(tchain, waiter))
                reset_tchain_interrupt(tchain);
+
+       /* if alarm had already gone off then its not on this tchain's list, 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 TRUE;
+       return ret;
 }
 
 /* waiter may be on the tchain, or it might have fired already and be off the
  * tchain.  Either way, this will put the waiter on the list, set to go off at
  * abs_time.  If you know the alarm has fired, don't call this.  Just set the
  * awaiter, and then set_alarm() */
-void __reset_alarm_abs(struct timer_chain *tchain, struct alarm_waiter *waiter,
+bool __reset_alarm_abs(struct timer_chain *tchain, struct alarm_waiter *waiter,
                        uint64_t abs_time)
 {
-       bool reset_int = FALSE;         /* whether or not to reset the interrupt */
+       /* The tchain's lock is held */
+       bool ret = waiter->on_tchain;
        /* 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);
+        * on the tchain).  If it has fired, it's like a fresh insert. We must also
+        * check if we need to reset the interrupt. */
+       bool reset_int = ret && __remove_awaiter(tchain, waiter);
        set_awaiter_abs(waiter, abs_time);
        /* regardless, we need to be reinserted */
        if (__insert_awaiter(tchain, waiter) || reset_int)
                reset_tchain_interrupt(tchain);
+       return ret;
 }
 
-void __reset_alarm_rel(struct timer_chain *tchain, struct alarm_waiter *waiter,
+bool __reset_alarm_rel(struct timer_chain *tchain, struct alarm_waiter *waiter,
                        uint64_t usleep)
 {
        uint64_t now, then;
        now = read_tsc();
        then = now + usec2tsc(usleep);
        assert(now <= then);
-       __reset_alarm_abs(tchain, waiter, then);
+       return __reset_alarm_abs(tchain, waiter, then);
 }
 
-void reset_alarm_abs(struct timer_chain *tchain, struct alarm_waiter *waiter,
+bool reset_alarm_abs(struct timer_chain *tchain, struct alarm_waiter *waiter,
                      uint64_t abs_time)
 {
        spin_lock_irqsave(&tchain->lock);
-       __reset_alarm_abs(tchain, waiter, abs_time);
+       bool ret = __reset_alarm_abs(tchain, waiter, abs_time);
        spin_unlock_irqsave(&tchain->lock);
+       return ret;
 }
 
-void reset_alarm_rel(struct timer_chain *tchain, struct alarm_waiter *waiter,
+bool reset_alarm_rel(struct timer_chain *tchain, struct alarm_waiter *waiter,
                      uint64_t usleep)
 {
        spin_lock_irqsave(&tchain->lock);
-       __reset_alarm_rel(tchain, waiter, usleep);
+       bool ret =__reset_alarm_rel(tchain, waiter, usleep);
        spin_unlock_irqsave(&tchain->lock);
+       return ret;
 }
 
 /* Attempts to sleep on the alarm.  Could fail if you aren't allowed to kthread