2015-05-12 14:48:32

by Xunlei Pang

[permalink] [raw]
Subject: [PATCH v3 1/4] sched/rt: Check to push the task away after its affinity was changed

From: Xunlei Pang <[email protected]>

We may suffer from extra rt overload rq due to the affinity,
so when the affinity of any runnable rt task is changed, we
should check to trigger balancing, otherwise it will cause
some unnecessary delayed real-time response. Unfortunately,
current RT global scheduler does nothing about this.

For example: a 2-cpu system with two runnable FIFO tasks(same
rt_priority) bound on CPU0, let's name them rt1(running) and
rt2(runnable) respectively; CPU1 has no RTs. Then, someone sets
the affinity of rt2 to 0x3(i.e. CPU0 and CPU1), but after this,
rt2 still can't be scheduled enters schedule(), this
definitely causes some/big response latency for rt2.

This patch modified set_cpus_allowed_rt(), if the target task
is runnable but not running, it tries to push it away once it
got migratable.

The patch also solves a problem about move_queued_task() called
in set_cpus_allowed_ptr():
When a lower priorioty rt task got migrated due to its curr cpu
isn't in the new affinity mask, after move_queued_task() it will
miss the chance of pushing away, because check_preempt_curr()
called by move_queued_task() doens't set the "need resched flag"
for lower priority tasks.

Signed-off-by: Xunlei Pang <[email protected]>
---
kernel/sched/core.c | 10 +++++++---
kernel/sched/deadline.c | 8 +++++---
kernel/sched/rt.c | 29 ++++++++++++++++++++++-------
kernel/sched/sched.h | 3 ++-
4 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index d13fc13..c995a02 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -4768,11 +4768,15 @@ static struct rq *move_queued_task(struct task_struct *p, int new_cpu)

void do_set_cpus_allowed(struct task_struct *p, const struct cpumask *new_mask)
{
+ bool updated = false;
+
if (p->sched_class->set_cpus_allowed)
- p->sched_class->set_cpus_allowed(p, new_mask);
+ updated = p->sched_class->set_cpus_allowed(p, new_mask);

- cpumask_copy(&p->cpus_allowed, new_mask);
- p->nr_cpus_allowed = cpumask_weight(new_mask);
+ if (!updated) {
+ cpumask_copy(&p->cpus_allowed, new_mask);
+ p->nr_cpus_allowed = cpumask_weight(new_mask);
+ }
}

/*
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 5e95145..3baffb2 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -1574,7 +1574,7 @@ static void task_woken_dl(struct rq *rq, struct task_struct *p)
}
}

-static void set_cpus_allowed_dl(struct task_struct *p,
+static bool set_cpus_allowed_dl(struct task_struct *p,
const struct cpumask *new_mask)
{
struct rq *rq;
@@ -1610,7 +1610,7 @@ static void set_cpus_allowed_dl(struct task_struct *p,
* it is on the rq AND it is not throttled).
*/
if (!on_dl_rq(&p->dl))
- return;
+ return false;

weight = cpumask_weight(new_mask);

@@ -1619,7 +1619,7 @@ static void set_cpus_allowed_dl(struct task_struct *p,
* can migrate or not.
*/
if ((p->nr_cpus_allowed > 1) == (weight > 1))
- return;
+ return false;

/*
* The process used to be able to migrate OR it can now migrate
@@ -1636,6 +1636,8 @@ static void set_cpus_allowed_dl(struct task_struct *p,
}

update_dl_migration(&rq->dl);
+
+ return false;
}

/* Assumes rq->lock is held */
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 8885b65..4a49c6a 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -2241,7 +2241,7 @@ static void task_woken_rt(struct rq *rq, struct task_struct *p)
push_rt_tasks(rq);
}

