Messages in this thread Patches in this message | | | Date | Fri, 19 Feb 2016 14:43:47 +0100 | From | luca abeni <> | Subject | Re: [PATCH 1/2] sched/deadline: add per rq tracking of admitted bandwidth |
| |
Hi all,
On Wed, 10 Feb 2016 11:58:12 +0000 Juri Lelli <juri.lelli@arm.com> wrote: [...] > > > } else if (!dl_policy(policy) && task_has_dl_policy(p)) { > > > __dl_clear(dl_b, p->dl.dl_bw); > > > + __dl_sub_ac(task_rq(p), p->dl.dl_bw); > > > > Instead of adding __dl_add_ac() and __dl_sub_ac) calls here, maybe > > they can be added in switched_to_dl() and switched_from_dl()? > > > > That might work too yes. I think there is value if we are able to move > all __dl_{add,sub}_ac calls in deadline.c. Actually, we should > probably move __dl_{add,clear} there as well, so that changes to rq > and root_domain happen at the same time. > > > I'll test this idea locally, and I'll send an updated patch if it > > works. > > > > Thanks! Will wait for it :).
I know there are still open issues with select_fallback_rq() and similar stuff, but as promised I worked on moving the __dl_*_ac() calls from core.c to deadline.c (which is orthogonal respect to the select_fallback_rq() thing). I post these patches so that people can see how the code would look like after moving __dl_*_ac() calls to deadline.c and can provide some comments; the patches are not submitted for inclusion (yet).
So, the first attached patch (to be applied over Juri's patch) just moves two __dl_sub_ac() and __dl_add_ac() invocations from dl_overflow() to deadline.c, using the switched_from_dl() and switched_to_dl() methods. This should cover the cases of tasks moving from SCHED_{OTHER,RT} to SCHED_DEADLINE and vice-versa. The case in which the deadline parameters of a task are changed still needs some __dl_* calls in dl_overflow().
These calls are moved to deadline.c by the second attached patch, which uses prio_changed_dl()... Here, prio_changed_dl() needs to know the "old" task utilization, while it is given the "old task priority" (which, for a deadline task is equal to the current priority)... So, I changed the meaning of the third parameter of prio_changed_dl(), from "old priority" to "old utilization". I know that changing the meaning of a parameter between different scheduling classes might be a bad idea... But the "prio_changed" method seems to be designed for priority-based schedulers only, and does not provide good information to the SCHED_DEADLINE scheduling class... The alternative is to change the prototype of the function, adding an "oldbw" (or, better, "oldperiod" and "oldruntime") parameter. Let me know what you think about this.
Notice that the second patch also contains two hunks that can be extracted from it (if needed): 1) remove the call to switched_to_dl() from prio_changed_dl(): this call seems to be useless 2) change __sched_setscheduler() to invoke prio_changed for deadline tasks changing their parameters. Currently, this method is not called (because when changing parameters deadline tasks do not change their priority, so new_effective_prio == oldprio). I suspect calling prio_changed is the correct behaviour; of course, this can be done in different ways, let me know your opinion.
I tested these two patches in various ways, and I do not see regressions respect to Juri's patch (but I expect issues with select_fallback_rq()... BTW, if anyone can provide some testcases for it, I can try to fix that case too).
Finally, when playing with switched_to_dl() I realized that it can be used to remove the dl_new field. So, I wrote patch 0003 (attached), which seems to be working correctly, but I am probably missing some important tests. Let me know what you think about it: I think it might be a nice simplification of the code, but as usual I might be missing something :)
Thanks, LucaFrom 704d8f7f803ff10d2e84a9fcd4737ce684d7ef78 Mon Sep 17 00:00:00 2001 From: Luca Abeni <luca.abeni@unitn.it> Date: Thu, 11 Feb 2016 13:12:12 +0100 Subject: [PATCH 1/4] Move some calls to __dl_{sub,add}_ac() from core.c to deadline.c
This moves some deadline-specific calls from core.c (dl_overflow()) switched_from_dl() and switched_to_dl(). Some __dl_{sub,add}_ac() calls are left in dl_overflow(), to handle the case in which the deadline parameters of a task are changed without changing the scheduling class. --- kernel/sched/core.c | 2 -- kernel/sched/deadline.c | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 0ee0ec2..a4f08d1 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -2445,7 +2445,6 @@ static int dl_overflow(struct task_struct *p, int policy, if (dl_policy(policy) && !task_has_dl_policy(p) && !__dl_overflow(dl_b, cpus, 0, new_bw)) { __dl_add(dl_b, new_bw); - __dl_add_ac(task_rq(p), new_bw); err = 0; } else if (dl_policy(policy) && task_has_dl_policy(p) && !__dl_overflow(dl_b, cpus, p->dl.dl_bw, new_bw)) { @@ -2456,7 +2455,6 @@ static int dl_overflow(struct task_struct *p, int policy, err = 0; } else if (!dl_policy(policy) && task_has_dl_policy(p)) { __dl_clear(dl_b, p->dl.dl_bw); - __dl_sub_ac(task_rq(p), p->dl.dl_bw); err = 0; } raw_spin_unlock(&dl_b->lock); diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c index 8ac0fee..4cc713a 100644 --- a/kernel/sched/deadline.c +++ b/kernel/sched/deadline.c @@ -1710,6 +1710,8 @@ void __init init_sched_dl_class(void) static void switched_from_dl(struct rq *rq, struct task_struct *p) { + __dl_sub_ac(task_rq(p), p->dl.dl_bw); + /* * Start the deadline timer; if we switch back to dl before this we'll * continue consuming our current CBS slice. If we stay outside of @@ -1736,6 +1738,7 @@ static void switched_from_dl(struct rq *rq, struct task_struct *p) */ static void switched_to_dl(struct rq *rq, struct task_struct *p) { + __dl_add_ac(rq, p->dl.dl_bw); if (task_on_rq_queued(p) && rq->curr != p) { #ifdef CONFIG_SMP if (p->nr_cpus_allowed > 1 && rq->dl.overloaded) -- 2.5.0 From e9703fcf14722111e16657ba4e9b08055fb4ba30 Mon Sep 17 00:00:00 2001 From: Luca Abeni <luca.abeni@unitn.it> Date: Thu, 11 Feb 2016 15:33:23 +0100 Subject: [PATCH 2/4] Move the remaining __dl_{sub,add}_ac() calls from core.c to deadline.c
To move these calls from dl_overflow() to deadline.c, we must change the meaning of the third parameter of prio_changed_dl(). Instead of passing the "old priority" (which is always equal to the current one, for SCHED_DEADLINE) we pass the old utilization. An alternative approach is to change the prototype of the "prio_changed" method of the scheduling class. --- kernel/sched/core.c | 10 ++++++---- kernel/sched/deadline.c | 10 +++++++--- 2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c index a4f08d1..5dc12db 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -2449,9 +2449,7 @@ static int dl_overflow(struct task_struct *p, int policy, } else if (dl_policy(policy) && task_has_dl_policy(p) && !__dl_overflow(dl_b, cpus, p->dl.dl_bw, new_bw)) { __dl_clear(dl_b, p->dl.dl_bw); - __dl_sub_ac(task_rq(p), p->dl.dl_bw); __dl_add(dl_b, new_bw); - __dl_add_ac(task_rq(p), new_bw); err = 0; } else if (!dl_policy(policy) && task_has_dl_policy(p)) { __dl_clear(dl_b, p->dl.dl_bw); @@ -3522,6 +3520,7 @@ void rt_mutex_setprio(struct task_struct *p, int prio) } else p->dl.dl_boosted = 0; p->sched_class = &dl_sched_class; + oldprio = 0; } else if (rt_prio(prio)) { if (dl_prio(oldprio)) p->dl.dl_boosted = 0; @@ -3891,7 +3890,7 @@ static int __sched_setscheduler(struct task_struct *p, { int newprio = dl_policy(attr->sched_policy) ? MAX_DL_PRIO - 1 : MAX_RT_PRIO - 1 - attr->sched_priority; - int retval, oldprio, oldpolicy = -1, queued, running; + int retval, oldprio, oldbw, oldpolicy = -1, queued, running; int new_effective_prio, policy = attr->sched_policy; unsigned long flags; const struct sched_class *prev_class; @@ -4069,6 +4068,7 @@ change: p->sched_reset_on_fork = reset_on_fork; oldprio = p->prio; + oldbw = p->dl.dl_bw; if (pi) { /* @@ -4081,6 +4081,8 @@ change: new_effective_prio = rt_mutex_get_effective_prio(p, newprio); if (new_effective_prio == oldprio) { __setscheduler_params(p, attr); + if (p->sched_class == &dl_sched_class) + p->sched_class->prio_changed(rq, p, oldbw); task_rq_unlock(rq, p, &flags); return 0; } @@ -4110,7 +4112,7 @@ change: enqueue_task(rq, p, enqueue_flags); } - check_class_changed(rq, p, prev_class, oldprio); + check_class_changed(rq, p, prev_class, ((prev_class == &dl_sched_class) && (p->sched_class == &dl_sched_class)) ? oldbw : oldprio); preempt_disable(); /* avoid rq from going away on us */ task_rq_unlock(rq, p, &flags); diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c index 4cc713a..959e7b7 100644 --- a/kernel/sched/deadline.c +++ b/kernel/sched/deadline.c @@ -1757,8 +1757,13 @@ static void switched_to_dl(struct rq *rq, struct task_struct *p) * a push or pull operation might be needed. */ static void prio_changed_dl(struct rq *rq, struct task_struct *p, - int oldprio) + int oldbw) { + if (oldbw) { + __dl_sub_ac(rq, oldbw); + __dl_add_ac(rq, p->dl.dl_bw); + } + if (task_on_rq_queued(p) || rq->curr == p) { #ifdef CONFIG_SMP /* @@ -1785,8 +1790,7 @@ static void prio_changed_dl(struct rq *rq, struct task_struct *p, */ resched_curr(rq); #endif /* CONFIG_SMP */ - } else - switched_to_dl(rq, p); + } } const struct sched_class dl_sched_class = { -- 2.5.0 From 12466b28951fc9327af66e450d046cd232a41354 Mon Sep 17 00:00:00 2001 From: Luca Abeni <luca.abeni@unitn.it> Date: Fri, 19 Feb 2016 12:22:59 +0100 Subject: [PATCH 3/4] Remove dl_new
switched_to_dl() can be used instead --- kernel/sched/core.c | 1 - kernel/sched/deadline.c | 28 +++++----------------------- 2 files changed, 5 insertions(+), 24 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 5dc12db..4246b1b 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -2183,7 +2183,6 @@ void __dl_clear_params(struct task_struct *p) dl_se->dl_bw = 0; dl_se->dl_throttled = 0; - dl_se->dl_new = 1; dl_se->dl_yielded = 0; } diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c index 959e7b7..12cb934 100644 --- a/kernel/sched/deadline.c +++ b/kernel/sched/deadline.c @@ -355,8 +355,6 @@ static inline void setup_new_dl_entity(struct sched_dl_entity *dl_se, struct dl_rq *dl_rq = dl_rq_of_se(dl_se); struct rq *rq = rq_of_dl_rq(dl_rq); - WARN_ON(!dl_se->dl_new || dl_se->dl_throttled); - /* * We use the regular wall clock time to set deadlines in the * future; in fact, we must consider execution overheads (time @@ -364,7 +362,6 @@ static inline void setup_new_dl_entity(struct sched_dl_entity *dl_se, */ dl_se->deadline = rq_clock(rq) + pi_se->dl_deadline; dl_se->runtime = pi_se->dl_runtime; - dl_se->dl_new = 0; } /* @@ -503,15 +500,6 @@ static void update_dl_entity(struct sched_dl_entity *dl_se, struct dl_rq *dl_rq = dl_rq_of_se(dl_se); struct rq *rq = rq_of_dl_rq(dl_rq); - /* - * The arrival of a new instance needs special treatment, i.e., - * the actual scheduling parameters have to be "renewed". - */ - if (dl_se->dl_new) { - setup_new_dl_entity(dl_se, pi_se); - return; - } - if (dl_time_before(dl_se->deadline, rq_clock(rq)) || dl_entity_overflow(dl_se, pi_se, rq_clock(rq))) { dl_se->deadline = rq_clock(rq) + pi_se->dl_deadline; @@ -608,16 +596,6 @@ static enum hrtimer_restart dl_task_timer(struct hrtimer *timer) } /* - * This is possible if switched_from_dl() raced against a running - * callback that took the above !dl_task() path and we've since then - * switched back into SCHED_DEADLINE. - * - * There's nothing to do except drop our task reference. - */ - if (dl_se->dl_new) - goto unlock; - - /* * The task might have been boosted by someone else and might be in the * boosting/deboosting path, its not throttled. */ @@ -920,7 +898,7 @@ enqueue_dl_entity(struct sched_dl_entity *dl_se, * parameters of the task might need updating. Otherwise, * we want a replenishment of its runtime. */ - if (dl_se->dl_new || flags & ENQUEUE_WAKEUP) + if (flags & ENQUEUE_WAKEUP) update_dl_entity(dl_se, pi_se); else if (flags & ENQUEUE_REPLENISH) replenish_dl_entity(dl_se, pi_se); @@ -1738,6 +1716,10 @@ static void switched_from_dl(struct rq *rq, struct task_struct *p) */ static void switched_to_dl(struct rq *rq, struct task_struct *p) { + if (p->dl.deadline <= rq_clock(rq)) { + setup_new_dl_entity(&p->dl, &p->dl); + } + __dl_add_ac(rq, p->dl.dl_bw); if (task_on_rq_queued(p) && rq->curr != p) { #ifdef CONFIG_SMP -- 2.5.0
| |