sched: Fix hotplug task migration

Dan Carpenter reported:

> kernel/sched/rt.c:1347 pick_next_task_rt() warn: variable dereferenced before check 'prev' (see line 1338)
> kernel/sched/deadline.c:1011 pick_next_task_dl() warn: variable dereferenced before check 'prev' (see line 1005)

Kirill also spotted that migrate_tasks() will have an instant NULL
deref because pick_next_task() will immediately deref prev.

Instead of fixing all the corner cases because migrate_tasks() can
pass in a NULL prev task in the unlikely case of hot-un-plug, provide
a fake task such that we can remove all the NULL checks from the far
more common paths.

A further problem; not previously spotted; is that because we pushed
pre_schedule() and idle_balance() into pick_next_task() we now need to
avoid those getting called and pulling more tasks on our dying CPU.

We avoid pull_{dl,rt}_task() by setting fake_task.prio to MAX_PRIO+1.
We also note that since we call pick_next_task() exactly the amount of
times we have runnable tasks present, we should never land in
idle_balance().

Fixes: 38033c37faab ("sched: Push down pre_schedule() and idle_balance()")
Cc: Juri Lelli <juri.lelli@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Reported-by: Kirill Tkhai <tkhai@yandex.ru>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20140212094930.GB3545@laptop.programming.kicks-ass.net
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index fb9764fbc5373549368eff48ef72dfdd0b20ff21..49db434a35d05dac966828190a80bec6b64aa954 100644 (file)
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -4681,6 +4681,22 @@ static void calc_load_migrate(struct rq *rq)
                atomic_long_add(delta, &calc_load_tasks);
 }
 
+static void put_prev_task_fake(struct rq *rq, struct task_struct *prev)
+{
+}
+
+static const struct sched_class fake_sched_class = {
+       .put_prev_task = put_prev_task_fake,
+};
+
+static struct task_struct fake_task = {
+       /*
+        * Avoid pull_{rt,dl}_task()
+        */
+       .prio = MAX_PRIO + 1,
+       .sched_class = &fake_sched_class,
+};
+
 /*
  * Migrate all tasks from the rq, sleeping tasks will be migrated by
  * try_to_wake_up()->select_task_rq().
@@ -4721,7 +4737,7 @@ static void migrate_tasks(unsigned int dead_cpu)
                if (rq->nr_running == 1)
                        break;
 
-               next = pick_next_task(rq, NULL);
+               next = pick_next_task(rq, &fake_task);
                BUG_ON(!next);
                next->sched_class->put_prev_task(rq, next);
 

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index ed31ef66ab9d1fac8cb15d60345db8fefd5747ba..bfeb84ecc32b13bc9bab0aee9262c5705f8dac8d 100644 (file)
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -1008,8 +1008,7 @@ struct task_struct *pick_next_task_dl(struct rq *rq, struct task_struct *prev)
        if (unlikely(!dl_rq->dl_nr_running))
                return NULL;
 
-       if (prev)
-               prev->sched_class->put_prev_task(rq, prev);
+       put_prev_task(rq, prev);
 
        dl_se = pick_next_dl_entity(rq, dl_rq);
        BUG_ON(!dl_se);

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 40c758bbdd571f4903098b0935a17043079dd580..e884e45982aff5ba9e40d311d65bfa894ca78ced 100644 (file)
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -4690,7 +4690,7 @@ again:
        if (!cfs_rq->nr_running)
                goto idle;
 
-       if (!prev || prev->sched_class != &fair_sched_class)
+       if (prev->sched_class != &fair_sched_class)
                goto simple;
 
        /*
@@ -4766,8 +4766,7 @@ simple:
        if (!cfs_rq->nr_running)
                goto idle;
 
-       if (prev)
-               prev->sched_class->put_prev_task(rq, prev);
+       put_prev_task(rq, prev);
 
        do {
                se = pick_next_entity(cfs_rq, NULL);

diff --git a/kernel/sched/idle_task.c b/kernel/sched/idle_task.c
index f7d03af79a5bcf22da623396bee01f21e92cf1f1..53ff9e7c76d29b79659191e0fc245ad9bfc6d8f1 100644 (file)
--- a/kernel/sched/idle_task.c
+++ b/kernel/sched/idle_task.c
@@ -26,8 +26,7 @@ static void check_preempt_curr_idle(struct rq *rq, struct task_struct *p, int fl
 static struct task_struct *
 pick_next_task_idle(struct rq *rq, struct task_struct *prev)
 {
-       if (prev)
-               prev->sched_class->put_prev_task(rq, prev);
+       put_prev_task(rq, prev);
 
        schedstat_inc(rq, sched_goidle);
 #ifdef CONFIG_SMP

diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 72f9ec759972053e23d3b1374ecbe5c4c29522f3..65c2d6881ac3cce78048cc545ba9914bf55649c1 100644 (file)
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -1344,8 +1344,7 @@ pick_next_task_rt(struct rq *rq, struct task_struct *prev)
        if (rt_rq_throttled(rt_rq))
                return NULL;
 
-       if (prev)
-               prev->sched_class->put_prev_task(rq, prev);
+       put_prev_task(rq, prev);
 
        p = _pick_next_task_rt(rq);
 

diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 92018f9821e836766ec1b9ee9c1e3eb56681406d..d276147ba5e407f9cc4e7992f46157aaa51171a2 100644 (file)
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1147,6 +1147,11 @@ struct sched_class {
 #endif
 };
 
+static inline void put_prev_task(struct rq *rq, struct task_struct *prev)
+{
+       prev->sched_class->put_prev_task(rq, prev);
+}
+
 #define sched_class_highest (&stop_sched_class)
 #define for_each_class(class) \
    for (class = sched_class_highest; class; class = class->next)

diff --git a/kernel/sched/stop_task.c b/kernel/sched/stop_task.c
index a4147c9d20178e52e4bd79d8bbf633beaf16afa1..d6ce65dde5412d4b4b9d8473caf92318ba7fcb24 100644 (file)
--- a/kernel/sched/stop_task.c
+++ b/kernel/sched/stop_task.c
@@ -31,8 +31,7 @@ pick_next_task_stop(struct rq *rq, struct task_struct *prev)
        if (!stop || !stop->on_rq)
                return NULL;
 
-       if (prev)
-               prev->sched_class->put_prev_task(rq, prev);
+       put_prev_task(rq, prev);
 
        stop->se.exec_start = rq_clock_task(rq);