Skip to contentSkip to conversation

sched: support dl task migration during cpu hotplug and other fixes

PATCH Superseded Wanpeng Li 4 participants 20 messages Nov 26, 2014 – Feb 4, 2015 Scheduler

v6 of 14 Superseded Latest is v14 →

lore
Share & tools
lore

Wanpeng Li

author
#0 lore raw

v6 cover sched: support dl task migration during cpu hotplug and other fixes

I collect recent send out patches to one patchset in order to get review easily.

For 7/7 I will remove rd->span related modification if Pang's effort merged.
https://lkml.org/lkml/2014/11/17/443

Wanpeng Li (7):
sched/deadline: fix start high-res preemption tick for a non-leftmost task sched/deadline: fix rt runtime corrupt when dl refuse a smaller bandwidth sched/deadline: fix dl entity is still mark yield after replenishing sched/deadline: reduce overhead if there are no scheduling parameters changed sched/fair: fix idle balance when remaining tasks are all non-CFS tasks
sched: fix start hrtick for short schedule slices on UP sched/deadline: support dl task migration during cpu hotplug

 kernel/sched/core.c     | 27 +++++++++++++++++----
 kernel/sched/deadline.c | 62 +++++++++++++++++++++++++++++++++++++++++++++----
 kernel/sched/fair.c     |  2 +-
 kernel/sched/sched.h    |  3 ++-
 4 files changed, 84 insertions(+), 10 deletions(-)
Signature · 1 line
1.9.1
author Applied
#1 lore raw

PATCH v6 1/7 sched/deadline: fix start high-res preemption tick for a non-leftmost task

Queued ticks are scheduled to match the budget, which means the budget is overall consumed and the dl task should be throttled.

Dl task will be replenished immediately if fail to start a dl timer. So the curr maybe not the left most dl task in the rb tree any more after this immediately replenished since reschedule is needed.

The parameter of task_tick_dl() queued == 1 means that hrtick is fired. hrtick() => task_tick_dl( , ,1), so p->dl.runtime should be <= 0 if queued == 1. If replenished immediately(fail to start dl timer), hrtick is start for task w/ queued == 1 && p->dl.runtime > 0 && p is not the left most task.

Start high-res preemption tick for this upcoming rescheduled dl task is not correct. This patch fix it by not starting high-res preemption tick for a non-leftmost dl task.

---
v1 -> v2:
* add a more explanatory changelog
* add a comment just above the check

 kernel/sched/deadline.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)
kernel/sched/deadline.c +12 -1
index 86d9a54..2d982ce 100644
@@ -1090,7 +1090,18 @@ static void task_tick_dl(struct rq *rq, struct task_struct *p, int queued)
 {
 	update_curr_dl(rq);
 
-	if (hrtick_enabled(rq) && queued && p->dl.runtime > 0)
+	/*
+	 * Queued tick means that hrtick is fired, when the budget is
+	 * consumed the task has to be throttled. However, task will
+	 * be replenished immediately if fail to start a dl timer, in
+	 * addition, the task maybe not the left most any more. Don't
+	 * start high-res preemption tick for the task who is not the
+	 * left most. The high-res preemption tick will be started for
+	 * this task just when it can be picked in pick_next_task_dl()
+	 * again.
+	 */
+	if (hrtick_enabled(rq) && queued && p->dl.runtime > 0 &&
+	    is_leftmost(p, &rq->dl))
 		start_hrtick_dl(rq, p);
 }
 
Signature · 1 line
1.9.1
#2 lore raw

[tip:sched/core] sched/deadline: Fix hrtick for a non-leftmost task

sched/deadline: Fix hrtick for a non-leftmost task
Commit-ID:  a7bebf488791aa1036f3e6629daf01d01f705dcb
Gitweb:     http://git.kernel.org/tip/a7bebf488791aa1036f3e6629daf01d01f705dcb
Author:     Wanpeng Li <[email protected]>
AuthorDate: Wed, 26 Nov 2014 08:44:01 +0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 4 Feb 2015 07:52:25 +0100

sched/deadline: Fix hrtick for a non-leftmost task

After update_curr_dl() the current task might not be the leftmost task anymore. In that case do not start a new hrtick for it.

In this case NEED_RESCHED will be set and the next schedule will start the hrtick for the new task if and when appropriate.

---
 kernel/sched/deadline.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
kernel/sched/deadline.c +7 -1
index e0e9c29..7b684f9 100644
@@ -1073,7 +1073,13 @@ static void task_tick_dl(struct rq *rq, struct task_struct *p, int queued)
 {
 	update_curr_dl(rq);
 
-	if (hrtick_enabled(rq) && queued && p->dl.runtime > 0)
+	/*
+	 * Even when we have runtime, update_curr_dl() might have resulted in us
+	 * not being the leftmost task anymore. In that case NEED_RESCHED will
+	 * be set and schedule() will start a new hrtick for the next task.
+	 */
+	if (hrtick_enabled(rq) && queued && p->dl.runtime > 0 &&
+	    is_leftmost(p, &rq->dl))
 		start_hrtick_dl(rq, p);
 }
 
