Commit 3f1d2a31 authored by Peter Zijlstra's avatar Peter Zijlstra Committed by Thomas Gleixner
Browse files

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: 38033c37 ("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: default avatarKirill Tkhai <tkhai@yandex.ru>
Reported-by: default avatarDan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: default avatarPeter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20140212094930.GB3545@laptop.programming.kicks-ass.net


Signed-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
parent 6e83125c

kernel/sched/core.c

+17 −1
Original line number Diff line number Diff line
@@ -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);

kernel/sched/deadline.c

+1 −2
Original line number Diff line number Diff line
@@ -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);

kernel/sched/fair.c

+2 −3
Original line number Diff line number Diff line
@@ -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);

kernel/sched/idle_task.c

+1 −2
Original line number Diff line number Diff line
@@ -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

kernel/sched/rt.c

+1 −2
Original line number Diff line number Diff line
@@ -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);

CRA Git | Maintained and supported by SUSTech CRA and CCSE