lkml.org 
[lkml]   [2016]   [Feb]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patches in this message
/
Date
From
SubjectRe: [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
\
 
 \ /
  Last update: 2016-02-19 15:01    [W:0.141 / U:0.640 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site