author
#3 lore raw

PATCH v6 1/7 sched/deadline: fix start high-res preemption tick for a non-leftmost task

In reply to Wanpeng Li

Dl class will refuse the bandwidth being set to some value smaller than the currently allocated bandwidth in any of the root_domains through sched_rt_runtime_us and sched_rt_period_us. RT runtime will be set according to sched_rt_runtime_us before dl class verify if the new bandwidth is suitable in the case of !CONFIG_RT_GROUP_SCHED.

However, rt runtime will be corrupt if dl refuse the new bandwidth since there is no undo to reset the rt runtime to the old value.

This patch fix it by verifying new bandwidth for deadline in advance.

---
v1 -> v2:
* move sched_dl_global_constraints before sched_rt_global_constraints, and change the name of the former to sched_dl_global_validate().

 kernel/sched/core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
kernel/sched/core.c +3 -3
index 603c462..981f075 100644
@@ -7808,7 +7808,7 @@ static int sched_rt_global_constraints(void)
 }
 #endif /* CONFIG_RT_GROUP_SCHED */
 
-static int sched_dl_global_constraints(void)
+static int sched_dl_global_validate(void)
 {
 	u64 runtime = global_rt_runtime();
 	u64 period = global_rt_period();
@@ -7909,11 +7909,11 @@ int sched_rt_handler(struct ctl_table *table, int write,
 		if (ret)
 			goto undo;
 
-		ret = sched_rt_global_constraints();
+		ret = sched_dl_global_validate();
 		if (ret)
 			goto undo;
 
-		ret = sched_dl_global_constraints();
+		ret = sched_rt_global_constraints();
 		if (ret)
 			goto undo;
 
Signature · 1 line
1.9.1
author
#4 lore raw

Re: [PATCH v6 1/7] sched/deadline: fix start high-res preemption tick for a non-leftmost task

Replace the subject by "Subject: [PATCH v6 2/7] sched/deadline: fix rt runtime corrupt when dl refuse a smaller bandwidth"

Quoted 50 lines · Wanpeng Li

Dl class will refuse the bandwidth being set to some value smaller than the currently allocated bandwidth in any of the root_domains through sched_rt_runtime_us and sched_rt_period_us. RT runtime will be set according to sched_rt_runtime_us before dl class verify if the new bandwidth is suitable in the case of !CONFIG_RT_GROUP_SCHED.

However, rt runtime will be corrupt if dl refuse the new bandwidth since there is no undo to reset the rt runtime to the old value.

This patch fix it by verifying new bandwidth for deadline in advance.

---
v1 -> v2:
* move sched_dl_global_constraints before sched_rt_global_constraints, and change the name of the former to sched_dl_global_validate().

kernel/sched/core.c | 6 +++---

1 file changed, 3 insertions(+), 3 deletions(-)
kernel/sched/core.c +3 -3
index 603c462..981f075 100644
@@ -7808,7 +7808,7 @@ static int sched_rt_global_constraints(void)
 }
 #endif /* CONFIG_RT_GROUP_SCHED */
 
-static int sched_dl_global_constraints(void)
+static int sched_dl_global_validate(void)
 {
 	u64 runtime = global_rt_runtime();
 	u64 period = global_rt_period();
@@ -7909,11 +7909,11 @@ int sched_rt_handler(struct ctl_table *table, int write,
 		if (ret)
 			goto undo;
 
-		ret = sched_rt_global_constraints();
+		ret = sched_dl_global_validate();
 		if (ret)
 			goto undo;
 
-		ret = sched_dl_global_constraints();
+		ret = sched_rt_global_constraints();
 		if (ret)
 			goto undo;
 
Signature · 1 line
1.9.1
author
#5 lore raw

PATCH v6 3/7 sched/deadline: fix dl entity is still mark yield after replenishing

In reply to Wanpeng Li

Yield task semantic for deadline task is get off from the CPU until our next instance, we mark the task got to sleep until its current deadline by forcing its runtime to zero and update_curr_dl() stops it and the bandwidth timer will wake it up and will give it new scheduling parameters.

Bandwidth timer may fail to start in update_curr_dl() and instead replenishing the budget immediately, however, dl_yielded of dl_se is cleared when bandwidth timer fire, the immediate replenishing miss to reset it.

This patch fix it by resetting the dl_yielded flag after the immediate replenishing if fail to start bandwidth timer.

---
 kernel/sched/deadline.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
kernel/sched/deadline.c +4 -1
index 2d982ce..b102f35 100644
@@ -633,8 +633,11 @@ static void update_curr_dl(struct rq *rq)
 		__dequeue_task_dl(rq, curr, 0);
 		if (likely(start_dl_timer(dl_se, curr->dl.dl_boosted)))
 			dl_se->dl_throttled = 1;
-		else
+		else {
 			enqueue_task_dl(rq, curr, ENQUEUE_REPLENISH);
+			if (unlikely(dl_se->dl_yielded))
+				dl_se->dl_yielded = 0;
+		}
 
 		if (!is_leftmost(curr, &rq->dl))
 			resched_curr(rq);
Signature · 1 line
1.9.1
#6 lore raw

[tip:sched/core] sched/deadline: Fix stale yield state

sched/deadline: Fix stale yield state
Commit-ID:  1019a359d3dc4b64d0e1e5a5efcb725d5e83994d
Gitweb:     http://git.kernel.org/tip/1019a359d3dc4b64d0e1e5a5efcb725d5e83994d
Author:     Peter Zijlstra <[email protected]>
AuthorDate: Wed, 26 Nov 2014 08:44:03 +0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 4 Feb 2015 07:52:26 +0100

sched/deadline: Fix stale yield state

When we fail to start the deadline timer in update_curr_dl(), we forget to clear ->dl_yielded, resulting in wrecked time keeping.

Since the natural place to clear both ->dl_yielded and ->dl_throttled is in replenish_dl_entity(); both are after all waiting for that event; make it so.

Luckily since 67dfa1b756f2 ("sched/deadline: Implement cancel_dl_timer() to use in switched_from_dl()") the task_on_rq_queued() condition in dl_task_timer() must be true, and can therefore call enqueue_task_dl() unconditionally.

---
 kernel/sched/deadline.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)
kernel/sched/deadline.c +19 -19
index 7b684f9..a027799 100644
@@ -350,6 +350,11 @@ static void replenish_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;
 	}
+
+	if (dl_se->dl_yielded)
+		dl_se->dl_yielded = 0;
+	if (dl_se->dl_throttled)
+		dl_se->dl_throttled = 0;
 }
 
 /*
@@ -536,23 +541,19 @@ again:
 
 	sched_clock_tick();
 	update_rq_clock(rq);
-	dl_se->dl_throttled = 0;
-	dl_se->dl_yielded = 0;
-	if (task_on_rq_queued(p)) {
-		enqueue_task_dl(rq, p, ENQUEUE_REPLENISH);
-		if (dl_task(rq->curr))
-			check_preempt_curr_dl(rq, p, 0);
-		else
-			resched_curr(rq);
+	enqueue_task_dl(rq, p, ENQUEUE_REPLENISH);
+	if (dl_task(rq->curr))
+		check_preempt_curr_dl(rq, p, 0);
+	else
+		resched_curr(rq);
 #ifdef CONFIG_SMP
-		/*
-		 * Queueing this task back might have overloaded rq,
-		 * check if we need to kick someone away.
-		 */
-		if (has_pushable_dl_tasks(rq))
-			push_dl_task(rq);
+	/*
+	 * Queueing this task back might have overloaded rq,
+	 * check if we need to kick someone away.
+	 */
+	if (has_pushable_dl_tasks(rq))
+		push_dl_task(rq);
 #endif