-static void set_cpus_allowed_rt(struct task_struct *p,
+static bool set_cpus_allowed_rt(struct task_struct *p,
const struct cpumask *new_mask)
{
struct rq *rq;
@@ -2250,18 +2250,19 @@ static void set_cpus_allowed_rt(struct task_struct *p,
BUG_ON(!rt_task(p));

if (!task_on_rq_queued(p))
- return;
+ return false;

weight = cpumask_weight(new_mask);

+ rq = task_rq(p);
+
/*
- * Only update if the process changes its state from whether it
- * can migrate or not.
+ * Skip updating the migration stuff if the process doesn't change
+ * its migrate state, but still need to check if it can be pushed
+ * away due to its new affinity.
*/
if ((p->nr_cpus_allowed > 1) == (weight > 1))
- return;
-
- rq = task_rq(p);
+ goto check_push;

/*
* The process used to be able to migrate OR it can now migrate
@@ -2278,6 +2279,20 @@ static void set_cpus_allowed_rt(struct task_struct *p,
}

update_rt_migration(&rq->rt);
+
+check_push:
+ if (weight > 1 &&
+ !task_running(rq, p) &&
+ !test_tsk_need_resched(rq->curr) &&
+ !cpumask_subset(new_mask, &p->cpus_allowed)) {
+ /* Update new affinity and try to push. */
+ cpumask_copy(&p->cpus_allowed, new_mask);
+ p->nr_cpus_allowed = weight;
+ push_rt_tasks(rq);
+ return true;
+ }
+
+ return false;
}

/* Assumes rq->lock is held */
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index e0e1299..101b359 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1189,7 +1189,8 @@ struct sched_class {
void (*task_waking) (struct task_struct *task);
void (*task_woken) (struct rq *this_rq, struct task_struct *task);

- void (*set_cpus_allowed)(struct task_struct *p,
+ /* Return true if p's affinity was updated, false otherwise. */
+ bool (*set_cpus_allowed)(struct task_struct *p,
const struct cpumask *newmask);

void (*rq_online)(struct rq *rq);
--
1.9.1


2015-05-12 14:48:18

by Xunlei Pang

[permalink] [raw]
Subject: [PATCH v3 2/4] sched/deadline: Check to push the task away after its affinity was changed

From: Xunlei Pang <[email protected]>

(Sync up the same behaviour as that of RT.)

We may suffer from extra dl overload rq due to the affinity,
so when the affinity of any runnable dl task is changed, we
should check to trigger balancing, otherwise it will cause
some unnecessary delayed real-time response. Unfortunately,
current DL global scheduler does nothing about this.

This patch modified set_cpus_allowed_dl(), if the target task
is runnable but not running and not throttled, it tries to push
it away once it got migratable.

The patch also solves a problem about move_queued_task() called
in set_cpus_allowed_ptr():
When a smaller deadline value dl task got migrated due to its curr
cpu isn't in the new affinity mask, after move_queued_task() it
will miss the chance of pushing away, because check_preempt_curr()
called by move_queued_task() doens't set the "need resched flag"
for smaller deadline value tasks.

Signed-off-by: Xunlei Pang <[email protected]>
---
kernel/sched/deadline.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 3baffb2..968612b 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -1615,11 +1615,12 @@ static bool set_cpus_allowed_dl(struct task_struct *p,
weight = cpumask_weight(new_mask);

/*
- * Only update if the process changes its state from whether it
- * can migrate or not.
+ * Skip updating the migration stuff if the process doesn't change
+ * its migrate state, but still need to check if it can be pushed
+ * away due to its new affinity.
*/
if ((p->nr_cpus_allowed > 1) == (weight > 1))
- return false;
+ goto check_push;

/*
* The process used to be able to migrate OR it can now migrate
@@ -1637,6 +1638,18 @@ static bool set_cpus_allowed_dl(struct task_struct *p,

update_dl_migration(&rq->dl);

+check_push:
+ if (weight > 1 &&
+ !task_running(rq, p) &&
+ !test_tsk_need_resched(rq->curr) &&
+ !cpumask_subset(new_mask, &p->cpus_allowed)) {
+ /* Update new affinity and try to push. */
+ cpumask_copy(&p->cpus_allowed, new_mask);
+ p->nr_cpus_allowed = weight;
+ push_dl_tasks(rq);
+ return true;
+ }
+
return false;
}

--
1.9.1

2015-05-12 14:48:55

by Xunlei Pang

[permalink] [raw]
Subject: [PATCH v3 3/4] sched/rt: Remove redundant conditions from task_woken_rt()

From: Xunlei Pang <[email protected]>

- Remove "has_pushable_tasks(rq)".
Because for queued p, "!task_running(rq, p)" and "p->nr_cpus_allowed > 1"
already imply that "has_pushable_tasks(rq)" is true.

- Remove "!test_tsk_need_resched(rq->curr)".
The condtion mainly intends to ensure higher priority rt tasks won't be pushed
away. I can think of two reasons below for getting rid of it.
1) With following "rq->curr->prio <= p->prio", we still can guarantee that
purpose. "rq->curr->prio <= p->prio" implies the "need resched flag" wasn't
set by check_preempt_curr() except the one set by check_preempt_equal_prio()
for equal prio cases(In this case, if the condition is removed, it may result
in an extra push_rt_tasks(), but this doesn't cause the wrong logic, in fact
this extra push_rt_tasks() will probably return quickly for the case).

Addtionally, there're also cases the "need resched flag" got set before the
waking, with current implementation it needn't to push lower priority tasks
as the cpu will schedule, while it will do an extra pushing if the condition
is removed. But on the other hand, we can get a timely pushing for the woken
tasks after the condition is removed(better for the non-preemptible kernel).

2) With following condtion "rq->curr->nr_cpus_allowed < 2" which was added by
commit b3bc211cfe7d ("sched: Give CPU bound RT tasks preference"), in the
scenario descibed in it, "need resched flag" was already set before by
check_preempt_curr(), thus "!test_tsk_need_resched(rq->curr)" is always false
which means with current implementation the commit is futile for task_woken_rt().
So, by removing this condition, we get the right logic.

Signed-off-by: Xunlei Pang <[email protected]>
---
kernel/sched/rt.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 4a49c6a..4bd8551 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -2232,8 +2232,6 @@ out:
static void task_woken_rt(struct rq *rq, struct task_struct *p)
{
if (!task_running(rq, p) &&
- !test_tsk_need_resched(rq->curr) &&
- has_pushable_tasks(rq) &&
p->nr_cpus_allowed > 1 &&
(dl_task(rq->curr) || rt_task(rq->curr)) &&
(rq->curr->nr_cpus_allowed < 2 ||
--
1.9.1

2015-05-12 14:48:52

by Xunlei Pang

[permalink] [raw]
Subject: [PATCH v3 4/4] sched/deadline: Remove redundant conditions from task_woken_dl()

From: Xunlei Pang <[email protected]>

Sync up the same behaviour as that of RT:
Remove "has_pushable_dl_tasks(rq)" and "!test_tsk_need_resched(rq->curr)".

Signed-off-by: Xunlei Pang <[email protected]>
---
kernel/sched/deadline.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 968612b..38c08c4 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -1564,8 +1564,6 @@ static void post_schedule_dl(struct rq *rq)
static void task_woken_dl(struct rq *rq, struct task_struct *p)
{
if (!task_running(rq, p) &&
- !test_tsk_need_resched(rq->curr) &&
- has_pushable_dl_tasks(rq) &&
p->nr_cpus_allowed > 1 &&
dl_task(rq->curr) &&
(rq->curr->nr_cpus_allowed < 2 ||
--
1.9.1