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

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

[Wanpeng Li]

Replace the subject by "Subject: [PATCH v6 2/7] sched/deadline: fix rt runtime corrupt when dl refuse a smaller bandwidth"
On Wed, Nov 26, 2014 at 08:44:02AM +0800, Wanpeng Li wrote:
>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.
>
>Cc: Juri Lelli <juri.lelli@arm.com>
>Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
>---
>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(-)
>
>diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>index 603c462..981f075 100644
>--- a/kernel/sched/core.c
>+++ b/kernel/sched/core.c
>@@ -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;
> 
>-- 
>1.9.1

[PATCH v6 0/7] sched: support dl task migration during cpu hotplug and other fixes

[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(-)

-- 
1.9.1

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

[Wanpeng Li]

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.

Cc: Juri Lelli <juri.lelli@arm.com>
Acked-by: Juri Lelli <juri.lelli@arm.com>
Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
---
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(-)

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 86d9a54..2d982ce 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -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);
 }
 
-- 
1.9.1

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

[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.

Cc: Juri Lelli <juri.lelli@arm.com>
Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
---
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(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 603c462..981f075 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -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;
 
-- 
1.9.1

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

[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.

Cc: Juri Lelli <juri.lelli@arm.com>
Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
---
 kernel/sched/deadline.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 2d982ce..b102f35 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -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);
-- 
1.9.1

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

[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.

Cc: Juri Lelli <juri.lelli@arm.com>
Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
---
v1 -> v2:
 * cleanup alignment
 * remove useless priority check

 kernel/sched/core.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 981f075..837d2ea 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -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;
-- 
1.9.1

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

[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.

Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
---
 kernel/sched/fair.c  | 2 +-
 kernel/sched/sched.h | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index a1c9267..8b7e131 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -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
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 31f1e4d..f7dd978 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -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;
-- 
1.9.1

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

[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.

Cc: Juri Lelli <juri.lelli@arm.com>
Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
---
 kernel/sched/core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 837d2ea..b71a10e 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -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);
 }
-- 
1.9.1

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

[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.

Cc: Juri Lelli <juri.lelli@arm.com>
Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
---
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(-)

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index b102f35..b907fdf 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -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);
-- 
1.9.1

Re: [PATCH v6 0/7] sched: support dl task migration during cpu hotplug and other fixes

[Wanpeng Li]

Ping Juri,
On Wed, Nov 26, 2014 at 08:44:00AM +0800, Wanpeng Li wrote:
>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(-)
>
>-- 
>1.9.1

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

[Peter Zijlstra]

On Wed, Nov 26, 2014 at 08:44:05AM +0800, Wanpeng Li wrote:
> 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.
> 

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.

> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 31f1e4d..f7dd978 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -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.

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

[Tim Chen]

On Mon, 2015-01-19 at 13:45 +0100, Peter Zijlstra wrote:
> On Wed, Nov 26, 2014 at 08:44:05AM +0800, Wanpeng Li wrote:
> > 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.
> > 
> 
> 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.
> 
> > diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> > index 31f1e4d..f7dd978 100644
> > --- a/kernel/sched/sched.h
> > +++ b/kernel/sched/sched.h
> > @@ -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;
> 

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

> 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.

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

[Wanpeng Li]

On Mon, Jan 19, 2015 at 09:48:45AM -0800, Tim Chen wrote:
>On Mon, 2015-01-19 at 13:45 +0100, Peter Zijlstra wrote:
>> On Wed, Nov 26, 2014 at 08:44:05AM +0800, Wanpeng Li wrote:
>> > 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.
>> > 
>> 
>> 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.
>> 
>> > diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
>> > index 31f1e4d..f7dd978 100644
>> > --- a/kernel/sched/sched.h
>> > +++ b/kernel/sched/sched.h
>> > @@ -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;
>> 
>
>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.

Got it, thanks.

Regards,
Wanpeng Li 

>
>Tim
>
>> 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.
>

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

[Wanpeng Li]

On Mon, Jan 19, 2015 at 01:45:28PM +0100, Peter Zijlstra wrote:
>On Wed, Nov 26, 2014 at 08:44:05AM +0800, Wanpeng Li wrote:
>> 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.
>> 
>
>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.
>
>> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
>> index 31f1e4d..f7dd978 100644
>> --- a/kernel/sched/sched.h
>> +++ b/kernel/sched/sched.h
>> @@ -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.

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 

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

[Wanpeng Li]

On Tue, Jan 20, 2015 at 07:18:37AM +0800, Wanpeng Li wrote:
>>>  		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,

>
>Regards,
>Wanpeng Li 

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

[Tim Chen]

On Thu, 2015-01-22 at 12:05 +0800, Wanpeng Li wrote:
> On Tue, Jan 20, 2015 at 07:18:37AM +0800, Wanpeng Li wrote:
> >>>  		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,
> 

Hard to respond without seeing your new patch.

Tim

> >
> >Regards,
> >Wanpeng Li 

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

[tip-bot for Wanpeng Li]

Commit-ID:  a7bebf488791aa1036f3e6629daf01d01f705dcb
Gitweb:     http://git.kernel.org/tip/a7bebf488791aa1036f3e6629daf01d01f705dcb
Author:     Wanpeng Li <wanpeng.li@linux.intel.com>
AuthorDate: Wed, 26 Nov 2014 08:44:01 +0800
Committer:  Ingo Molnar <mingo@kernel.org>
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.

Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
Acked-by: Juri Lelli <juri.lelli@arm.com>
[ Rewrote the changelog and comment. ]
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Kirill Tkhai <ktkhai@parallels.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/1416962647-76792-2-git-send-email-wanpeng.li@linux.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/sched/deadline.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index e0e9c29..7b684f9 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -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);
 }
 

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

[tip-bot for Peter Zijlstra]

Commit-ID:  1019a359d3dc4b64d0e1e5a5efcb725d5e83994d
Gitweb:     http://git.kernel.org/tip/1019a359d3dc4b64d0e1e5a5efcb725d5e83994d
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Wed, 26 Nov 2014 08:44:03 +0800
Committer:  Ingo Molnar <mingo@kernel.org>
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.

Reported-by: Wanpeng Li <wanpeng.li@linux.intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Kirill Tkhai <ktkhai@parallels.com>
Cc: Juri Lelli <juri.lelli@arm.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/1416962647-76792-4-git-send-email-wanpeng.li@linux.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/sched/deadline.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 7b684f9..a027799 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -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);

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