-	}
 unlock:
 	raw_spin_unlock(&rq->lock);
 
@@ -613,10 +614,9 @@ static void update_curr_dl(struct rq *rq)
 
 	dl_se->runtime -= dl_se->dl_yielded ? 0 : delta_exec;
 	if (dl_runtime_exceeded(rq, dl_se)) {
+		dl_se->dl_throttled = 1;
 		__dequeue_task_dl(rq, curr, 0);
-		if (likely(start_dl_timer(dl_se, curr->dl.dl_boosted)))
-			dl_se->dl_throttled = 1;
-		else
+		if (unlikely(!start_dl_timer(dl_se, curr->dl.dl_boosted)))
 			enqueue_task_dl(rq, curr, ENQUEUE_REPLENISH);
 
 		if (!is_leftmost(curr, &rq->dl))
@@ -853,7 +853,7 @@ static void enqueue_task_dl(struct rq *rq, struct task_struct *p, int flags)
 	 * its rq, the bandwidth timer callback (which clearly has not
 	 * run yet) will take care of this.
 	 */
-	if (p->dl.dl_throttled)
+	if (p->dl.dl_throttled && !(flags & ENQUEUE_REPLENISH))
 		return;
 
 	enqueue_dl_entity(&p->dl, pi_se, flags);
author
#7 lore raw

PATCH v6 4/7 sched/deadline: reduce overhead if there are no scheduling parameters changed

In reply to Wanpeng Li

There is no need to dequeue/enqueue and push/pull if there are no scheduling parameters changed for dl class, actually both fair and rt class have already check if parameters changed for them to avoid unnecessary overhead. This patch add the parameters changed verify for dl class in order to reduce overhead.

---
v1 -> v2:
* cleanup alignment
* remove useless priority check

 kernel/sched/core.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)
