Remove dead code, handle delayed migrations, and fix invalid memory handle

As we should never see one of our job completion functions called
on a task after it goes through edfsc_task_exit(), simplify that
logic to just handle migrations.

Also, hold a reference to the task_struct so that it still exists
when task_deadline_callback() is called.

2 files changed, 50 insertions, 34 deletions

diff --git a/include/litmus/rt_param.h b/include/litmus/rt_param.h
index b9169fb37d46..22ce3da19245 100644
--- a/include/litmus/rt_param.h
+++ b/include/litmus/rt_param.h
@@ -122,7 +122,6 @@ struct edfsc_params {
         cont_domain_t *domain;
         struct task_struct *container_task;
         int id;
-        int will_remove;
         int can_release; //only used for containers
     // Moved these to the struct task_struct in include/linux/sched.h so that
     // the runtime can build
diff --git a/litmus/sched_edfsc.c b/litmus/sched_edfsc.c
index 5711a09a6c36..c66461bcfca7 100644
--- a/litmus/sched_edfsc.c
+++ b/litmus/sched_edfsc.c
@@ -439,16 +439,25 @@ static void c_remove_task(struct task_struct *t)
         future_sys_util -= get_rt_utilization(t);
 }
 
-static void remove_task(struct task_struct *t)
+/**
+ * Remove a task from it's current domain and put it in a different domain.
+ * Must be called at the greater time of job completion and deadline to respect
+ * EDF-sc invariants.
+ */
+static void migrate_task(struct task_struct *t)
 {
         BUG_ON(!t);
         BUG_ON(is_container(t));
+        BUG_ON(!tsk_rt(t)->edfsc_params.move_to);
 
+        if (is_queued(t))
+                remove(tsk_rt(t)->domain, t);
         if (is_fixed(t)) {
                 c_remove_task(t);
         } else {
                 g_remove_task(t);
         }
+        tsk_rt(t)->edfsc_params.move_to = NULL;
 }
 
 // migrating or container task job_completion, called from edfsc_gschedule
@@ -463,17 +472,14 @@ static noinline void g_job_completion(struct task_struct* t, int forced)
         tsk_rt(t)->completed = 0;
         unlink(t);
 
-        // When migrating is being removed, or turned into a fixed task
-        if (is_migrating(t) && tsk_rt(t)->edfsc_params.will_remove) {
+        // When a migrating task is being turned turned into a fixed task
+        if (is_migrating(t) && tsk_rt(t)->edfsc_params.move_to) {
                 if (t->rt_param.job_params.lateness > 0) {
-                        // remove the task now
-                        if (is_queued(t))
-                                remove(tsk_rt(t)->domain, t);
-                        //g_remove_task will properly add t to the move_to container
-                        g_remove_task(t);
-                }
-                else {
-                        //TODO: deadline timer to call g_remove_task
+                        // Don't wait if late
+                        migrate_task(t);
+                } else {
+                        hrtimer_start(&t->edfsc_deadline_timer, ns_to_ktime(get_deadline(t)),
+                                                HRTIMER_MODE_ABS_PINNED);
                 }
         // When a migrating job finishes
         } else if (is_migrating(t)) {
@@ -528,18 +534,8 @@ static void c_job_completion(struct task_struct* t, int forced)
         TRACE_TASK(t, "job_completion(forced=%d).\n", forced);
 
         tsk_rt(t)->completed = 0;
-
-        if (tsk_rt(t)->edfsc_params.will_remove) {
-                if (t->rt_param.job_params.lateness > 0) {
-                        // remove the task now
-                        if (is_queued(t))
-                                remove(tsk_rt(t)->domain, t);
-                        c_remove_task(t);
-                }
-        } else {
-                prepare_for_next_period(t);
-                requeue(t);
-        }
+        prepare_for_next_period(t);
+        requeue(t);
 }
 
 // need to update cpu entries after global scheduling
@@ -947,9 +943,8 @@ static enum hrtimer_restart container_boundary(struct hrtimer *timer)
         // is actually working
         list_for_each(it, &migrating_tasks) {
                 struct task_struct* t = task_of_list_node(it);
-                if (!(tsk_rt(t)->edfsc_params.will_remove) && !is_released(t, now)
+                if (!(tsk_rt(t)->edfsc_params.move_to) && !is_released(t, now)
                                 && get_deadline(t) < get_deadline(&container_tasks[0]) + get_rt_period(&container_tasks[0])) {
-                        tsk_rt(t)->edfsc_params.will_remove = 1;
                         tsk_rt(t)->edfsc_params.move_to = NULL;
 
                         container = NULL;
@@ -1070,13 +1065,26 @@ static enum hrtimer_restart container_boundary(struct hrtimer *timer)
         return HRTIMER_RESTART;
 }
 
+/**
+ * Fired when a task reaches its deadline and is pending deletion or migration
+ */
 static enum hrtimer_restart task_deadline_callback(struct hrtimer* timer) {
         struct task_struct *t = container_of(timer, struct task_struct, edfsc_deadline_timer);
 
         BUG_ON(is_container(t));
-
-        if (!is_released(t, litmus_clock()) || budget_exhausted(t)) {
-                remove_task(t);
+        if (tsk_rt(t)->edfsc_params.move_to) {
+                // Migrate here if the task is not late, otherwise migrate in job_complete
+                if (!is_released(t, litmus_clock()) || budget_exhausted(t))
+                        migrate_task(t);
+        } else {
+                // A move to NULL means deletion
+                // HACK: See comment in edfsc_task_exit()
+                if (tsk_rt(t)->task_params.cpu)
+                        c_remove_task(t);
+                else
+                        g_remove_task(t);
+                // Release our reference to the task struct
+                put_task_struct(t);
         }
         return HRTIMER_NORESTART;
 }
@@ -1135,9 +1143,6 @@ static void edfsc_task_exit(struct task_struct* t)
 
         BUG_ON(is_container(t));
         raw_spin_lock_irqsave(&g_lock, flags);
-        // XXX: Superfluous. We never see `t` as one of our tasks again.
-        tsk_rt(t)->edfsc_params.will_remove = 1;
-
 
         // Remove this task from all members of its scheduling domain
         unlink(t);
@@ -1178,10 +1183,22 @@ static void edfsc_task_exit(struct task_struct* t)
                         unaccount_time = 0;
         }
 
+        /* Take out an extra reference on the task struct so that it's not freed until
+         * the deadline boundary timer fires and we finish with it
+         */
+        get_task_struct(t);
+        // Make it clear that this task is going away
+        tsk_rt(t)->edfsc_params.move_to = NULL;
+        /* HACK: Unfortunately, even though we hold a reference to the task struct,
+         * LITMUS clears edfsc_params before the timer expires. task_params seem
+         * untouched, so hijack task_params.cpu to indicate if this is a fixed task.
+         */
+        tsk_rt(t)->task_params.cpu = is_fixed(t);
+
         if (unaccount_time == 0)
-                remove_task(t);
+                // Don't bother setting a zero-length timer - just skip straight to the callback
+                task_deadline_callback(&t->edfsc_deadline_timer);
         else
-                // FIXME: This timer uses task struct data that may or may not exist at expiration
                 hrtimer_start(&t->edfsc_deadline_timer, ns_to_ktime(unaccount_time),
                                         HRTIMER_MODE_ABS_PINNED);