Commit 40767b0d authored by Peter Zijlstra's avatar Peter Zijlstra Committed by Ingo Molnar
Browse files

sched/deadline: Fix deadline parameter modification handling 



Commit 67dfa1b7 ("sched/deadline: Implement cancel_dl_timer() to
use in switched_from_dl()") removed the hrtimer_try_cancel() function
call out from init_dl_task_timer(), which gets called from
__setparam_dl().

The result is that we can now re-init the timer while its active --
this is bad and corrupts timer state.

Furthermore; changing the parameters of an active deadline task is
tricky in that you want to maintain guarantees, while immediately
effective change would allow one to circumvent the CBS guarantees --
this too is bad, as one (bad) task should not be able to affect the
others.

Rework things to avoid both problems. We only need to initialize the
timer once, so move that to __sched_fork() for new tasks.

Then make sure __setparam_dl() doesn't affect the current running
state but only updates the parameters used to calculate the next
scheduling period -- this guarantees the CBS functions as expected
(albeit slightly pessimistic).

This however means we need to make sure __dl_clear_params() needs to
reset the active state otherwise new (and tasks flipping between
classes) will not properly (re)compute their first instance.

Todo: close class flipping CBS hole.
Todo: implement delayed BW release.

Reported-by: default avatarLuca Abeni <luca.abeni@unitn.it>
Acked-by: default avatarJuri Lelli <juri.lelli@arm.com>
Tested-by: default avatarLuca Abeni <luca.abeni@unitn.it>
Fixes: 67dfa1b7 ("sched/deadline: Implement cancel_dl_timer() to use in switched_from_dl()")
Signed-off-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
Cc: <stable@vger.kernel.org>
Cc: Kirill Tkhai <tkhai@yandex.ru>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/20150128140803.GF23038@twins.programming.kicks-ass.net


Signed-off-by: default avatarIngo Molnar <mingo@kernel.org>
parent 3e875238

kernel/sched/core.c

+28 −5
Original line number Diff line number Diff line
@@ -1814,6 +1814,10 @@ void __dl_clear_params(struct task_struct *p) 
	dl_se->dl_period = 0;

	dl_se->flags = 0;

	dl_se->dl_bw = 0;



	dl_se->dl_throttled = 0;

	dl_se->dl_new = 1;

	dl_se->dl_yielded = 0;

}



/*
@@ -1839,7 +1843,7 @@ static void __sched_fork(unsigned long clone_flags, struct task_struct *p) 
#endif



	RB_CLEAR_NODE(&p->dl.rb_node);

	hrtimer_init(&p->dl.dl_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);

	init_dl_task_timer(&p->dl);

	__dl_clear_params(p);



	INIT_LIST_HEAD(&p->rt.run_list);
@@ -2049,6 +2053,9 @@ static inline int dl_bw_cpus(int i) 
 * allocated bandwidth to reflect the new situation.

 *

 * This function is called while holding p's rq->lock.

 *

 * XXX we should delay bw change until the task's 0-lag point, see

 * __setparam_dl().

 */

static int dl_overflow(struct task_struct *p, int policy,

		       const struct sched_attr *attr)
@@ -3251,15 +3258,31 @@ __setparam_dl(struct task_struct *p, const struct sched_attr *attr) 
{

	struct sched_dl_entity *dl_se = &p->dl;



	init_dl_task_timer(dl_se);

	dl_se->dl_runtime = attr->sched_runtime;

	dl_se->dl_deadline = attr->sched_deadline;

	dl_se->dl_period = attr->sched_period ?: dl_se->dl_deadline;

	dl_se->flags = attr->sched_flags;

	dl_se->dl_bw = to_ratio(dl_se->dl_period, dl_se->dl_runtime);

	dl_se->dl_throttled = 0;

	dl_se->dl_new = 1;

	dl_se->dl_yielded = 0;



	/*

	 * Changing the parameters of a task is 'tricky' and we're not doing

	 * the correct thing -- also see task_dead_dl() and switched_from_dl().

	 *

	 * What we SHOULD do is delay the bandwidth release until the 0-lag

	 * point. This would include retaining the task_struct until that time

	 * and change dl_overflow() to not immediately decrement the current

	 * amount.

	 *

	 * Instead we retain the current runtime/deadline and let the new

	 * parameters take effect after the current reservation period lapses.

	 * This is safe (albeit pessimistic) because the 0-lag point is always

	 * before the current scheduling deadline.

	 *

	 * We can still have temporary overloads because we do not delay the

	 * change in bandwidth until that time; so admission control is

	 * not on the safe side. It does however guarantee tasks will never

	 * consume more than promised.

	 */

}



/*

kernel/sched/deadline.c

+2 −1
Original line number Diff line number Diff line
@@ -1094,6 +1094,7 @@ static void task_dead_dl(struct task_struct *p) 
	 * Since we are TASK_DEAD we won't slip out of the domain!

	 */

	raw_spin_lock_irq(&dl_b->lock);

	/* XXX we should retain the bw until 0-lag */

	dl_b->total_bw -= p->dl.dl_bw;

	raw_spin_unlock_irq(&dl_b->lock);
@@ -1614,8 +1615,8 @@ static void cancel_dl_timer(struct rq *rq, struct task_struct *p) 


static void switched_from_dl(struct rq *rq, struct task_struct *p)

{

	/* XXX we should retain the bw until 0-lag */

	cancel_dl_timer(rq, p);



	__dl_clear_params(p);



	/*

CRA Git | Maintained and supported by SUSTech CRA and CCSE