kernel/sched/core.c +15 -1
index 981f075..837d2ea 100644
@@ -3402,6 +3402,20 @@ static bool check_same_owner(struct task_struct *p)
 	return match;
 }
 
+static bool dl_param_changed(struct task_struct *p,
+		const struct sched_attr *attr)
+{
+	struct sched_dl_entity *dl_se = &p->dl;
+
+	if (dl_se->dl_runtime != attr->sched_runtime ||
+		dl_se->dl_deadline != attr->sched_deadline ||
+		dl_se->dl_period != attr->sched_period ||
+		dl_se->flags != attr->sched_flags)
+		return true;
+
+	return false;
+}
+
 static int __sched_setscheduler(struct task_struct *p,
 				const struct sched_attr *attr,
 				bool user)
@@ -3530,7 +3544,7 @@ recheck:
 			goto change;
 		if (rt_policy(policy) && attr->sched_priority != p->rt_priority)
 			goto change;
-		if (dl_policy(policy))
+		if (dl_policy(policy) && dl_param_changed(p, attr))
 			goto change;
 
 		p->sched_reset_on_fork = reset_on_fork;
Signature · 1 line
1.9.1
#8 lore raw

[tip:sched/core] sched/deadline: Avoid pointless __setscheduler()

sched/deadline: Avoid pointless __setscheduler()
Commit-ID:  75381608e8410a72ae8b4080849dc86b472c01fb
Gitweb:     http://git.kernel.org/tip/75381608e8410a72ae8b4080849dc86b472c01fb
Author:     Wanpeng Li <[email protected]>
AuthorDate: Wed, 26 Nov 2014 08:44:04 +0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 4 Feb 2015 07:52:27 +0100

sched/deadline: Avoid pointless __setscheduler()

There is no need to dequeue/enqueue and push/pull if there are no scheduling parameters changed for the DL class.

Both fair and RT classes already check if parameters changed for them to avoid unnecessary overhead. This patch add the parameters changed test for the DL class in order to reduce overhead.

---
 kernel/sched/core.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)
kernel/sched/core.c +15 -1
index 50a5352..d59652d 100644
@@ -3417,6 +3417,20 @@ static bool check_same_owner(struct task_struct *p)
 	return match;
 }
 
+static bool dl_param_changed(struct task_struct *p,
+		const struct sched_attr *attr)
+{
+	struct sched_dl_entity *dl_se = &p->dl;
+
+	if (dl_se->dl_runtime != attr->sched_runtime ||
+		dl_se->dl_deadline != attr->sched_deadline ||
+		dl_se->dl_period != attr->sched_period ||
+		dl_se->flags != attr->sched_flags)
+		return true;
+
+	return false;
+}
+
 static int __sched_setscheduler(struct task_struct *p,
 				const struct sched_attr *attr,
 				bool user)
@@ -3545,7 +3559,7 @@ recheck:
 			goto change;
 		if (rt_policy(policy) && attr->sched_priority != p->rt_priority)
 			goto change;
-		if (dl_policy(policy))
+		if (dl_policy(policy) && dl_param_changed(p, attr))
 			goto change;
 
 		p->sched_reset_on_fork = reset_on_fork;
author
#9 lore raw

PATCH v6 5/7 sched/fair: fix idle balance when remaining tasks are all non-CFS tasks

In reply to Wanpeng Li

The overload indicator is used for knowing when we can totally avoid load balancing to a cpu that is about to go idle. We can avoid load balancing when no cpu has cfs task and both rt and deadline have push/pull mechanism to do their own balancing.

However, rq->nr_running on behalf of the total number of each class tasks on the cpu, do idle balance when remaining tasks are all non-CFS tasks does not make any sense.

This patch fix it by idle balance when there are still other CFS tasks in the rq's root domain.

---
 kernel/sched/fair.c  | 2 +-
 kernel/sched/sched.h | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)