[tip-bot for Wanpeng Li]

Commit-ID:  75381608e8410a72ae8b4080849dc86b472c01fb
Gitweb:     http://git.kernel.org/tip/75381608e8410a72ae8b4080849dc86b472c01fb
Author:     Wanpeng Li <wanpeng.li@linux.intel.com>
AuthorDate: Wed, 26 Nov 2014 08:44:04 +0800
Committer:  Ingo Molnar <mingo@kernel.org>
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.

Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
[ Fixed up the changelog. ]
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Juri Lelli <juri.lelli@arm.com>
Cc: Kirill Tkhai <ktkhai@parallels.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/1416962647-76792-5-git-send-email-wanpeng.li@linux.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/sched/core.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 50a5352..d59652d 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -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;

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

[tip-bot for Wanpeng Li]

Commit-ID:  868933359a3bdda25b562e9d41bce7071edc1b08
Gitweb:     http://git.kernel.org/tip/868933359a3bdda25b562e9d41bce7071edc1b08
Author:     Wanpeng Li <wanpeng.li@linux.intel.com>
AuthorDate: Wed, 26 Nov 2014 08:44:06 +0800
Committer:  Ingo Molnar <mingo@kernel.org>
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.

Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
Fixes: 177ef2a6315e ("sched/deadline: Fix a precision problem in the microseconds range")
[ Fixed the changelog. ]
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Juri Lelli <juri.lelli@arm.com>
Cc: Kirill Tkhai <ktkhai@parallels.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/1416962647-76792-7-git-send-email-wanpeng.li@linux.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/sched/core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index d59652d..5091fd4 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -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);
 }