kernel/sched/fair.c +1 -1
index a1c9267..8b7e131 100644
@@ -6192,7 +6192,7 @@ static inline void update_sg_lb_stats(struct lb_env *env,
 		sgs->group_load += load;
 		sgs->sum_nr_running += rq->cfs.h_nr_running;
 
-		if (rq->nr_running > 1)
+		if (rq->nr_running > 1 && rq->cfs.h_nr_running > 0)
 			*overload = true;
 
 #ifdef CONFIG_NUMA_BALANCING
kernel/sched/sched.h +2 -1
index 31f1e4d..f7dd978 100644
@@ -1269,7 +1269,8 @@ static inline void add_nr_running(struct rq *rq, unsigned count)
 
 	rq->nr_running = prev_nr + count;
 
-	if (prev_nr < 2 && rq->nr_running >= 2) {
+	if (prev_nr < 2 && rq->nr_running >= 2 &&
+		rq->cfs.h_nr_running > 0) {
 #ifdef CONFIG_SMP
 		if (!rq->rd->overload)
 			rq->rd->overload = true;
Signature · 1 line
1.9.1
maintainer
#10 lore raw

Re: [PATCH v6 5/7] sched/fair: fix idle balance when remaining tasks are all non-CFS tasks

Quoted 5 lines · Wanpeng Li

The overload indicator is used for knowing when we can totally avoid load balancing to a cpu that is about to go idle. We can avoid load balancing when no cpu has cfs task and both rt and deadline have push/pull mechanism to do their own balancing.

Quoted 7 lines

However, rq->nr_running on behalf of the total number of each class tasks on the cpu, do idle balance when remaining tasks are all non-CFS tasks does not make any sense.

This patch fix it by idle balance when there are still other CFS tasks in the rq's root domain.

Please always try and Cc the people who touched that code last; for the idle_balance bits commit 4486edd12b5a ("sched/fair: Implement fast idling of CPUs when the system is partially loaded") gives a fair clue as to who that would be.

Quoted 5 lines
kernel/sched/sched.h +0 -0
index 31f1e4d..f7dd978 100644

kernel/sched/sched.h static inline void add_nr_running(struct rq *rq, unsigned count) · line 1276

Quoted 9 lines
kernel/sched/sched.h +2 -1
@@ -1269,7 +1269,8 @@ static inline void add_nr_running(struct rq *rq, unsigned count)
 
 	rq->nr_running = prev_nr + count;
 
-	if (prev_nr < 2 && rq->nr_running >= 2) {
+	if (prev_nr < 2 && rq->nr_running >= 2 &&
+		rq->cfs.h_nr_running > 0) {
 #ifdef CONFIG_SMP
 		if (!rq->rd->overload)
 			rq->rd->overload = true;

Here 3882ec643997 ("nohz: Use IPI implicit full barrier against rq->nr_running r/w") might be a clue.

Also, this is wrong, it breaks NOHZ_FULL.

#11 lore raw

Re: [PATCH v6 5/7] sched/fair: fix idle balance when remaining tasks are all non-CFS tasks

Quoted 25 lines · Peter Zijlstra
Quoted 5 lines · Wanpeng Li

The overload indicator is used for knowing when we can totally avoid load balancing to a cpu that is about to go idle. We can avoid load balancing when no cpu has cfs task and both rt and deadline have push/pull mechanism to do their own balancing.

Quoted 7 lines

However, rq->nr_running on behalf of the total number of each class tasks on the cpu, do idle balance when remaining tasks are all non-CFS tasks does not make any sense.

This patch fix it by idle balance when there are still other CFS tasks in the rq's root domain.

Please always try and Cc the people who touched that code last; for the idle_balance bits commit 4486edd12b5a ("sched/fair: Implement fast idling of CPUs when the system is partially loaded") gives a fair clue as to who that would be.

Quoted 6 lines
kernel/sched/sched.h +0 -0
index 31f1e4d..f7dd978 100644
@@ -1269,7 +1269,8 @@ static inline void add_nr_running(struct rq *rq, unsigned count)
 
Quoted 9 lines

kernel/sched/sched.h static inline void add_nr_running(struct rq *rq, unsigned count) · line 1276

Quoted 8 lines
kernel/sched/sched.h +2 -1
 	rq->nr_running = prev_nr + count;
 
-	if (prev_nr < 2 && rq->nr_running >= 2) {
+	if (prev_nr < 2 && rq->nr_running >= 2 &&
+		rq->cfs.h_nr_running > 0) {
 #ifdef CONFIG_SMP
 		if (!rq->rd->overload)
 			rq->rd->overload = true;

After this change, you will not start cfs load balance if you have one cfs task and a few other non-cfs tasks on a run-queue. In this case if there's a less loaded run queue available, the cfs scheduler would not move the cfs task there. So you will be forcing the deadline and rt scheduler to move their tasks away. In this case, a cfs task will behave like a "higher" priority task than the rt and deadline tasks in the sense that it forces the other classes of tasks to be moved to accommodate cfs tasks. I don't think this is the right behavior.

Also, add_nr_running could add more than one cfs tasks. So if there are more than one cfs tasks being added, you should load balance and that need to be checked in add_nr_running if you make the change there.

Tim

Quoted 4 lines

Here 3882ec643997 ("nohz: Use IPI implicit full barrier against rq->nr_running r/w") might be a clue.

Also, this is wrong, it breaks NOHZ_FULL.

author
#12 lore raw

Re: [PATCH v6 5/7] sched/fair: fix idle balance when remaining tasks are all non-CFS tasks

Quoted 45 lines · Tim Chen
Quoted 25 lines · Peter Zijlstra
Quoted 5 lines · Wanpeng Li

The overload indicator is used for knowing when we can totally avoid load balancing to a cpu that is about to go idle. We can avoid load balancing when no cpu has cfs task and both rt and deadline have push/pull mechanism to do their own balancing.

Quoted 7 lines

However, rq->nr_running on behalf of the total number of each class tasks on the cpu, do idle balance when remaining tasks are all non-CFS tasks does not make any sense.

This patch fix it by idle balance when there are still other CFS tasks in the rq's root domain.

Please always try and Cc the people who touched that code last; for the idle_balance bits commit 4486edd12b5a ("sched/fair: Implement fast idling of CPUs when the system is partially loaded") gives a fair clue as to who that would be.

Quoted 6 lines
kernel/sched/sched.h +0 -0
index 31f1e4d..f7dd978 100644
@@ -1269,7 +1269,8 @@ static inline void add_nr_running(struct rq *rq, unsigned count)
 
Quoted 9 lines

kernel/sched/sched.h static inline void add_nr_running(struct rq *rq, unsigned count) · line 1276

Quoted 8 lines
kernel/sched/sched.h +2 -1
 	rq->nr_running = prev_nr + count;
 
-	if (prev_nr < 2 && rq->nr_running >= 2) {
+	if (prev_nr < 2 && rq->nr_running >= 2 &&
+		rq->cfs.h_nr_running > 0) {
 #ifdef CONFIG_SMP
 		if (!rq->rd->overload)
 			rq->rd->overload = true;

After this change, you will not start cfs load balance if you have one cfs task and a few other non-cfs tasks on a run-queue. In this case if there's a less loaded run queue available, the cfs scheduler would not move the cfs task there. So you will be forcing the deadline and rt scheduler to move their tasks away. In this case, a cfs task will behave like a "higher" priority task than the rt and deadline tasks in the sense that it forces the other classes of tasks to be moved to accommodate cfs tasks. I don't think this is the right behavior.

Quoted 3 lines

Also, add_nr_running could add more than one cfs tasks. So if there are more than one cfs tasks being added, you should load balance and that need to be checked in add_nr_running if you make the change there.

Got it, thanks.

Regards,
Wanpeng Li

Quoted 8 lines

Tim

Quoted 4 lines

Here 3882ec643997 ("nohz: Use IPI implicit full barrier against rq->nr_running r/w") might be a clue.

Also, this is wrong, it breaks NOHZ_FULL.

author
#13 lore raw

Re: [PATCH v6 5/7] sched/fair: fix idle balance when remaining tasks are all non-CFS tasks

In reply to Peter Zijlstra
Quoted 37 lines · Peter Zijlstra
Quoted 5 lines · Wanpeng Li

The overload indicator is used for knowing when we can totally avoid load balancing to a cpu that is about to go idle. We can avoid load balancing when no cpu has cfs task and both rt and deadline have push/pull mechanism to do their own balancing.

Quoted 7 lines

However, rq->nr_running on behalf of the total number of each class tasks on the cpu, do idle balance when remaining tasks are all non-CFS tasks does not make any sense.

This patch fix it by idle balance when there are still other CFS tasks in the rq's root domain.

Please always try and Cc the people who touched that code last; for the idle_balance bits commit 4486edd12b5a ("sched/fair: Implement fast idling of CPUs when the system is partially loaded") gives a fair clue as to who that would be.

Quoted 5 lines
kernel/sched/sched.h +0 -0
index 31f1e4d..f7dd978 100644

kernel/sched/sched.h static inline void add_nr_running(struct rq *rq, unsigned count) · line 1276

Quoted 9 lines
kernel/sched/sched.h +2 -1
@@ -1269,7 +1269,8 @@ static inline void add_nr_running(struct rq *rq, unsigned count)
 
 	rq->nr_running = prev_nr + count;
 
-	if (prev_nr < 2 && rq->nr_running >= 2) {
+	if (prev_nr < 2 && rq->nr_running >= 2 &&
+		rq->cfs.h_nr_running > 0) {
 #ifdef CONFIG_SMP
 		if (!rq->rd->overload)
 			rq->rd->overload = true;

Here 3882ec643997 ("nohz: Use IPI implicit full barrier against rq->nr_running r/w") might be a clue.

Quoted 1 line

Also, this is wrong, it breaks NOHZ_FULL.

Got it. If the modification in update_sg_lb_stats of my patch is still corrrect, if so I can send out a new version.

Regards,
Wanpeng Li

author
#14 lore raw

Re: [PATCH v6 5/7] sched/fair: fix idle balance when remaining tasks are all non-CFS tasks

Quoted 10 lines · Wanpeng Li
Quoted 7 lines

kernel/sched/sched.h static inline void add_nr_running(struct rq *rq, unsigned count) · line 1276

Quoted 2 lines
kernel/sched/sched.h +0 -0
 		if (!rq->rd->overload)
 			rq->rd->overload = true;

Here 3882ec643997 ("nohz: Use IPI implicit full barrier against rq->nr_running r/w") might be a clue.

Also, this is wrong, it breaks NOHZ_FULL.

Got it. If the modification in update_sg_lb_stats of my patch is still corrrect, if so I can send out a new version.

Ping,

Quoted 3 lines

Regards,
Wanpeng Li

#15 lore raw

Re: [PATCH v6 5/7] sched/fair: fix idle balance when remaining tasks are all non-CFS tasks

Quoted 7 lines · Wanpeng Li
Quoted 6 lines · Wanpeng Li
Quoted 6 lines

kernel/sched/sched.h static inline void add_nr_running(struct rq *rq, unsigned count) · line 1276

Quoted 2 lines
kernel/sched/sched.h +0 -0
 		if (!rq->rd->overload)
 			rq->rd->overload = true;

Here 3882ec643997 ("nohz: Use IPI implicit full barrier against rq->nr_running r/w") might be a clue.

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

Also, this is wrong, it breaks NOHZ_FULL.

Got it. If the modification in update_sg_lb_stats of my patch is still corrrect, if so I can send out a new version.

Ping,

Hard to respond without seeing your new patch.

Tim

Quoted 3 lines
Quoted 3 lines

Regards,
Wanpeng Li

author
#16 lore raw

PATCH v6 6/7 sched: fix start hrtick for short schedule slices on UP

In reply to Wanpeng Li

Start hrtick for schedule slices shorter than 10000ns doesn't make any sense and can cause timer DoS.

The commit 177ef2a6315e ("sched/deadline: Fix a precision problem in the microseconds range") fixes the shorter slices issue for dl class. Both the UP and SMP sides of hrtick_start() will handle the shorter slices issue before the commit, however, the UP side of hrtick_start() doesn't handle shorter slides issue any more after the commit applied.

This patch fix it by making sure the scheduling slice won't be smaller than 10us in the UP side of hrtick_start() for fair and dl schedule class.

---
 kernel/sched/core.c | 5 +++++
 1 file changed, 5 insertions(+)
kernel/sched/core.c +5 -0
index 837d2ea..b71a10e 100644
@@ -490,6 +490,11 @@ static __init void init_hrtick(void)
  */
 void hrtick_start(struct rq *rq, u64 delay)
 {
+	/*
+	 * Don't schedule slices shorter than 10000ns, that just
+	 * doesn't make sense. Rely on vruntime for fairness.
+	 */
+	delay = max_t(u64, delay, 10000LL);
 	__hrtimer_start_range_ns(&rq->hrtick_timer, ns_to_ktime(delay), 0,
 			HRTIMER_MODE_REL_PINNED, 0);
 }
Signature · 1 line
1.9.1
#17 lore raw

[tip:sched/core] sched: Fix hrtick_start() on UP

sched: Fix hrtick_start() on UP
Commit-ID:  868933359a3bdda25b562e9d41bce7071edc1b08
Gitweb:     http://git.kernel.org/tip/868933359a3bdda25b562e9d41bce7071edc1b08
Author:     Wanpeng Li <[email protected]>
AuthorDate: Wed, 26 Nov 2014 08:44:06 +0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 4 Feb 2015 07:52:28 +0100

sched: Fix hrtick_start() on UP

The commit 177ef2a6315e ("sched/deadline: Fix a precision problem in the microseconds range") forgot to change the UP version of hrtick_start(), do so now.

Fixes 177ef2a6315e ("sched/deadline: Fix a precision problem in the microseconds range")
[ Fixed the changelog. ]
Cc Juri Lelli <[email protected]>
Cc Kirill Tkhai <[email protected]>
Cc Linus Torvalds <[email protected]>
---
 kernel/sched/core.c | 5 +++++
 1 file changed, 5 insertions(+)
kernel/sched/core.c +5 -0
index d59652d..5091fd4 100644
@@ -492,6 +492,11 @@ static __init void init_hrtick(void)
  */
 void hrtick_start(struct rq *rq, u64 delay)
 {
+	/*
+	 * Don't schedule slices shorter than 10000ns, that just
+	 * doesn't make sense. Rely on vruntime for fairness.
+	 */
+	delay = max_t(u64, delay, 10000LL);
 	__hrtimer_start_range_ns(&rq->hrtick_timer, ns_to_ktime(delay), 0,
 			HRTIMER_MODE_REL_PINNED, 0);
 }
author
#18 lore raw

PATCH v6 7/7 sched/deadline: support dl task migration during cpu hotplug

In reply to Wanpeng Li

I observe that dl task can't be migrated to other cpus during cpu hotplug, in addition, task may/may not be running again if cpu is added back. The root cause which I found is that dl task will be throtted and removed from dl rq after comsuming all budget, which leads to stop task can't pick it up from dl rq and migrate to other cpus during hotplug.

The method to reproduce:
schedtool -E -t 50000:100000 -e ./test
Actually test is just a simple for loop. Then observe which cpu the test task is on.
echo 0 > /sys/devices/system/cpu/cpuN/online

This patch adds the dl task migration during cpu hotplug by finding a most suitable later deadline rq after dl timer fire if current rq is offline, if fail to find a suitable later deadline rq then fallback to any eligible online cpu in order that the deadline task will come back to us, and the push/pull mechanism should then move it around properly.

---
v5 -> v6:
* add double_lock_balance in the fallback path
v4 -> v5:
* remove raw_spin_unlock(&rq->lock)
* cleanup codes, spotted by Peterz
* cleanup patch description
v3 -> v4:
* use tsk_cpus_allowed wrapper
* fix compile error
v2 -> v3:
* don't get_task_struct
* if cannot preempt any rq, fallback to pick any online cpus
* use cpu_active_mask as original later_mask if cpu is offline
v1 -> v2:
* push the task to another cpu in dl_task_timer() if rq is offline.

 kernel/sched/deadline.c | 44 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 42 insertions(+), 2 deletions(-)
kernel/sched/deadline.c +42 -2
index b102f35..b907fdf 100644
@@ -487,6 +487,7 @@ static int start_dl_timer(struct sched_dl_entity *dl_se, bool boosted)
 	return hrtimer_active(&dl_se->dl_timer);
 }
 
+static struct rq *find_lock_later_rq(struct task_struct *task, struct rq *rq);
 /*
  * This is the bandwidth enforcement timer callback. If here, we know
  * a task is not on its dl_rq, since the fact that the timer was running
@@ -538,6 +539,44 @@ again:
 	update_rq_clock(rq);
 	dl_se->dl_throttled = 0;
 	dl_se->dl_yielded = 0;
+
+	/*
+	 * So if we find that the rq the task was on is no longer
+	 * available, we need to select a new rq.
+	 */
+	if (unlikely(!rq->online)) {
+		struct rq *later_rq = NULL;
+
+		later_rq = find_lock_later_rq(p, rq);
+
+		if (!later_rq) {
+			int cpu;
+
+			/*
+			 * If cannot preempt any rq, fallback to pick any
+			 * online cpu.
+			 */
+			cpu = cpumask_any_and(cpu_active_mask,
+					tsk_cpus_allowed(p));
+			if (cpu >= nr_cpu_ids) {
+				pr_warn("fail to find any online cpu and task will never come back\n");
+				goto unlock;
+			}
+			later_rq = cpu_rq(cpu);
+			double_lock_balance(rq, later_rq);
+		}
+
+		deactivate_task(rq, p, 0);
+		set_task_cpu(p, later_rq->cpu);
+		activate_task(later_rq, p, 0);
+
+		resched_curr(later_rq);
+
+		double_unlock_balance(rq, later_rq);
+
+		goto unlock;
+	}
+
 	if (task_on_rq_queued(p)) {
 		enqueue_task_dl(rq, p, ENQUEUE_REPLENISH);
 		if (dl_task(rq->curr))
@@ -1196,8 +1235,9 @@ static int find_later_rq(struct task_struct *task)
 	 * We have to consider system topology and task affinity
 	 * first, then we can look for a suitable cpu.
 	 */
-	cpumask_copy(later_mask, task_rq(task)->rd->span);
-	cpumask_and(later_mask, later_mask, cpu_active_mask);
+	cpumask_copy(later_mask, cpu_active_mask);
+	if (likely(task_rq(task)->online))
+		cpumask_and(later_mask, later_mask, task_rq(task)->rd->span);
 	cpumask_and(later_mask, later_mask, &task->cpus_allowed);
 	best_cpu = cpudl_find(&task_rq(task)->rd->cpudl,
 			task, later_mask);
Signature · 1 line
1.9.1
author
#19 lore raw
In reply to Wanpeng Li

Ping Juri,

Quoted 22 lines · Wanpeng Li

I collect recent send out patches to one patchset in order to get review easily.

For 7/7 I will remove rd->span related modification if Pang's effort merged.
https://lkml.org/lkml/2014/11/17/443

Wanpeng Li (7):
sched/deadline: fix start high-res preemption tick for a non-leftmost task sched/deadline: fix rt runtime corrupt when dl refuse a smaller bandwidth sched/deadline: fix dl entity is still mark yield after replenishing sched/deadline: reduce overhead if there are no scheduling parameters changed sched/fair: fix idle balance when remaining tasks are all non-CFS tasks
sched: fix start hrtick for short schedule slices on UP sched/deadline: support dl task migration during cpu hotplug

kernel/sched/core.c     | 27 +++++++++++++++++----
kernel/sched/deadline.c | 62 +++++++++++++++++++++++++++++++++++++++++++++----
kernel/sched/fair.c     |  2 +-
kernel/sched/sched.h    |  3 ++-
4 files changed, 84 insertions(+), 10 deletions(-)
Signature · 1 line
1.9.1